Changeset 85586 in webkit

Timestamp:

May 2, 2011 11:04:54 PM (13 years ago)

Author:

Simon Fraser

Message:

2011-05-02 Simon Fraser <Simon Fraser>

Reviewed by Dan Bernstein.

Possible crash when removing elements with reflections
​https://bugs.webkit.org/show_bug.cgi?id=60009

RenderLayer's destructor deleted its z-order list Vector pointers
before removing the reflection layer. However, the reflection cleanup
code could call back into the RenderLayer to dirty z-order lists,
so move reflection cleanup to before z-order vector deletion.

The test crashes when run manually a few times with MallocScribble enabled,
but I was not able to create a test that crashed reliably.

Test: fast/reflections/remove-reflection-crash.html

• rendering/RenderLayer.cpp: (WebCore::RenderLayer::~RenderLayer):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/reflections/remove-reflection-crash-expected.txt (added)
• LayoutTests/fast/reflections/remove-reflection-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-05-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        Possible crash when removing elements with reflections
+        https://bugs.webkit.org/show_bug.cgi?id=60009
+        
+        Testcase that sometimes crashes if run with MallocScribble enabled.
+
+        * fast/reflections/remove-reflection-crash-expected.txt: Added.
+        * fast/reflections/remove-reflection-crash.html: Added.
+
 2011-05-02  Ian Henderson  <ianh@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-05-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        Possible crash when removing elements with reflections
+        https://bugs.webkit.org/show_bug.cgi?id=60009
+        
+        RenderLayer's destructor deleted its z-order list Vector pointers
+        before removing the reflection layer. However, the reflection cleanup
+        code could call back into the RenderLayer to dirty z-order lists,
+        so move reflection cleanup to before z-order vector deletion.
+        
+        The test crashes when run manually a few times with MallocScribble enabled,
+        but I was not able to create a test that crashed reliably.
+
+        Test: fast/reflections/remove-reflection-crash.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::~RenderLayer):
+
 2011-05-02  Ian Henderson  <ianh@apple.com>
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -212 +212 @@
     destroyScrollbar(VerticalScrollbar);
 
+    if (m_reflection)
+        removeReflection();
+
     // Child layers will be deleted by their corresponding render objects, so
     // we don't need to delete them ourselves.
@@ -226 +229 @@
     // Make sure we have no lingering clip rects.
     ASSERT(!m_clipRects);
-    
-    if (m_reflection)
-        removeReflection();
     
     if (m_scrollCorner)