In WebKit, if a window is removed while it is triggering a wheel event, it will crash on a bad memory access.
This crash is triggered in the following function, on what I believe is assembly corresponding to the delegatesScrolling()
call.
WebCore/page/FrameView.cpp
// ...
bool FrameView::wheelEvent(const PlatformWheelEvent& wheelEvent)
{
// Note that to allow for rubber-band over-scroll behavior, even non-scrollable views
// should handle wheel events.
#if !ENABLE(RUBBER_BANDING)
if (!isScrollable())
return false;
#endif
if (delegatesScrolling()) {
IntSize offset = scrollOffset();
IntPoint oldPosition = scrollPosition();
IntSize newOffset = IntSize(offset.width() - wheelEvent.deltaX(), offset.height() - wheelEvent.deltaY());
if (offset != newOffset) {
ScrollView::scrollTo(newOffset);
scrollPositionChanged(oldPosition, scrollPosition());
didChangeScrollOffset();
}
return true;
}
// We don't allow mouse wheeling to happen in a ScrollView that has had its scrollbars explicitly disabled.
if (!canHaveScrollbars())
return false;
if (platformWidget())
return false;
#if ENABLE(ASYNC_SCROLLING)
if (Page* page = frame().page()) {
if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) {
if (scrollingCoordinator->coordinatesScrollingForFrameView(*this))
return scrollingCoordinator->handleWheelEvent(*this, wheelEvent);
}
}
#endif
return ScrollableArea::handleWheelEvent(wheelEvent);
}
// ...
However, the fault appears to lie with the function calling it, seen below.
WebCore/page/mac/EventHandlerMac.mm
// ...
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();
ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();
if (wheelEvent.useLatchedEventElement() && !latchingIsLockedToAncestorOfThisFrame(m_frame) && latchingState && latchingState->scrollableContainer()) {
m_isHandlingWheelEvent = false;
// WebKit2 code path
if (!frameHasPlatformWidget(m_frame) && !latchingState->startedGestureAtScrollLimit() && scrollableContainer == latchingState->scrollableContainer() && scrollableArea && view != scrollableArea) {
// If we did not start at the scroll limit, do not pass the event on to be handled by enclosing scrollable regions.
return true;
}
if (!latchingState->startedGestureAtScrollLimit())
view = frameViewForLatchingState(m_frame, latchingState);
ASSERT(view);
bool didHandleWheelEvent = view->wheelEvent(wheelEvent);
if (scrollableContainer == latchingState->scrollableContainer()) {
// If we are just starting a scroll event, and have nowhere left to scroll, allow
// the enclosing frame to handle the scroll.
didHandleWheelEvent = !latchingState->startedGestureAtScrollLimit();
}
// If the platform widget is handling the event, we always want to return false.
if (scrollableArea == view && view->platformWidget())
didHandleWheelEvent = false;
return didHandleWheelEvent;
}
bool didHandleEvent = view->wheelEvent(wheelEvent);
m_isHandlingWheelEvent = false;
return didHandleEvent;
}
// ...
There is actually a comment suggesting they know about the issue.
// We do another check on the frame view because the event handler can run JS which results in the frame getting
However their solution is to check that the view
is not a null pointer by using ASSERT
, which does not handle it gracefully, and is removed from release builds.
Interestingly, a similar function in the WebKit source code handles this situation properly, only calling wheelEvent
if the view
is not a null pointer.
WebCore/page/EventHandler.cpp
// ...
bool EventHandler::platformCompleteWheelEvent(const PlatformWheelEvent& event, ContainerNode*, ScrollableArea*)
{
// We do another check on the frame view because the event handler can run JS which results in the frame getting destroyed.
FrameView* view = m_frame.view();
bool didHandleEvent = view ? view->wheelEvent(event) : false;
m_isHandlingWheelEvent = false;
return didHandleEvent;
}
// ...
Currently, this can be seen affecting both the latest Nightly WebKit browser and Apple's Safari, as well as anything that links against the same libraries, such as those using WebView
.
Apparently, these are not the only functions affected by the issue. For example, I ran a test with pywebview, which I believe is using a legacy API for these libraries, and it crashes in the following function.
WebCore/page/mac/EventHandlerMac.mm
// ...
bool EventHandler::platformCompletePlatformWidgetWheelEvent(const PlatformWheelEvent& wheelEvent, const Widget& widget, ContainerNode* scrollableContainer)
{
// WebKit1: Prevent multiple copies of the scrollWheel event from being sent to the NSScrollView widget.
if (frameHasPlatformWidget(m_frame) && widget.isFrameView())
return true;
ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();
if (!latchingState)
return false;
if (wheelEvent.useLatchedEventElement() && latchingState->scrollableContainer() && scrollableContainer == latchingState->scrollableContainer())
return !latchingState->startedGestureAtScrollLimit();
return false;
}
// ...
The crash can be avoided in JavaScript by deferring removal of the window element with setTimeout
. Of course, it should not be possible to cause a crash in the first place.