Wheel event callback removing the window causes crash in WebCore.
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 22:59:25 +0000 (22:59 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 22:59:25 +0000 (22:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150871

Reviewed by Brent Fulgham.

Source/WebCore:

Null check the FrameView before using it, since the iframe may have been removed
from its parent document inside the event handler.

The new test triggered a cross-load side-effect, where wheel event filtering wasn't
reset between page loads. Fix by calling clearLatchedState() in EventHandler::clear(),
which resets the filtering.

Test: fast/events/wheel-event-destroys-frame.html

* page/EventHandler.cpp:
(WebCore::EventHandler::clear):
* page/WheelEventDeltaFilter.cpp:
(WebCore::WheelEventDeltaFilter::filteredDelta):
* page/mac/EventHandlerMac.mm:
(WebCore::EventHandler::platformCompleteWheelEvent):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollTo):

LayoutTests:

* fast/events/wheel-event-destroys-frame-expected.txt: Added.
* fast/events/wheel-event-destroys-frame.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/wheel-event-destroys-frame-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/wheel-event-destroys-frame.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/EventHandler.cpp
Source/WebCore/page/WheelEventDeltaFilter.cpp
Source/WebCore/page/mac/EventHandlerMac.mm
Source/WebCore/rendering/RenderLayer.cpp

index ce5f34f..b880fc2 100644 (file)
@@ -1,3 +1,13 @@
+2016-02-19  Simon Fraser  <simon.fraser@apple.com>
+
+        Wheel event callback removing the window causes crash in WebCore.
+        https://bugs.webkit.org/show_bug.cgi?id=150871
+
+        Reviewed by Brent Fulgham.
+
+        * fast/events/wheel-event-destroys-frame-expected.txt: Added.
+        * fast/events/wheel-event-destroys-frame.html: Added.
+
 2016-02-19  Antti Koivisto  <antti@apple.com>
 
         ComposedTreeIterator traverses normal children for elements with empty shadow root
diff --git a/LayoutTests/fast/events/wheel-event-destroys-frame-expected.txt b/LayoutTests/fast/events/wheel-event-destroys-frame-expected.txt
new file mode 100644 (file)
index 0000000..6fb47db
--- /dev/null
@@ -0,0 +1,3 @@
+This test should not crash
+
+
diff --git a/LayoutTests/fast/events/wheel-event-destroys-frame.html b/LayoutTests/fast/events/wheel-event-destroys-frame.html
new file mode 100644 (file)
index 0000000..4e67f07
--- /dev/null
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script>
+        if (window.testRunner) {
+            testRunner.waitUntilDone();
+            testRunner.dumpAsText();
+        }
+
+        function frameLoaded(iframe)
+        {
+            iframe.contentWindow.addEventListener('wheel', function() {
+                // Removing the window during event firing causes crash.
+                window.document.body.removeChild(iframe);
+                window.setTimeout(function() {
+                    if (window.testRunner)
+                        testRunner.notifyDone();
+                }, 0);
+            });
+
+            if (!window.eventSender)
+                return;
+
+            var iframeTarget = document.getElementById('iframe');
+            var iframeBounds = iframeTarget.getBoundingClientRect();
+
+            eventSender.mouseMoveTo(iframeBounds.left + 10, iframeBounds.top + 10);
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+        }
+    </script>
+</head>
+<body>
+    <p>This test should not crash</p>
+    <iframe id="iframe" onload="frameLoaded(this)" src="data:text/html,<body>Wheel here</body>"></iframe>
+</body>
+</html>
index de62eae..e9b24e8 100644 (file)
@@ -1,3 +1,28 @@
+2016-02-19  Simon Fraser  <simon.fraser@apple.com>
+
+        Wheel event callback removing the window causes crash in WebCore.
+        https://bugs.webkit.org/show_bug.cgi?id=150871
+
+        Reviewed by Brent Fulgham.
+
+        Null check the FrameView before using it, since the iframe may have been removed
+        from its parent document inside the event handler.
+        
+        The new test triggered a cross-load side-effect, where wheel event filtering wasn't
+        reset between page loads. Fix by calling clearLatchedState() in EventHandler::clear(),
+        which resets the filtering.
+
+        Test: fast/events/wheel-event-destroys-frame.html
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::clear):
+        * page/WheelEventDeltaFilter.cpp:
+        (WebCore::WheelEventDeltaFilter::filteredDelta):
+        * page/mac/EventHandlerMac.mm:
+        (WebCore::EventHandler::platformCompleteWheelEvent):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollTo):
+
 2016-02-19  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [Win] [SVG -> OTF Converter] All uses of a font except the first one are invisible
index a1e58e3..4d5f620 100644 (file)
@@ -452,9 +452,7 @@ void EventHandler::clear()
     m_mousePressed = false;
     m_capturesDragging = false;
     m_capturingMouseEventsElement = nullptr;
-#if PLATFORM(MAC)
-    m_frame.mainFrame().resetLatchingState();
-#endif
+    clearLatchedState();
 #if ENABLE(TOUCH_EVENTS) && !ENABLE(IOS_TOUCH_EVENTS)
     m_originatingTouchPointTargets.clear();
     m_originatingTouchPointDocument = nullptr;
@@ -2664,7 +2662,8 @@ void EventHandler::clearLatchedState()
 #if PLATFORM(MAC)
     m_frame.mainFrame().resetLatchingState();
 #endif
-    m_frame.mainFrame().wheelEventDeltaFilter()->endFilteringDeltas();
+    if (WheelEventDeltaFilter* filter = m_frame.mainFrame().wheelEventDeltaFilter())
+        filter->endFilteringDeltas();
 }
 
 void EventHandler::defaultWheelEventHandler(Node* startNode, WheelEvent* wheelEvent)
index 597b666..229ab21 100644 (file)
@@ -31,6 +31,8 @@
 #endif
 
 #include "FloatSize.h"
+#include "Logging.h"
+#include "TextStream.h"
 
 namespace WebCore {
     
@@ -58,6 +60,7 @@ bool WheelEventDeltaFilter::isFilteringDeltas() const
 
 FloatSize WheelEventDeltaFilter::filteredDelta() const
 {
+    LOG_WITH_STREAM(Scrolling, stream << "BasicWheelEventDeltaFilter::filteredDelta returning " << m_currentFilteredDelta);
     return m_currentFilteredDelta;
 }
 
index 73ee3c0..e8f96a5 100644 (file)
@@ -1008,9 +1008,10 @@ static FrameView* frameViewForLatchingState(Frame& frame, ScrollLatchingState* l
 
 bool EventHandler::platformCompleteWheelEvent(const PlatformWheelEvent& wheelEvent, ContainerNode* scrollableContainer, ScrollableArea* scrollableArea)
 {
-    // We do another check on the frame view because the event handler can run JS which results in the frame getting destroyed.
-    ASSERT(m_frame.view());
     FrameView* view = m_frame.view();
+    // We do another check on the frame view because the event handler can run JS which results in the frame getting destroyed.
+    if (!view)
+        return false;
 
     ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();
     if (wheelEvent.useLatchedEventElement() && !latchingIsLockedToAncestorOfThisFrame(m_frame) && latchingState && latchingState->scrollableContainer()) {
index 2da5622..dfcbf67 100644 (file)
@@ -2349,6 +2349,8 @@ void RenderLayer::scrollTo(const ScrollPosition& position)
     if (!box)
         return;
 
+    LOG_WITH_STREAM(Scrolling, stream << "RenderLayer::scrollTo " << position);
+
     ScrollPosition newPosition = position;
     if (box->style().overflowX() != OMARQUEE) {
         // Ensure that the dimensions will be computed if they need to be (for overflow:hidden blocks).