From 444a266459136548f432c6d2ef1c8cd079ee1063 Mon Sep 17 00:00:00 2001 From: "simon.fraser@apple.com" Date: Thu, 30 Apr 2015 04:15:11 +0000 Subject: [PATCH] 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. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183614 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 17 ++++++ .../resources/wheel-event-handler-on-document.html | 12 ++++ .../fast/events/wheelevent-in-frame-expected.txt | 29 ++++++++++ LayoutTests/fast/events/wheelevent-in-frame.html | 63 +++++++++++++++++++++ .../wheelevent-in-reattached-frame-expected.txt | 29 ++++++++++ .../events/wheelevent-in-reattached-frame.html | 66 ++++++++++++++++++++++ Source/WebCore/ChangeLog | 22 ++++++++ Source/WebCore/dom/Document.cpp | 10 ---- 8 files changed, 238 insertions(+), 10 deletions(-) create mode 100644 LayoutTests/fast/events/resources/wheel-event-handler-on-document.html create mode 100644 LayoutTests/fast/events/wheelevent-in-frame-expected.txt create mode 100644 LayoutTests/fast/events/wheelevent-in-frame.html create mode 100644 LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt create mode 100644 LayoutTests/fast/events/wheelevent-in-reattached-frame.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index fa4733a..6db79ac 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,20 @@ +2015-04-29 Simon Fraser + + 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 LiveNodeList may unexpectedly return an element for empty string diff --git a/LayoutTests/fast/events/resources/wheel-event-handler-on-document.html b/LayoutTests/fast/events/resources/wheel-event-handler-on-document.html new file mode 100644 index 0000000..0b4e6f8 --- /dev/null +++ b/LayoutTests/fast/events/resources/wheel-event-handler-on-document.html @@ -0,0 +1,12 @@ + + + + + + + + + + diff --git a/LayoutTests/fast/events/wheelevent-in-frame-expected.txt b/LayoutTests/fast/events/wheelevent-in-frame-expected.txt new file mode 100644 index 0000000..96d05f4 --- /dev/null +++ b/LayoutTests/fast/events/wheelevent-in-frame-expected.txt @@ -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 index 0000000..8788ac3 --- /dev/null +++ b/LayoutTests/fast/events/wheelevent-in-frame.html @@ -0,0 +1,63 @@ + + + + + + + + + + + + + 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 index 0000000..462ae3a --- /dev/null +++ b/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt @@ -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 index 0000000..795cf90 --- /dev/null +++ b/LayoutTests/fast/events/wheelevent-in-reattached-frame.html @@ -0,0 +1,66 @@ + + + + + + + + + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index cdd6143..0390a4b 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,25 @@ +2015-04-29 Simon Fraser + + 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 Not all videos should automatically play to playback target diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp index 5ca8a43..5dae186 100644 --- a/Source/WebCore/dom/Document.cpp +++ b/Source/WebCore/dom/Document.cpp @@ -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()) -- 1.8.3.1