Changeset 199181 in webkit


Ignore:
Timestamp:
Apr 7, 2016 2:15:34 PM (8 years ago)
Author:
Brent Fulgham
Message:

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.

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.

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.

LayoutTests:

  • 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.
Location:
trunk
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r199179 r199181  
     12016-04-07  Brent Fulgham  <bfulgham@apple.com>
     2
     3        Wheel event callback removing the window causes crash in WebCore.
     4        https://bugs.webkit.org/show_bug.cgi?id=150871
     5        <rdar://problem/23418283>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * fast/events/wheel-event-destroys-frame-expected.txt: Added.
     10        * fast/events/wheel-event-destroys-frame.html: Added.
     11        * platform/ios-simulator/TestExpectations: Skip wheel-event test on iOS.
     12
    1132016-04-07  Saam barati  <sbarati@apple.com>
    214

  • trunk/LayoutTests/platform/ios-simulator/TestExpectations

    r199086 r199181  
    5858fast/scrolling/iframe-scrollable-after-back.html [ Skip ]
    5959fast/scrolling/overflow-scrollable-after-back.html [ Skip ]
     60fast/events/wheel-event-destroys-frame.html [ Skip ]
    6061
    6162# Not supported on iOS

  • trunk/Source/WebCore/ChangeLog

    r199174 r199181  
     12016-04-07  Brent Fulgham  <bfulgham@apple.com>
     2
     3        Wheel event callback removing the window causes crash in WebCore.
     4        https://bugs.webkit.org/show_bug.cgi?id=150871
     5        <rdar://problem/23418283>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Null check the FrameView before using it, since the iframe may have been removed
     10        from its parent document inside the event handler.
     11       
     12        The new test triggered a cross-load side-effect, where wheel event filtering wasn't
     13        reset between page loads. Fix by calling clearLatchedState() in EventHandler::clear(),
     14        which resets the filtering.
     15
     16        Since the Frame destructor invokes EventHandler::clear, which invokes MainFrame methods,
     17        we run the risk of attempting to dereference destroyed MainFrame elements of the current
     18        Frame object. Instead, clear the EventHandler in the MainFrame destructor.
     19
     20        Finally, confirm that the mainFrame member is not being destroyed in the handful of
     21        places that might attempt to access the mainFrame during object destruction (essentially
     22        cleanup methods).
     23
     24        Test: fast/events/wheel-event-destroys-frame.html
     25
     26        * loader/FrameLoader.cpp:
     27        (WebCore::FrameLoader::clear): Protect against accessing mainFrame content during destruction.
     28        * page/EventHandler.cpp:
     29        (WebCore::EventHandler::clear): Call 'clearLatchedState' instead of endFilteringDeltas.
     30        (WebCore::EventHandler::clearLatchedState): Null-check the filter before calling it.
     31        * page/Frame.cpp:
     32        (WebCore::Frame::~Frame): Do not call 'setView' in the destructor for a MainFrame.
     33        (WebCore::Frame::setView): Check for a null event handler before invoking it.
     34        (WebCore::Frame::setMainFrameWasDestroyed): Added. Mark that the MainFrame
     35        member of the Frame is being destroyed (if the current Frame is a MainFrame) and clear
     36        the EventHandler member so that it doesn't attempt to access mainFrame content.
     37        (WebCore::Frame::mainFrame): When accessing the mainFrame member, assert that the
     38        mainFrame is not being destroyed.
     39        * page/MainFrame.cpp:
     40        (WebCore::MainFrame::~MainFrame): Set the m_recentWheelEventDeltaFilter to nullptr to
     41        prevent attempts to access it during object destruction. Call the new 'setMainFrameWasDestroyed'
     42        method to reset eventHandler and mark the MainFrame as being in the process of destruction.
     43        * page/WheelEventDeltaFilter.cpp:
     44        (WebCore::WheelEventDeltaFilter::filteredDelta): Add logging.
     45        * page/mac/EventHandlerMac.mm:
     46        (WebCore::EventHandler::platformCompleteWheelEvent): Add null check.
     47        * rendering/RenderLayer.cpp:
     48        (WebCore::RenderLayer::scrollTo): Add logging.
     49
    1502016-04-05  Ada Chan  <adachan@apple.com>
    251

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r198956 r199181  
    605605
    606606    m_frame.selection().prepareForDestruction();
    607     m_frame.eventHandler().clear();
     607
     608    // We may call this code during object destruction, so need to make sure eventHandler is present.
     609    if (auto eventHandler = m_frame.eventHandlerPtr())
     610        eventHandler->clear();
     611
    608612    if (clearFrameView && m_frame.view())
    609613        m_frame.view()->clear();

  • trunk/Source/WebCore/page/EventHandler.cpp

    r198909 r199181  
    11/*
    2  * Copyright (C) 2006-2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
    33 * Copyright (C) 2006 Alexey Proskuryakov (ap@webkit.org)
    44 * Copyright (C) 2012 Digia Plc. and/or its subsidiary(-ies)
    … …  
    456456    m_capturesDragging = false;
    457457    m_capturingMouseEventsElement = nullptr;
    458 #if PLATFORM(MAC)
    459     m_frame.mainFrame().resetLatchingState();
    460 #endif
     458    clearLatchedState();
    461459#if ENABLE(TOUCH_EVENTS) && !ENABLE(IOS_TOUCH_EVENTS)
    462460    m_originatingTouchPointTargets.clear();
    … …  
    26942692    m_frame.mainFrame().resetLatchingState();
    26952693#endif
    2696     m_frame.mainFrame().wheelEventDeltaFilter()->endFilteringDeltas();
     2694    if (auto filter = m_frame.mainFrame().wheelEventDeltaFilter())
     2695        filter->endFilteringDeltas();
    26972696}
    26982697

  • trunk/Source/WebCore/page/Frame.cpp

    r198180 r199181  
    66 *                     2000 Stefan Schimanski <1Stein@gmx.de>
    77 *                     2001 George Staikos <staikos@kde.org>
    8  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
     8 * Copyright (C) 2004-2016 Apple Inc. All rights reserved.
    99 * Copyright (C) 2005 Alexey Proskuryakov <ap@nypop.com>
    1010 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
    … …  
    161161    , m_editor(std::make_unique<Editor>(*this))
    162162    , m_selection(std::make_unique<FrameSelection>(this))
    163     , m_eventHandler(std::make_unique<EventHandler>(*this))
    164163    , m_animationController(std::make_unique<AnimationController>(*this))
    165164#if PLATFORM(IOS)
    … …  
    170169    , m_textZoomFactor(parentTextZoomFactor(this))
    171170    , m_activeDOMObjectsAndAnimationsSuspendedCount(0)
     171    , m_eventHandler(std::make_unique<EventHandler>(*this))
    172172{
    173173    AtomicString::init();
    … …  
    252252        m_view->unscheduleRelayout();
    253253   
    254     eventHandler().clear();
     254    // This may be called during destruction, so need to do a null check.
     255    if (m_eventHandler)
     256        m_eventHandler->clear();
    255257
    256258    m_view = WTFMove(view);

  • trunk/Source/WebCore/page/Frame.h

    r198180 r199181  
    66 *                     2000-2001 Dirk Mueller <mueller@kde.org>
    77 *                     2000 Stefan Schimanski <1Stein@gmx.de>
    8  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
     8 * Copyright (C) 2004-2016 Apple Inc. All rights reserved.
    99 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
    1010 * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
    … …  
    152152        Editor& editor() const;
    153153        EventHandler& eventHandler() const;
     154        EventHandler* eventHandlerPtr() const;
    154155        FrameLoader& loader() const;
    155156        NavigationScheduler& navigationScheduler() const;
    … …  
    279280    protected:
    280281        Frame(Page&, HTMLFrameOwnerElement*, FrameLoaderClient&);
     282        void setMainFrameWasDestroyed();
    281283
    282284    private:
    … …  
    297299        const std::unique_ptr<Editor> m_editor;
    298300        const std::unique_ptr<FrameSelection> m_selection;
    299         const std::unique_ptr<EventHandler> m_eventHandler;
    300301        const std::unique_ptr<AnimationController> m_animationController;
    301302
    … …  
    327328
    328329        int m_activeDOMObjectsAndAnimationsSuspendedCount;
     330        bool m_mainFrameWasDestroyed { false };
     331
     332    protected:
     333        std::unique_ptr<EventHandler> m_eventHandler;
    329334    };
    330335
    … …  
    399404    }
    400405
     406    inline EventHandler* Frame::eventHandlerPtr() const
     407    {
     408        return m_eventHandler.get();
     409    }
     410
    401411    inline MainFrame& Frame::mainFrame() const
    402412    {
     413        ASSERT_WITH_SECURITY_IMPLICATION(!m_mainFrameWasDestroyed);
    403414        return m_mainFrame;
    404415    }
    405416
     417    inline void Frame::setMainFrameWasDestroyed()
     418    {
     419        m_mainFrameWasDestroyed = false;
     420    }
     421
    406422} // namespace WebCore
    407423

  • trunk/Source/WebCore/page/MainFrame.cpp

    r195937 r199181  
    11/*
    2  * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    6767    if (m_diagnosticLoggingClient)
    6868        m_diagnosticLoggingClient->mainFrameDestroyed();
     69
     70    m_recentWheelEventDeltaFilter = nullptr;
     71    m_eventHandler = nullptr;
     72
     73    setMainFrameWasDestroyed();
    6974}
    7075

  • trunk/Source/WebCore/page/WheelEventDeltaFilter.cpp

    r196872 r199181  
    11/*
    2  * Copyright (C) 2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3232
    3333#include "FloatSize.h"
     34#include "Logging.h"
     35#include "TextStream.h"
    3436
    3537namespace WebCore {
    … …  
    5961FloatSize WheelEventDeltaFilter::filteredDelta() const
    6062{
     63    LOG_WITH_STREAM(Scrolling, stream << "BasicWheelEventDeltaFilter::filteredDelta returning " << m_currentFilteredDelta);
    6164    return m_currentFilteredDelta;
    6265}

  • trunk/Source/WebCore/page/mac/EventHandlerMac.mm

    r198805 r199181  
    11/*
    2  * Copyright (C) 2006, 2007, 2008, 2009, 2014-2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    10501050bool EventHandler::platformCompleteWheelEvent(const PlatformWheelEvent& wheelEvent, ContainerNode* scrollableContainer, ScrollableArea* scrollableArea)
    10511051{
     1052    FrameView* view = m_frame.view();
    10521053    // We do another check on the frame view because the event handler can run JS which results in the frame getting destroyed.
    1053     ASSERT(m_frame.view());
    1054     FrameView* view = m_frame.view();
     1054    if (!view)
     1055        return false;
    10551056
    10561057    ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r198841 r199181  
    11/*
    2  * Copyright (C) 2006-2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2016 Apple Inc. All rights reserved.
    33 *
    44 * Portions are Copyright (C) 1998 Netscape Communications Corporation.
    … …  
    23472347        return;
    23482348
     2349    LOG_WITH_STREAM(Scrolling, stream << "RenderLayer::scrollTo " << position);
     2350
    23492351    ScrollPosition newPosition = position;
    23502352    if (!box->isHTMLMarquee()) {
Note: See TracChangeset for help on using the changeset viewer.