Title: [95074] trunk
Revision
95074
Author
infe...@chromium.org
Date
2011-09-13 21:19:45 -0700 (Tue, 13 Sep 2011)

Log Message

Crash in RenderScrollbarPart::imageChanged.
https://bugs.webkit.org/show_bug.cgi?id=68009

Reviewed by Simon Fraser.

Source/WebCore: 

When a custom scrollbar is removed from its FrameView, its destruction
can be delayed because of RefPtr maintained in EventHandler class
(m_lastScrollbarUnderMouse). Upon removal, we delete all the scrollbar
parts so that they don't link back to scrollbar. However, because of the
delay, we can have a call to updateScrollbarPart which recreates it.
When scrollbar is getting destroyed, we just check to see if there are
remaining scrollbar parts and if yes, we destroy them.

Test: scrollbars/scrollbar-part-created-with-no-parent-crash.html

* rendering/RenderScrollbar.cpp:
(WebCore::RenderScrollbar::~RenderScrollbar):

LayoutTests: 

* scrollbars/scrollbar-part-created-with-no-parent-crash-expected.txt: Added.
* scrollbars/scrollbar-part-created-with-no-parent-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (95073 => 95074)


--- trunk/LayoutTests/ChangeLog	2011-09-14 03:36:09 UTC (rev 95073)
+++ trunk/LayoutTests/ChangeLog	2011-09-14 04:19:45 UTC (rev 95074)
@@ -1,3 +1,13 @@
+2011-09-13  Abhishek Arya  <infe...@chromium.org>
+
+        Crash in RenderScrollbarPart::imageChanged.
+        https://bugs.webkit.org/show_bug.cgi?id=68009
+
+        Reviewed by Simon Fraser.
+
+        * scrollbars/scrollbar-part-created-with-no-parent-crash-expected.txt: Added.
+        * scrollbars/scrollbar-part-created-with-no-parent-crash.html: Added.
+
 2011-09-13  Fumitoshi Ukai  <u...@chromium.org>
 
         Unreviewed, update chromium test expectations.

Added: trunk/LayoutTests/scrollbars/scrollbar-part-created-with-no-parent-crash-expected.txt (0 => 95074)


--- trunk/LayoutTests/scrollbars/scrollbar-part-created-with-no-parent-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-part-created-with-no-parent-crash-expected.txt	2011-09-14 04:19:45 UTC (rev 95074)
@@ -0,0 +1,2 @@
+Test passes if it does not crash.
+PASS

Added: trunk/LayoutTests/scrollbars/scrollbar-part-created-with-no-parent-crash.html (0 => 95074)


--- trunk/LayoutTests/scrollbars/scrollbar-part-created-with-no-parent-crash.html	                        (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-part-created-with-no-parent-crash.html	2011-09-14 04:19:45 UTC (rev 95074)
@@ -0,0 +1,53 @@
+<html>
+Test passes if it does not crash.
+<style>
+body
+{
+    margin: 0;
+}
+::-webkit-scrollbar {
+    -webkit-logical-height: 65536;
+    -webkit-border-image: url(does_not_exist) 0 2 0 2;
+}
+
+.inner:not(table) {
+    padding: 400px;
+}
+</style>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function showScroller()
+{
+    var scroller = document.createElement('div');
+    scroller.className = 'scroller';
+
+    var contents = document.createElement('div')
+    contents.className = 'inner';
+    scroller.appendChild(contents);
+
+    document.getElementById('container').appendChild(scroller);
+}
+  
+function hideScroller()
+{
+    var scroller = document.getElementById('container').querySelectorAll('.scroller')[0];
+    scroller.parentNode.removeChild(scroller);
+}
+  
+function doTest()
+{
+    if (window.eventSender) {
+        eventSender.mouseMoveTo(50, 40);
+        eventSender.mouseMoveTo(50, 55);
+        eventSender.mouseMoveTo(50, 0);
+    }
+}
+
+window.addEventListener('load', doTest, false);
+</script>
+<div id="container" _onmouseover_="showScroller()" _onmouseout_="hideScroller()">
+<p>PASS</p>
+</div>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (95073 => 95074)


--- trunk/Source/WebCore/ChangeLog	2011-09-14 03:36:09 UTC (rev 95073)
+++ trunk/Source/WebCore/ChangeLog	2011-09-14 04:19:45 UTC (rev 95074)
@@ -1,3 +1,23 @@
+2011-09-13  Abhishek Arya  <infe...@chromium.org>
+
+        Crash in RenderScrollbarPart::imageChanged.
+        https://bugs.webkit.org/show_bug.cgi?id=68009
+
+        Reviewed by Simon Fraser.
+
+        When a custom scrollbar is removed from its FrameView, its destruction
+        can be delayed because of RefPtr maintained in EventHandler class
+        (m_lastScrollbarUnderMouse). Upon removal, we delete all the scrollbar
+        parts so that they don't link back to scrollbar. However, because of the
+        delay, we can have a call to updateScrollbarPart which recreates it.
+        When scrollbar is getting destroyed, we just check to see if there are
+        remaining scrollbar parts and if yes, we destroy them.
+
+        Test: scrollbars/scrollbar-part-created-with-no-parent-crash.html
+
+        * rendering/RenderScrollbar.cpp:
+        (WebCore::RenderScrollbar::~RenderScrollbar):
+
 2011-09-13  Adam Klein  <ad...@chromium.org>
 
         Fix cssText property of counter-valued CSSPrimitiveValue and avoid uninitialized read

Modified: trunk/Source/WebCore/rendering/RenderScrollbar.cpp (95073 => 95074)


--- trunk/Source/WebCore/rendering/RenderScrollbar.cpp	2011-09-14 03:36:09 UTC (rev 95073)
+++ trunk/Source/WebCore/rendering/RenderScrollbar.cpp	2011-09-14 04:19:45 UTC (rev 95074)
@@ -64,7 +64,15 @@
 
 RenderScrollbar::~RenderScrollbar()
 {
-    ASSERT(m_parts.isEmpty());
+    if (!m_parts.isEmpty()) {
+        // When a scrollbar is detached from its parent (causing all parts removal) and 
+        // ready to be destroyed, its destruction can be delayed because of RefPtr
+        // maintained in other classes such as EventHandler (m_lastScrollbarUnderMouse).
+        // Meanwhile, we can have a call to updateScrollbarPart which recreates the 
+        // scrollbar part. So, we need to destroy these parts since we don't want them
+        // to call on a destroyed scrollbar. See webkit bug 68009.
+        updateScrollbarParts(true);
+    }
 }
 
 RenderBox* RenderScrollbar::owningRenderer() const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to