Crash at WebCore::Document::absoluteRegionForEventTargets
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Apr 2015 01:38:20 +0000 (01:38 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Apr 2015 01:38:20 +0000 (01:38 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183600 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/wheelevent-in-frame-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/wheelevent-in-frame.html [new file with mode: 0644]
LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/wheelevent-in-reattached-frame.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp

index 6aa8cbb..754ece6 100644 (file)
@@ -1,5 +1,22 @@
 2015-04-29  Simon Fraser  <simon.fraser@apple.com>
 
+        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  Simon Fraser  <simon.fraser@apple.com>
+
         Compute the non-fast-scrollable region in main-document coordinates
         https://bugs.webkit.org/show_bug.cgi?id=144420
 
diff --git a/LayoutTests/fast/events/wheelevent-in-frame-expected.txt b/LayoutTests/fast/events/wheelevent-in-frame-expected.txt
new file mode 100644 (file)
index 0000000..96d05f4
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/fast/events/wheelevent-in-frame.html b/LayoutTests/fast/events/wheelevent-in-frame.html
new file mode 100644 (file)
index 0000000..8788ac3
--- /dev/null
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    </style>
+    <script src="../../resources/js-test-pre.js"></script>
+    <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 = 'resources/wheel-event-handler-on-document.html';
+        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>
diff --git a/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt b/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt
new file mode 100644 (file)
index 0000000..462ae3a
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/fast/events/wheelevent-in-reattached-frame.html b/LayoutTests/fast/events/wheelevent-in-reattached-frame.html
new file mode 100644 (file)
index 0000000..209cbb0
--- /dev/null
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    </style>
+    <script src="../../resources/js-test-pre.js"></script>
+    <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 = 'resources/wheel-event-handlers-dynamic.html';
+        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>
index c9e8b8c..0894ce0 100644 (file)
@@ -1,3 +1,25 @@
+2015-04-29  Simon Fraser  <simon.fraser@apple.com>
+
+        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  David Kilzer  <ddkilzer@apple.com>
 
         Attempt #2: Switch QuickLook soft-linking to use QuickLookSoftLink.{h,mm}
index 5ca8a43..5dae186 100644 (file)
@@ -5949,11 +5949,6 @@ void Document::didAddWheelEventHandler(Node& node)
 
     m_wheelEventTargets->add(&node);
 
-    if (Document* parent = parentDocument()) {
-        parent->didAddWheelEventHandler(*this);
-        return;
-    }
-
     wheelEventHandlersChanged();
 
     if (Frame* frame = this->frame())
@@ -5979,11 +5974,6 @@ void Document::didRemoveWheelEventHandler(Node& node, EventHandlerRemoval remova
     if (!removeHandlerFromSet(*m_wheelEventTargets, node, removal))
         return;
 
-    if (Document* parent = parentDocument()) {
-        parent->didRemoveWheelEventHandler(*this);
-        return;
-    }
-
     wheelEventHandlersChanged();
 
     if (Frame* frame = this->frame())