Title: [183614] trunk
Revision
183614
Author
[email protected]
Date
2015-04-29 21:15:11 -0700 (Wed, 29 Apr 2015)

Log Message

Crash at WebCore::Document::absoluteRegionForEventTargets
https://bugs.webkit.org/show_bug.cgi?id=144426
rdar://problem/20502166

Reviewed by Tim Horton.

Source/WebCore:

When a frame had wheel event handlers, we would register the document itself
as a handler in its parent document. This is problematic, because there's not
code path that removes it when the frame is destroyed.

It turns out we don't need to do this at all; the non-fast scrollable region
already takes handlers in subframes into account.

Tests: fast/events/wheelevent-in-frame.html
       fast/events/wheelevent-in-reattached-frame.html

* dom/Document.cpp:
(WebCore::Document::didAddWheelEventHandler):
(WebCore::Document::didRemoveWheelEventHandler):

LayoutTests:

Test that disconnects a frame with a wheel event handler then GCs, and one that
disconnects are reconnects. In both case, the parent document should have zero
wheel event handlers registered on it.

* fast/events/wheelevent-in-frame-expected.txt: Added.
* fast/events/wheelevent-in-frame.html: Added.
* fast/events/wheelevent-in-reattached-frame-expected.txt: Added.
* fast/events/wheelevent-in-reattached-frame.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (183613 => 183614)


--- trunk/LayoutTests/ChangeLog	2015-04-30 03:40:16 UTC (rev 183613)
+++ trunk/LayoutTests/ChangeLog	2015-04-30 04:15:11 UTC (rev 183614)
@@ -1,3 +1,20 @@
+2015-04-29  Simon Fraser  <[email protected]>
+
+        Crash at WebCore::Document::absoluteRegionForEventTargets 
+        https://bugs.webkit.org/show_bug.cgi?id=144426
+        rdar://problem/20502166
+
+        Reviewed by Tim Horton.
+
+        Test that disconnects a frame with a wheel event handler then GCs, and one that
+        disconnects are reconnects. In both case, the parent document should have zero
+        wheel event handlers registered on it.
+
+        * fast/events/wheelevent-in-frame-expected.txt: Added.
+        * fast/events/wheelevent-in-frame.html: Added.
+        * fast/events/wheelevent-in-reattached-frame-expected.txt: Added.
+        * fast/events/wheelevent-in-reattached-frame.html: Added.
+
 2015-04-29  Joseph Pecoraro  <[email protected]>
 
         LiveNodeList may unexpectedly return an element for empty string

Added: trunk/LayoutTests/fast/events/resources/wheel-event-handler-on-document.html (0 => 183614)


--- trunk/LayoutTests/fast/events/resources/wheel-event-handler-on-document.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/resources/wheel-event-handler-on-document.html	2015-04-30 04:15:11 UTC (rev 183614)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+</head>
+<body>
+
+<script>
+window.addEventListener('mousewheel', function() {}, false);
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/fast/events/wheelevent-in-frame-expected.txt (0 => 183614)


--- trunk/LayoutTests/fast/events/wheelevent-in-frame-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-frame-expected.txt	2015-04-30 04:15:11 UTC (rev 183614)
@@ -0,0 +1,29 @@
+Tests that detaching a frame with a wheel event handlers doesn't crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/events/wheelevent-in-frame.html (0 => 183614)


--- trunk/LayoutTests/fast/events/wheelevent-in-frame.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-frame.html	2015-04-30 04:15:11 UTC (rev 183614)
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    </style>
+    <script src=""
+    <script>
+    </script>
+</head>
+<body>
+
+    <script>
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    description("Tests that detaching a frame with a wheel event handlers doesn't crash.");
+    
+    const maxLoads = 10;
+    var loadCount = 0;
+
+    function makeFrame()
+    {
+        var frame = document.createElement('iframe');
+        frame.addEventListener('load', function() {
+            if (window.internals)
+                shouldBe("internals.wheelEventHandlerCount()", "0");
+
+            frame.remove();
+            window.setTimeout(checkFrameRemoved, 0);
+        });
+
+        frame.src = '';
+        addFrameToDocument(frame);
+    }
+    
+    function checkFrameRemoved()
+    {
+        gc();
+
+        if (window.internals)
+            shouldBe("internals.wheelEventHandlerCount()", "0");
+
+        if (++loadCount == maxLoads) {
+            isSuccessfullyParsed();
+            if (window.testRunner)
+                testRunner.notifyDone();
+
+            return;
+        }
+
+        window.setTimeout(makeFrame, 0);
+    }
+
+    function addFrameToDocument(frame)
+    {
+        document.body.appendChild(frame);
+    }
+    
+    makeFrame();
+    </script>
+</body>
+</html>

Added: trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt (0 => 183614)


--- trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt	2015-04-30 04:15:11 UTC (rev 183614)
@@ -0,0 +1,29 @@
+Tests that detaching and reattaching a frame with a wheel event handlers doesn't crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame.html (0 => 183614)


--- trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame.html	2015-04-30 04:15:11 UTC (rev 183614)
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    </style>
+    <script src=""
+    <script>
+    </script>
+</head>
+<body>
+
+    <script>
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    description("Tests that detaching and reattaching a frame with a wheel event handlers doesn't crash.");
+    
+    const maxLoads = 10;
+    var loadCount = 0;
+
+    var frame;
+    function makeFrame()
+    {
+        frame = document.createElement('iframe');
+        frame.addEventListener('load', function() {
+            if (window.internals)
+                shouldBe("internals.wheelEventHandlerCount()", "0");
+
+            frame.remove();
+            window.setTimeout(checkFrameRemoved, 0);
+        });
+
+        frame.src = '';
+        addFrameToDocument(frame);
+    }
+    
+    function checkFrameRemoved()
+    {
+        gc();
+
+        if (window.internals)
+            shouldBe("internals.wheelEventHandlerCount()", "0");
+
+        if (++loadCount == maxLoads) {
+            isSuccessfullyParsed();
+            if (window.testRunner)
+                testRunner.notifyDone();
+
+            return;
+        }
+
+        window.setTimeout(function() {
+            addFrameToDocument(frame);
+        }, 0);
+    }
+
+    function addFrameToDocument(frame)
+    {
+        document.body.appendChild(frame);
+    }
+    
+    makeFrame();
+    </script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (183613 => 183614)


--- trunk/Source/WebCore/ChangeLog	2015-04-30 03:40:16 UTC (rev 183613)
+++ trunk/Source/WebCore/ChangeLog	2015-04-30 04:15:11 UTC (rev 183614)
@@ -1,3 +1,25 @@
+2015-04-29  Simon Fraser  <[email protected]>
+
+        Crash at WebCore::Document::absoluteRegionForEventTargets 
+        https://bugs.webkit.org/show_bug.cgi?id=144426
+        rdar://problem/20502166
+
+        Reviewed by Tim Horton.
+
+        When a frame had wheel event handlers, we would register the document itself
+        as a handler in its parent document. This is problematic, because there's not
+        code path that removes it when the frame is destroyed.
+        
+        It turns out we don't need to do this at all; the non-fast scrollable region
+        already takes handlers in subframes into account.
+
+        Tests: fast/events/wheelevent-in-frame.html
+               fast/events/wheelevent-in-reattached-frame.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::didAddWheelEventHandler):
+        (WebCore::Document::didRemoveWheelEventHandler):
+
 2015-04-29  Eric Carlson  <[email protected]>
 
         Not all videos should automatically play to playback target

Modified: trunk/Source/WebCore/dom/Document.cpp (183613 => 183614)


--- trunk/Source/WebCore/dom/Document.cpp	2015-04-30 03:40:16 UTC (rev 183613)
+++ trunk/Source/WebCore/dom/Document.cpp	2015-04-30 04:15:11 UTC (rev 183614)
@@ -5949,11 +5949,6 @@
 
     m_wheelEventTargets->add(&node);
 
-    if (Document* parent = parentDocument()) {
-        parent->didAddWheelEventHandler(*this);
-        return;
-    }
-
     wheelEventHandlersChanged();
 
     if (Frame* frame = this->frame())
@@ -5979,11 +5974,6 @@
     if (!removeHandlerFromSet(*m_wheelEventTargets, node, removal))
         return;
 
-    if (Document* parent = parentDocument()) {
-        parent->didRemoveWheelEventHandler(*this);
-        return;
-    }
-
     wheelEventHandlersChanged();
 
     if (Frame* frame = this->frame())
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to