Changeset 183614 in webkit

Timestamp:

Apr 29, 2015 9:15:11 PM (9 years ago)

Author:

Simon Fraser

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/resources/wheel-event-handler-on-document.html (added)
• LayoutTests/fast/events/wheelevent-in-frame-expected.txt (added)
• LayoutTests/fast/events/wheelevent-in-frame.html (added)
• LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt (added)
• LayoutTests/fast/events/wheelevent-in-reattached-frame.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -5950 +5950 @@
     m_wheelEventTargets->add(&node);
 
-    if (Document* parent = parentDocument()) {
-        parent->didAddWheelEventHandler(*this);
-        return;
-    }
-
     wheelEventHandlersChanged();
 
@@ -5979 +5974 @@
     if (!removeHandlerFromSet(*m_wheelEventTargets, node, removal))
         return;
-
-    if (Document* parent = parentDocument()) {
-        parent->didRemoveWheelEventHandler(*this);
-        return;
-    }
 
     wheelEventHandlersChanged();