[webkit-changes] [117885] releases/WebKitGTK/webkit-1.8

Mon, 21 May 2012 20:03:20 -0700

Title: [117885] releases/WebKitGTK/webkit-1.8
Revision
117885
Author
[email protected]
Date
2012-05-21 20:04:03 -0700 (Mon, 21 May 2012)

Log Message

Merge 115343 - Crash from removal of line break object after layout
https://bugs.webkit.org/show_bug.cgi?id=75461

Source/WebCore:

Patch by Ken Buchanan <[email protected]> on 2012-04-26
Reviewed by David Hyatt.

There is a condition where objects can get removed from underneath
inlines while they represent a line break object in a RootInlineBox
of an ancestor block. If an intermediary inline has already been
marked as needing layout, then the line box will not get dirtied
because dirtyLineFromChangedChild thinks it already has been.

This patch introduces a new set in RenderObject to indicate whether
an ancestral line box corresponding to the current line has been
marked dirty or not. dirtyLinesFromChangedChild() can use this set
rather than m_selfNeedsLayout, so it will not be confused if a
container was dirtied for some other reason that did not affect the
line box.

* rendering/RenderLineBoxList.cpp:
(WebCore::RenderLineBoxList::dirtyLinesFromChangedChild): Use the new
set rather than m_selfNeedsLayout in the container to determine
whether to continue propagating upward.
* rendering/RenderObject.cpp:
(WebCore::RenderObject::s_ancestorLineboxDirtySet): Instantiate the
static member.
(WebCore::RenderObject::willBeDestroyed): Clears the object from the
linebox set when it is being destroyed.
* rendering/RenderObject.h:
(WebCore::RenderObject::s_ancestorLineboxDirtySet): Added static
member set.
(WebCore::RenderObject::setNeedsLayout): Clears the
object from the linebox set when layout bits are getting cleared.
(WebCore::RenderObject::ancestorLineBoxDirty): Added.
(WebCore::RenderObject::setAncestorLineBoxDirty): Added.

LayoutTests:

Patch by Ken Buchanan <[email protected]> on 2012-04-25
Reviewed by David Hyatt.

Test exercising crashing condition in bug 75461.

* fast/block/line-layout/line-break-obj-removal-crash-expected.txt: Added
* fast/block/line-layout/line-break-obj-removal-crash.html: Added

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-1.8/LayoutTests/ChangeLog (117884 => 117885)


--- releases/WebKitGTK/webkit-1.8/LayoutTests/ChangeLog	2012-05-22 03:03:41 UTC (rev 117884)
+++ releases/WebKitGTK/webkit-1.8/LayoutTests/ChangeLog	2012-05-22 03:04:03 UTC (rev 117885)
@@ -1,3 +1,15 @@
+2012-04-25 Ken Buchanan  <[email protected]>
+
+        Crash from removal of line break object after layout
+        https://bugs.webkit.org/show_bug.cgi?id=75461
+
+        Reviewed by David Hyatt.
+
+        Test exercising crashing condition in bug 75461.
+
+        * fast/block/line-layout/line-break-obj-removal-crash-expected.txt: Added
+        * fast/block/line-layout/line-break-obj-removal-crash.html: Added
+
 2012-04-04  Jeffrey Pfau  <[email protected]>
 
         Move pending sheet removal from ~HTMLLinkElement to removal from document.

Added: releases/WebKitGTK/webkit-1.8/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash-expected.txt (0 => 117885)


--- releases/WebKitGTK/webkit-1.8/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-1.8/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash-expected.txt	2012-05-22 03:04:03 UTC (rev 117885)
@@ -0,0 +1,2 @@
+
+PASS, if no crash or assert in debug

Added: releases/WebKitGTK/webkit-1.8/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash.html (0 => 117885)


--- releases/WebKitGTK/webkit-1.8/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-1.8/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash.html	2012-05-22 03:04:03 UTC (rev 117885)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<style>
+.style1:nth-last-child(odd) { padding-right: 100px; }
+.styleCounterBefore:before { position: absolute; content: counter(section); }
+</style>
+<script>
+window._onload_ = function() {
+    divElem = document.createElement('div');
+    divElem.setAttribute('class', 'styleCounterBefore');
+    document.documentElement.appendChild(divElem);
+    brElem = document.createElement('br');
+    document.documentElement.appendChild(brElem);
+    sampElem = document.createElement('samp');
+    sampElem.setAttribute('class', 'style1');
+    document.documentElement.appendChild(sampElem);
+    spanElem = document.createElement('span');
+    spanElem.setAttribute('class', 'styleCounterBefore');
+    trElem = document.createElement('tr');
+    document.documentElement.appendChild(trElem);
+    sampElem.appendChild(spanElem);
+    document.documentElement.offsetHeight;
+    document.documentElement.removeChild(trElem);
+
+    document.documentElement.appendChild(document.createTextNode('PASS, if no crash or assert in debug'));
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+}
+</script>
+</html>

Modified: releases/WebKitGTK/webkit-1.8/Source/WebCore/ChangeLog (117884 => 117885)


--- releases/WebKitGTK/webkit-1.8/Source/WebCore/ChangeLog	2012-05-22 03:03:41 UTC (rev 117884)
+++ releases/WebKitGTK/webkit-1.8/Source/WebCore/ChangeLog	2012-05-22 03:04:03 UTC (rev 117885)
@@ -1,3 +1,40 @@
+2012-04-26  Ken Buchanan  <[email protected]>
+
+        Crash from removal of line break object after layout
+        https://bugs.webkit.org/show_bug.cgi?id=75461
+
+        Reviewed by David Hyatt.
+
+        There is a condition where objects can get removed from underneath
+        inlines while they represent a line break object in a RootInlineBox
+        of an ancestor block. If an intermediary inline has already been
+        marked as needing layout, then the line box will not get dirtied
+        because dirtyLineFromChangedChild thinks it already has been.
+
+        This patch introduces a new set in RenderObject to indicate whether
+        an ancestral line box corresponding to the current line has been
+        marked dirty or not. dirtyLinesFromChangedChild() can use this set 
+        rather than m_selfNeedsLayout, so it will not be confused if a
+        container was dirtied for some other reason that did not affect the
+        line box.
+
+        * rendering/RenderLineBoxList.cpp:
+        (WebCore::RenderLineBoxList::dirtyLinesFromChangedChild): Use the new
+        set rather than m_selfNeedsLayout in the container to determine
+        whether to continue propagating upward.
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::s_ancestorLineboxDirtySet): Instantiate the
+        static member.
+        (WebCore::RenderObject::willBeDestroyed): Clears the object from the
+        linebox set when it is being destroyed.
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::s_ancestorLineboxDirtySet): Added static
+        member set.
+        (WebCore::RenderObject::setNeedsLayout): Clears the
+        object from the linebox set when layout bits are getting cleared.
+        (WebCore::RenderObject::ancestorLineBoxDirty): Added.
+        (WebCore::RenderObject::setAncestorLineBoxDirty): Added.
+
 2012-03-01  Abhishek Arya  <[email protected]>
 
         Prevent layout root to remain set on renderers getting destroyed.

Modified: releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderLineBoxList.cpp (117884 => 117885)


--- releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderLineBoxList.cpp	2012-05-22 03:03:41 UTC (rev 117884)
+++ releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderLineBoxList.cpp	2012-05-22 03:04:03 UTC (rev 117885)
@@ -321,9 +321,9 @@
     if (!firstBox) {
         // For an empty inline, go ahead and propagate the check up to our parent, unless the parent
         // is already dirty.
-        if (container->isInline() && !container->parent()->selfNeedsLayout()) {
+        if (container->isInline() && !container->parent()->ancestorLineBoxDirty()) {
             container->parent()->dirtyLinesFromChangedChild(container);
-            container->setNeedsLayout(true); // Mark the container as needing layout to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
+            container->setAncestorLineBoxDirty(); // Mark the container to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
         }
         return;
     }
@@ -361,9 +361,9 @@
             // we won't find a previous sibling, but firstBox can be pointing to a following sibling.
             // This isn't good enough, since we won't locate the root line box that encloses the removed
             // <br>. We have to just over-invalidate a bit and go up to our parent.
-            if (!inlineContainer->parent()->selfNeedsLayout()) {
+            if (!inlineContainer->parent()->ancestorLineBoxDirty()) {
                 inlineContainer->parent()->dirtyLinesFromChangedChild(inlineContainer);
-                inlineContainer->setNeedsLayout(true); // Mark the container as needing layout to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
+                inlineContainer->setAncestorLineBoxDirty(); // Mark the container to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
             }
             return;
         }

Modified: releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderObject.cpp (117884 => 117885)


--- releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderObject.cpp	2012-05-22 03:03:41 UTC (rev 117884)
+++ releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderObject.cpp	2012-05-22 03:04:03 UTC (rev 117885)
@@ -100,6 +100,8 @@
 
 bool RenderObject::s_affectsParentBlock = false;
 
+RenderObjectAncestorLineboxDirtySet* RenderObject::s_ancestorLineboxDirtySet = 0;
+
 void* RenderObject::operator new(size_t sz, RenderArena* renderArena)
 {
     return renderArena->allocate(sz);
@@ -2259,6 +2261,8 @@
         toRenderBoxModelObject(this)->destroyLayer();
     }
 
+    setAncestorLineBoxDirty(false);
+
     clearLayoutRootIfNeeded();
 }

Modified: releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderObject.h (117884 => 117885)


--- releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderObject.h	2012-05-22 03:03:41 UTC (rev 117884)
+++ releases/WebKitGTK/webkit-1.8/Source/WebCore/rendering/RenderObject.h	2012-05-22 03:04:03 UTC (rev 117885)
@@ -36,6 +36,7 @@
 #include "RenderStyle.h"
 #include "TextAffinity.h"
 #include "TransformationMatrix.h"
+#include <wtf/HashSet.h>
 #include <wtf/UnusedParam.h>
 
 #if USE(CG) || USE(CAIRO) || USE(SKIA) || PLATFORM(QT)
@@ -116,6 +117,8 @@
 };
 #endif
 
+typedef WTF::HashSet<const RenderObject*> RenderObjectAncestorLineboxDirtySet;
+
 #ifndef NDEBUG
 const int showTreeCharacterOffset = 39;
 #endif
@@ -371,6 +374,22 @@
     bool hasColumns() const { return m_bitfields.hasColumns(); }
     void setHasColumns(bool b = true) { m_bitfields.setHasColumns(b); }
 
+    bool ancestorLineBoxDirty() const { return s_ancestorLineboxDirtySet && s_ancestorLineboxDirtySet->contains(this); }
+    void setAncestorLineBoxDirty(bool b = true)
+    {
+        if (b) {
+            if (!s_ancestorLineboxDirtySet)
+                s_ancestorLineboxDirtySet = new RenderObjectAncestorLineboxDirtySet;
+            s_ancestorLineboxDirtySet->add(this);
+        } else if (s_ancestorLineboxDirtySet) {
+            s_ancestorLineboxDirtySet->remove(this);
+            if (s_ancestorLineboxDirtySet->isEmpty()) {
+                delete s_ancestorLineboxDirtySet;
+                s_ancestorLineboxDirtySet = 0;
+            }
+        }
+    }
+
     bool inRenderFlowThread() const { return m_bitfields.inRenderFlowThread(); }
     void setInRenderFlowThread(bool b = true) { m_bitfields.setInRenderFlowThread(b); }
 
@@ -891,6 +910,8 @@
     RenderObject* m_previous;
     RenderObject* m_next;
 
+    static RenderObjectAncestorLineboxDirtySet* s_ancestorLineboxDirtySet;
+
 #ifndef NDEBUG
     bool m_hasAXObject             : 1;
     bool m_setNeedsLayoutForbidden : 1;
@@ -1053,6 +1074,7 @@
         setNeedsSimplifiedNormalFlowLayout(false);
         setNormalChildNeedsLayout(false);
         setNeedsPositionedMovementLayout(false);
+        setAncestorLineBoxDirty(false);
     }
 }
 

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to