Diff
Modified: trunk/LayoutTests/ChangeLog (199180 => 199181)
--- trunk/LayoutTests/ChangeLog 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/LayoutTests/ChangeLog 2016-04-07 21:15:34 UTC (rev 199181)
@@ -1,3 +1,15 @@
+2016-04-07 Brent Fulgham <[email protected]>
+
+ Wheel event callback removing the window causes crash in WebCore.
+ https://bugs.webkit.org/show_bug.cgi?id=150871
+ <rdar://problem/23418283>
+
+ Reviewed by Simon Fraser.
+
+ * fast/events/wheel-event-destroys-frame-expected.txt: Added.
+ * fast/events/wheel-event-destroys-frame.html: Added.
+ * platform/ios-simulator/TestExpectations: Skip wheel-event test on iOS.
+
2016-04-07 Saam barati <[email protected]>
Initial implementation of annex b.3.3 behavior was incorrect
Added: trunk/LayoutTests/fast/events/wheel-event-destroys-frame-expected.txt (0 => 199181)
--- trunk/LayoutTests/fast/events/wheel-event-destroys-frame-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/wheel-event-destroys-frame-expected.txt 2016-04-07 21:15:34 UTC (rev 199181)
@@ -0,0 +1,3 @@
+This test should not crash
+
+
Added: trunk/LayoutTests/fast/events/wheel-event-destroys-frame.html (0 => 199181)
--- trunk/LayoutTests/fast/events/wheel-event-destroys-frame.html (rev 0)
+++ trunk/LayoutTests/fast/events/wheel-event-destroys-frame.html 2016-04-07 21:15:34 UTC (rev 199181)
@@ -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="" here</body>"></iframe>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios-simulator/TestExpectations (199180 => 199181)
--- trunk/LayoutTests/platform/ios-simulator/TestExpectations 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/LayoutTests/platform/ios-simulator/TestExpectations 2016-04-07 21:15:34 UTC (rev 199181)
@@ -57,6 +57,7 @@
# No mousewheel events on iOS
fast/scrolling/iframe-scrollable-after-back.html [ Skip ]
fast/scrolling/overflow-scrollable-after-back.html [ Skip ]
+fast/events/wheel-event-destroys-frame.html [ Skip ]
# Not supported on iOS
batterystatus
Modified: trunk/Source/WebCore/ChangeLog (199180 => 199181)
--- trunk/Source/WebCore/ChangeLog 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/ChangeLog 2016-04-07 21:15:34 UTC (rev 199181)
@@ -1,3 +1,52 @@
+2016-04-07 Brent Fulgham <[email protected]>
+
+ Wheel event callback removing the window causes crash in WebCore.
+ https://bugs.webkit.org/show_bug.cgi?id=150871
+ <rdar://problem/23418283>
+
+ Reviewed by Simon Fraser.
+
+ 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.
+
+ Since the Frame destructor invokes EventHandler::clear, which invokes MainFrame methods,
+ we run the risk of attempting to dereference destroyed MainFrame elements of the current
+ Frame object. Instead, clear the EventHandler in the MainFrame destructor.
+
+ Finally, confirm that the mainFrame member is not being destroyed in the handful of
+ places that might attempt to access the mainFrame during object destruction (essentially
+ cleanup methods).
+
+ Test: fast/events/wheel-event-destroys-frame.html
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::clear): Protect against accessing mainFrame content during destruction.
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::clear): Call 'clearLatchedState' instead of endFilteringDeltas.
+ (WebCore::EventHandler::clearLatchedState): Null-check the filter before calling it.
+ * page/Frame.cpp:
+ (WebCore::Frame::~Frame): Do not call 'setView' in the destructor for a MainFrame.
+ (WebCore::Frame::setView): Check for a null event handler before invoking it.
+ (WebCore::Frame::setMainFrameWasDestroyed): Added. Mark that the MainFrame
+ member of the Frame is being destroyed (if the current Frame is a MainFrame) and clear
+ the EventHandler member so that it doesn't attempt to access mainFrame content.
+ (WebCore::Frame::mainFrame): When accessing the mainFrame member, assert that the
+ mainFrame is not being destroyed.
+ * page/MainFrame.cpp:
+ (WebCore::MainFrame::~MainFrame): Set the m_recentWheelEventDeltaFilter to nullptr to
+ prevent attempts to access it during object destruction. Call the new 'setMainFrameWasDestroyed'
+ method to reset eventHandler and mark the MainFrame as being in the process of destruction.
+ * page/WheelEventDeltaFilter.cpp:
+ (WebCore::WheelEventDeltaFilter::filteredDelta): Add logging.
+ * page/mac/EventHandlerMac.mm:
+ (WebCore::EventHandler::platformCompleteWheelEvent): Add null check.
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::scrollTo): Add logging.
+
2016-04-05 Ada Chan <[email protected]>
Rename TextTrackRepresentationiOS to TextTrackRepresentationCocoa and enable on Mac
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (199180 => 199181)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2016-04-07 21:15:34 UTC (rev 199181)
@@ -604,7 +604,11 @@
}
m_frame.selection().prepareForDestruction();
- m_frame.eventHandler().clear();
+
+ // We may call this code during object destruction, so need to make sure eventHandler is present.
+ if (auto eventHandler = m_frame.eventHandlerPtr())
+ eventHandler->clear();
+
if (clearFrameView && m_frame.view())
m_frame.view()->clear();
Modified: trunk/Source/WebCore/page/EventHandler.cpp (199180 => 199181)
--- trunk/Source/WebCore/page/EventHandler.cpp 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2016-04-07 21:15:34 UTC (rev 199181)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
* Copyright (C) 2006 Alexey Proskuryakov ([email protected])
* Copyright (C) 2012 Digia Plc. and/or its subsidiary(-ies)
*
@@ -455,9 +455,7 @@
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;
@@ -2693,7 +2691,8 @@
#if PLATFORM(MAC)
m_frame.mainFrame().resetLatchingState();
#endif
- m_frame.mainFrame().wheelEventDeltaFilter()->endFilteringDeltas();
+ if (auto filter = m_frame.mainFrame().wheelEventDeltaFilter())
+ filter->endFilteringDeltas();
}
void EventHandler::defaultWheelEventHandler(Node* startNode, WheelEvent* wheelEvent)
Modified: trunk/Source/WebCore/page/Frame.cpp (199180 => 199181)
--- trunk/Source/WebCore/page/Frame.cpp 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/page/Frame.cpp 2016-04-07 21:15:34 UTC (rev 199181)
@@ -5,7 +5,7 @@
* 2000 Simon Hausmann <[email protected]>
* 2000 Stefan Schimanski <[email protected]>
* 2001 George Staikos <[email protected]>
- * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2016 Apple Inc. All rights reserved.
* Copyright (C) 2005 Alexey Proskuryakov <[email protected]>
* Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
* Copyright (C) 2008 Eric Seidel <[email protected]>
@@ -160,7 +160,6 @@
, m_script(std::make_unique<ScriptController>(*this))
, m_editor(std::make_unique<Editor>(*this))
, m_selection(std::make_unique<FrameSelection>(this))
- , m_eventHandler(std::make_unique<EventHandler>(*this))
, m_animationController(std::make_unique<AnimationController>(*this))
#if PLATFORM(IOS)
, m_overflowAutoScrollTimer(*this, &Frame::overflowAutoScrollTimerFired)
@@ -169,6 +168,7 @@
, m_pageZoomFactor(parentPageZoomFactor(this))
, m_textZoomFactor(parentTextZoomFactor(this))
, m_activeDOMObjectsAndAnimationsSuspendedCount(0)
+ , m_eventHandler(std::make_unique<EventHandler>(*this))
{
AtomicString::init();
HTMLNames::init();
@@ -251,7 +251,9 @@
if (m_view)
m_view->unscheduleRelayout();
- eventHandler().clear();
+ // This may be called during destruction, so need to do a null check.
+ if (m_eventHandler)
+ m_eventHandler->clear();
m_view = WTFMove(view);
Modified: trunk/Source/WebCore/page/Frame.h (199180 => 199181)
--- trunk/Source/WebCore/page/Frame.h 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/page/Frame.h 2016-04-07 21:15:34 UTC (rev 199181)
@@ -5,7 +5,7 @@
* 2000-2001 Simon Hausmann <[email protected]>
* 2000-2001 Dirk Mueller <[email protected]>
* 2000 Stefan Schimanski <[email protected]>
- * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2016 Apple Inc. All rights reserved.
* Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
* Copyright (C) 2008 Eric Seidel <[email protected]>
*
@@ -151,6 +151,7 @@
Editor& editor() const;
EventHandler& eventHandler() const;
+ EventHandler* eventHandlerPtr() const;
FrameLoader& loader() const;
NavigationScheduler& navigationScheduler() const;
FrameSelection& selection() const;
@@ -278,6 +279,7 @@
protected:
Frame(Page&, HTMLFrameOwnerElement*, FrameLoaderClient&);
+ void setMainFrameWasDestroyed();
private:
HashSet<FrameDestructionObserver*> m_destructionObservers;
@@ -296,7 +298,6 @@
const std::unique_ptr<ScriptController> m_script;
const std::unique_ptr<Editor> m_editor;
const std::unique_ptr<FrameSelection> m_selection;
- const std::unique_ptr<EventHandler> m_eventHandler;
const std::unique_ptr<AnimationController> m_animationController;
#if ENABLE(DATA_DETECTION)
@@ -326,6 +327,10 @@
float m_textZoomFactor;
int m_activeDOMObjectsAndAnimationsSuspendedCount;
+ bool m_mainFrameWasDestroyed { false };
+
+ protected:
+ std::unique_ptr<EventHandler> m_eventHandler;
};
inline void Frame::init()
@@ -398,11 +403,22 @@
return *m_eventHandler;
}
+ inline EventHandler* Frame::eventHandlerPtr() const
+ {
+ return m_eventHandler.get();
+ }
+
inline MainFrame& Frame::mainFrame() const
{
+ ASSERT_WITH_SECURITY_IMPLICATION(!m_mainFrameWasDestroyed);
return m_mainFrame;
}
+ inline void Frame::setMainFrameWasDestroyed()
+ {
+ m_mainFrameWasDestroyed = false;
+ }
+
} // namespace WebCore
#endif // Frame_h
Modified: trunk/Source/WebCore/page/MainFrame.cpp (199180 => 199181)
--- trunk/Source/WebCore/page/MainFrame.cpp 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/page/MainFrame.cpp 2016-04-07 21:15:34 UTC (rev 199181)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -66,6 +66,11 @@
{
if (m_diagnosticLoggingClient)
m_diagnosticLoggingClient->mainFrameDestroyed();
+
+ m_recentWheelEventDeltaFilter = nullptr;
+ m_eventHandler = nullptr;
+
+ setMainFrameWasDestroyed();
}
Ref<MainFrame> MainFrame::create(Page& page, PageConfiguration& configuration)
Modified: trunk/Source/WebCore/page/WheelEventDeltaFilter.cpp (199180 => 199181)
--- trunk/Source/WebCore/page/WheelEventDeltaFilter.cpp 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/page/WheelEventDeltaFilter.cpp 2016-04-07 21:15:34 UTC (rev 199181)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -31,6 +31,8 @@
#endif
#include "FloatSize.h"
+#include "Logging.h"
+#include "TextStream.h"
namespace WebCore {
@@ -58,6 +60,7 @@
FloatSize WheelEventDeltaFilter::filteredDelta() const
{
+ LOG_WITH_STREAM(Scrolling, stream << "BasicWheelEventDeltaFilter::filteredDelta returning " << m_currentFilteredDelta);
return m_currentFilteredDelta;
}
Modified: trunk/Source/WebCore/page/mac/EventHandlerMac.mm (199180 => 199181)
--- trunk/Source/WebCore/page/mac/EventHandlerMac.mm 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/page/mac/EventHandlerMac.mm 2016-04-07 21:15:34 UTC (rev 199181)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006, 2007, 2008, 2009, 2014-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -1049,9 +1049,10 @@
bool EventHandler::platformCompleteWheelEvent(const PlatformWheelEvent& wheelEvent, ContainerNode* scrollableContainer, ScrollableArea* scrollableArea)
{
+ 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.
- ASSERT(m_frame.view());
- FrameView* view = m_frame.view();
+ if (!view)
+ return false;
ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();
if (wheelEvent.useLatchedEventElement() && !latchingIsLockedToAncestorOfThisFrame(m_frame) && latchingState && latchingState->scrollableContainer()) {
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (199180 => 199181)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2016-04-07 21:08:40 UTC (rev 199180)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2016-04-07 21:15:34 UTC (rev 199181)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
*
* Portions are Copyright (C) 1998 Netscape Communications Corporation.
*
@@ -2346,6 +2346,8 @@
if (!box)
return;
+ LOG_WITH_STREAM(Scrolling, stream << "RenderLayer::scrollTo " << position);
+
ScrollPosition newPosition = position;
if (!box->isHTMLMarquee()) {
// Ensure that the dimensions will be computed if they need to be (for overflow:hidden blocks).
