Title: [118542] trunk/Source/WebCore
Revision
118542
Author
[email protected]
Date
2012-05-25 11:01:21 -0700 (Fri, 25 May 2012)

Log Message

Layout root not getting cleared for anonymous renderers geting destroyed
https://bugs.webkit.org/show_bug.cgi?id=84002

Reviewed by Abhishek Arya.

This is a follow-up to r109406, which added a check to clear layout
roots when they point to a renderer that is being destroyed. The
thinking was that layout roots would never be anonymous renderers,
but there are some cases where this is not true (in particular,
generated content containers with overflow clips can be layout roots).

As in r109406, this patch has no layout test. This is because any test
that exercises this behavior is caused by an existing layout bug where
a child is not properly getting layout (or a renderer is getting dirtied
out of order during layout) and will fail multiple ASSERTs:
in particular, ASSERT(!m_layoutRoot->container() || !m_layoutRoot->
container()->needsLayout()) in FrameView::scheduleRelayoutOfSubtree(),
and ASSERT_NOT_REACHED() in RenderObject::clearLayoutRootIfNeeded().
We are preventing those bugs from manifesting as security issues with
this patch.

This also removes an ASSERT from the RenderObject destructor. This is
redundant with the condition in RenderObject::clearLayoutRootIfNeeded()
which is always called in RenderObject::willBeDestroyed(), so the check
is not needed. It had to be removed because it fails when I try to
adjust the ASSERT condition by removing the !node()
check, due to RenderWidget clearing its node() during destruction.

* rendering/RenderObject.cpp:
(WebCore::RenderObject::~RenderObject):
(WebCore::RenderObject::willBeDestroyed):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (118541 => 118542)


--- trunk/Source/WebCore/ChangeLog	2012-05-25 17:47:30 UTC (rev 118541)
+++ trunk/Source/WebCore/ChangeLog	2012-05-25 18:01:21 UTC (rev 118542)
@@ -1,3 +1,37 @@
+2012-05-25  Ken Buchanan  <[email protected]>
+
+        Layout root not getting cleared for anonymous renderers geting destroyed
+        https://bugs.webkit.org/show_bug.cgi?id=84002
+
+        Reviewed by Abhishek Arya.
+
+        This is a follow-up to r109406, which added a check to clear layout
+        roots when they point to a renderer that is being destroyed. The
+        thinking was that layout roots would never be anonymous renderers,
+        but there are some cases where this is not true (in particular,
+        generated content containers with overflow clips can be layout roots).
+
+        As in r109406, this patch has no layout test. This is because any test
+        that exercises this behavior is caused by an existing layout bug where
+        a child is not properly getting layout (or a renderer is getting dirtied
+        out of order during layout) and will fail multiple ASSERTs:
+        in particular, ASSERT(!m_layoutRoot->container() || !m_layoutRoot->
+        container()->needsLayout()) in FrameView::scheduleRelayoutOfSubtree(),
+        and ASSERT_NOT_REACHED() in RenderObject::clearLayoutRootIfNeeded().
+        We are preventing those bugs from manifesting as security issues with
+        this patch.
+
+        This also removes an ASSERT from the RenderObject destructor. This is
+        redundant with the condition in RenderObject::clearLayoutRootIfNeeded()
+        which is always called in RenderObject::willBeDestroyed(), so the check 
+        is not needed. It had to be removed because it fails when I try to
+        adjust the ASSERT condition by removing the !node()
+        check, due to RenderWidget clearing its node() during destruction.
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::~RenderObject):
+        (WebCore::RenderObject::willBeDestroyed):
+
 2012-05-25  Alexander Pavlov  <[email protected]>
 
         Web Inspector: Hangup when continuously changing a css width value in Inspector

Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (118541 => 118542)


--- trunk/Source/WebCore/rendering/RenderObject.cpp	2012-05-25 17:47:30 UTC (rev 118541)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp	2012-05-25 18:01:21 UTC (rev 118542)
@@ -219,7 +219,6 @@
 
 RenderObject::~RenderObject()
 {
-    ASSERT(!node() || documentBeingDestroyed() || !frame()->view() || frame()->view()->layoutRoot() != this);
 #ifndef NDEBUG
     ASSERT(!m_hasAXObject);
     renderObjectCounter.decrement();
@@ -2260,7 +2259,7 @@
 
 inline void RenderObject::clearLayoutRootIfNeeded() const
 {
-    if (node() && !documentBeingDestroyed() && frame()) {
+    if (!documentBeingDestroyed() && frame()) {
         if (FrameView* view = frame()->view()) {
             if (view->layoutRoot() == this) {
                 ASSERT_NOT_REACHED();
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to