Changeset 136045 in webkit

Timestamp:

Nov 28, 2012 12:39:52 PM (11 years ago)

Author:

Alexandru Chiculita

Message:

[CSS Regions] Crash when using hover and first-letter inside a flow-thread
​https://bugs.webkit.org/show_bug.cgi?id=102957

Reviewed by David Hyatt.

Source/WebCore:

Some RenderObjects use a different path when they are destroyed. That's because they are dynamically
added just before layout happens and their parent is usually not their actual owner. In those cases the parent
will remove the object from the tree, but it's actually the owner that will destroy the object and all its
children.

RenderFlowThread maintains a RenderBoxRegionInfo object for each RenderObject that is rendered inside the
flow-thread. When the RenderObject is removed from the RenderFlowThread, the associated RenderBoxRegionInfo object
also needs to be removed.

In these special cases (list-marker, first-letter), the object itself was removed from the RenderFlowThread,
but its children were still left in the flow-thread. When the these special objects were later destroyed,
they will remove their own children. Removing their children means it will try to remove them from the
associated RenderFlowThread. However, in this cases there would be no link back to the parent flow-thread,
as the tree is now detached from the enclosing RenderFlowThread.

Added code that recursively removes the whole children tree from the RenderFlowThread when the root is removed.

Tests: fast/regions/firstletter-inside-flowthread.html

fast/regions/listmarker-inside-flowthread.html

• rendering/RenderObject.cpp:

(WebCore::RenderObject::willBeRemovedFromTree):
(WebCore::RenderObject::removeFromRenderFlowThread):
(WebCore):
(WebCore::RenderObject::removeFromRenderFlowThreadRecursive):

• rendering/RenderObject.h:

(RenderObject):

LayoutTests:

Added CSS Regions tests for the firstLetter and listMarker render objects that use
different destroy paths in the code.

• fast/regions/firstletter-inside-flowthread-expected.html: Added.
• fast/regions/firstletter-inside-flowthread.html: Added.
• fast/regions/listmarker-inside-flowthread-expected.html: Added.
• fast/regions/listmarker-inside-flowthread.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/regions/firstletter-inside-flowthread-expected.html (added)
• LayoutTests/fast/regions/firstletter-inside-flowthread.html (added)
• LayoutTests/fast/regions/listmarker-inside-flowthread-expected.html (added)
• LayoutTests/fast/regions/listmarker-inside-flowthread.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderObject.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderObject.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-11-28  Alexandru Chiculita  <achicu@adobe.com>
+
+        [CSS Regions] Crash when using hover and first-letter inside a flow-thread
+        https://bugs.webkit.org/show_bug.cgi?id=102957
+
+        Reviewed by David Hyatt.
+
+        Added CSS Regions tests for the firstLetter and listMarker render objects that use 
+        different destroy paths in the code.
+
+        * fast/regions/firstletter-inside-flowthread-expected.html: Added.
+        * fast/regions/firstletter-inside-flowthread.html: Added.
+        * fast/regions/listmarker-inside-flowthread-expected.html: Added.
+        * fast/regions/listmarker-inside-flowthread.html: Added.
+
 2012-11-28  Tony Chang  <tony@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-11-28  Alexandru Chiculita  <achicu@adobe.com>
+
+        [CSS Regions] Crash when using hover and first-letter inside a flow-thread
+        https://bugs.webkit.org/show_bug.cgi?id=102957
+
+        Reviewed by David Hyatt.
+
+        Some RenderObjects use a different path when they are destroyed. That's because they are dynamically
+        added just before layout happens and their parent is usually not their actual owner. In those cases the parent
+        will remove the object from the tree, but it's actually the owner that will destroy the object and all its
+        children.
+
+        RenderFlowThread maintains a RenderBoxRegionInfo object for each RenderObject that is rendered inside the
+        flow-thread. When the RenderObject is removed from the RenderFlowThread, the associated RenderBoxRegionInfo object
+        also needs to be removed.
+
+        In these special cases (list-marker, first-letter), the object itself was removed from the RenderFlowThread,
+        but its children were still left in the flow-thread. When the these special objects were later destroyed, 
+        they will remove their own children. Removing their children means it will try to remove them from the
+        associated RenderFlowThread. However, in this cases there would be no link back to the parent flow-thread,
+        as the tree is now detached from the enclosing RenderFlowThread.
+
+        Added code that recursively removes the whole children tree from the RenderFlowThread when the root is removed.
+
+        Tests: fast/regions/firstletter-inside-flowthread.html
+               fast/regions/listmarker-inside-flowthread.html
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::willBeRemovedFromTree):
+        (WebCore::RenderObject::removeFromRenderFlowThread):
+        (WebCore):
+        (WebCore::RenderObject::removeFromRenderFlowThreadRecursive):
+        * rendering/RenderObject.h:
+        (RenderObject):
+
 2012-11-28  Alexandru Chiculita  <achicu@adobe.com>
 

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -2448 +2448 @@
         parent()->dirtyLinesFromChangedChild(this);
 
-    if (inRenderFlowThread()) {
-        ASSERT(enclosingRenderFlowThread());
-        enclosingRenderFlowThread()->removeFlowChildInfo(this);
-    }
+    if (inRenderFlowThread())
+        removeFromRenderFlowThread();
 
     if (RenderNamedFlowThread* containerFlowThread = parent()->enclosingRenderNamedFlowThread())
@@ -2460 +2458 @@
     parent()->setNeedsBoundariesUpdate();
 #endif
+}
+
+void RenderObject::removeFromRenderFlowThread()
+{
+    RenderFlowThread* renderFlowThread = enclosingRenderFlowThread();
+    ASSERT(renderFlowThread);
+    // Sometimes we remove the element from the flow, but it's not destroyed at that time. 
+    // It's only until later when we actually destroy it and remove all the children from it. 
+    // Currently, that happens for firstLetter elements and list markers.
+    // Pass in the flow thread so that we don't have to look it up for all the children.
+    removeFromRenderFlowThreadRecursive(renderFlowThread);
+}
+
+void RenderObject::removeFromRenderFlowThreadRecursive(RenderFlowThread* renderFlowThread)
+{
+    if (const RenderObjectChildList* children = virtualChildren()) {
+        for (RenderObject* child = children->firstChild(); child; child = child->nextSibling())
+            child->removeFromRenderFlowThreadRecursive(renderFlowThread);
+    }
+    renderFlowThread->removeFlowChildInfo(this);
+    setInRenderFlowThread(false);
 }
 

trunk/Source/WebCore/rendering/RenderObject.h

@@ -986 +986 @@
 
 private:
+    void removeFromRenderFlowThread();
+    void removeFromRenderFlowThreadRecursive(RenderFlowThread*);
+
     RenderStyle* cachedFirstLineStyle() const;
     StyleDifference adjustStyleDifference(StyleDifference, unsigned contextSensitiveProperties) const;