Changeset 171283 in webkit


Ignore:
Timestamp:
Jul 20, 2014 1:11:57 PM (10 years ago)
Author:
Darin Adler
Message:

Crashes seen in wheel event handling
​https://bugs.webkit.org/show_bug.cgi?id=135102

Reviewed by Beth Dakin.

Speculative fix based on guesses about what could be crashing.
The crash seems to be calling ref on an event target, and my guess is that this
has something to do with latching.

  • page/EventHandler.cpp:

(WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
(WebCore::EventHandler::handleWheelEvent): Refactored a little and made some local
variables use RefPtr instead of raw pointers. Also added some comments.

  • page/EventHandler.h: Changed argument types to RefPtr.
  • page/mac/EventHandlerMac.mm:

(WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
Also added a FIXME.

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r171282 r171283  
     12014-07-20  Darin Adler  <darin@apple.com>
     2
     3        Crashes seen in wheel event handling
     4        https://bugs.webkit.org/show_bug.cgi?id=135102
     5
     6        Reviewed by Beth Dakin.
     7
     8        Speculative fix based on guesses about what could be crashing.
     9        The crash seems to be calling ref on an event target, and my guess is that this
     10        has something to do with latching.
     11
     12        * page/EventHandler.cpp:
     13        (WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
     14        (WebCore::EventHandler::handleWheelEvent): Refactored a little and made some local
     15        variables use RefPtr instead of raw pointers. Also added some comments.
     16
     17        * page/EventHandler.h: Changed argument types to RefPtr.
     18
     19        * page/mac/EventHandlerMac.mm:
     20        (WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
     21        Also added a FIXME.
     22
    1232014-07-20  Simon Fraser  <simon.fraser@apple.com>
    224

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

    r170765 r171283  
    25062506
    25072507#if !PLATFORM(GTK)
     2508
    25082509bool EventHandler::shouldTurnVerticalTicksIntoHorizontal(const HitTestResult&, const PlatformWheelEvent&) const
    25092510{
    25102511    return false;
    25112512}
     2513
    25122514#endif
    25132515
    25142516#if !PLATFORM(MAC)
    2515 void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, Element*&, ContainerNode*&, ScrollableArea*&, bool&)
     2517
     2518void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, RefPtr<Element>&, RefPtr<ContainerNode>&, ScrollableArea*&, bool&)
    25162519{
    25172520}
    … …  
    25362539    return true;
    25372540}
    2538 #endif
    2539 
    2540 bool EventHandler::handleWheelEvent(const PlatformWheelEvent& e)
     2541
     2542#endif
     2543
     2544bool EventHandler::handleWheelEvent(const PlatformWheelEvent& event)
    25412545{
    25422546    Document* document = m_frame.document();
    2543 
    25442547    if (!document->renderView())
    25452548        return false;
    2546    
     2549
    25472550    RefPtr<FrameView> protector(m_frame.view());
    25482551
    … …  
    25532556    m_isHandlingWheelEvent = true;
    25542557    setFrameWasScrolledByUser();
    2555     LayoutPoint vPoint = view->windowToContents(e.position());
    25562558
    25572559    HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::DisallowShadowContent);
    2558     HitTestResult result(vPoint);
     2560    HitTestResult result(view->windowToContents(event.position()));
    25592561    document->renderView()->hitTest(request, result);
    25602562
    2561     Element* element = result.innerElement();
    2562 
     2563    RefPtr<Element> element = result.innerElement();
     2564    RefPtr<ContainerNode> scrollableContainer;
     2565    ScrollableArea* scrollableArea = nullptr;
    25632566    bool isOverWidget = result.isOverWidget();
    2564 
    2565     ContainerNode* scrollableContainer = nullptr;
    2566     ScrollableArea* scrollableArea = nullptr;
    2567     platformPrepareForWheelEvents(e, result, element, scrollableContainer, scrollableArea, isOverWidget);
     2567    platformPrepareForWheelEvents(event, result, element, scrollableContainer, scrollableArea, isOverWidget);
    25682568
    25692569#if PLATFORM(COCOA)
    2570     if (e.phase() == PlatformWheelEventPhaseNone && e.momentumPhase() == PlatformWheelEventPhaseNone) {
    2571 #else
    2572     if (!e.useLatchedEventElement()) {
    2573 #endif
     2570    if (event.phase() == PlatformWheelEventPhaseNone && event.momentumPhase() == PlatformWheelEventPhaseNone)
     2571#endif
     2572    {
    25742573        m_latchedWheelEventElement = nullptr;
    25752574        m_previousWheelScrolledElement = nullptr;
    … …  
    25772576
    25782577    // FIXME: It should not be necessary to do this mutation here.
    2579     // Instead, the handlers should know convert vertical scrolls
    2580     // appropriately.
    2581     PlatformWheelEvent event = e;
    2582     if (m_baseEventType == PlatformEvent::NoType && shouldTurnVerticalTicksIntoHorizontal(result, e))
    2583         event = event.copyTurningVerticalTicksIntoHorizontalTicks();
    2584 
    2585     platformRecordWheelEvent(event);
     2578    // Instead, the handlers should know convert vertical scrolls appropriately.
     2579    PlatformWheelEvent adjustedEvent = event;
     2580    if (m_baseEventType == PlatformEvent::NoType && shouldTurnVerticalTicksIntoHorizontal(result, event))
     2581        adjustedEvent = event.copyTurningVerticalTicksIntoHorizontalTicks();
     2582
     2583    platformRecordWheelEvent(adjustedEvent);
    25862584
    25872585    if (element) {
    2588         // Figure out which view to send the event to.
    2589         RenderElement* target = element->renderer();
    2590        
    2591         if (isOverWidget && target && target->isWidget()) {
    2592             Widget* widget = toRenderWidget(target)->widget();
    2593             if (widget && passWheelEventToWidget(e, widget)) {
    2594                 m_isHandlingWheelEvent = false;
    2595                 if (scrollableArea)
    2596                     scrollableArea->setScrolledProgrammatically(false);
    2597                 if (widget->platformWidget())
    2598                     return platformCompletePlatformWidgetWheelEvent(e, scrollableContainer);
    2599                 return true;
     2586        if (isOverWidget) {
     2587            RenderElement* target = element->renderer();
     2588            if (target && target->isWidget()) {
     2589                Widget* widget = toRenderWidget(target)->widget();
     2590                if (widget && passWheelEventToWidget(event, widget)) {
     2591                    m_isHandlingWheelEvent = false;
     2592                    if (scrollableArea)
     2593                        scrollableArea->setScrolledProgrammatically(false);
     2594                    if (!widget->platformWidget())
     2595                        return true;
     2596                    return platformCompletePlatformWidgetWheelEvent(event, scrollableContainer.get());
     2597                }
    26002598            }
    26012599        }
    26022600
    2603         if (!element->dispatchWheelEvent(event)) {
     2601        if (!element->dispatchWheelEvent(adjustedEvent)) {
    26042602            m_isHandlingWheelEvent = false;
    2605 
    26062603            if (scrollableArea && scrollableArea->isScrolledProgrammatically()) {
    2607                 // Web developer is controlling scrolling. Don't attempt to latch ourselves:
     2604                // Web developer is controlling scrolling, so don't attempt to latch.
    26082605                clearLatchedState();
    26092606                scrollableArea->setScrolledProgrammatically(false);
    26102607            }
    2611 
    26122608            return true;
    26132609        }
    … …  
    26172613        scrollableArea->setScrolledProgrammatically(false);
    26182614
    2619     return platformCompleteWheelEvent(e, element, scrollableContainer, scrollableArea);
     2615    return platformCompleteWheelEvent(event, element.get(), scrollableContainer.get(), scrollableArea);
    26202616}
    26212617

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

    r170765 r171283  
    198198    bool handlePasteGlobalSelection(const PlatformMouseEvent&);
    199199
    200     void platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, Element*& wheelEventTarget, ContainerNode*& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget);
     200    void platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, RefPtr<Element>& eventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*&, bool& isOverWidget);
    201201    void platformRecordWheelEvent(const PlatformWheelEvent&);
    202     bool platformCompleteWheelEvent(const PlatformWheelEvent&, Element* wheelEventTarget, ContainerNode* scrollableContainer, ScrollableArea*);
     202    bool platformCompleteWheelEvent(const PlatformWheelEvent&, Element* eventTarget, ContainerNode* scrollableContainer, ScrollableArea*);
    203203    bool platformCompletePlatformWidgetWheelEvent(const PlatformWheelEvent&, ContainerNode* scrollableContainer);
    204204

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

    r170765 r171283  
    816816}
    817817
    818 void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, Element*& wheelEventTarget, ContainerNode*& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
     818void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, RefPtr<Element>& wheelEventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
    819819{
    820820    FrameView* view = m_frame.view();
    … …  
    826826        scrollableArea = view;
    827827    } else {
    828         if (eventTargetIsPlatformWidget(wheelEventTarget)) {
     828        if (eventTargetIsPlatformWidget(wheelEventTarget.get())) {
    829829            scrollableContainer = wheelEventTarget;
    830             scrollableArea = scrollViewForEventTarget(wheelEventTarget);
     830            scrollableArea = scrollViewForEventTarget(wheelEventTarget.get());
    831831        } else {
    832832            scrollableContainer = findEnclosingScrollableContainer(*wheelEventTarget);
    … …  
    848848            m_startedGestureAtScrollLimit = false;
    849849        m_latchedWheelEventElement = wheelEventTarget;
     850        // FIXME: What prevents us from deleting this scrollable container while still holding a pointer to it?
    850851        m_latchedScrollableContainer = scrollableContainer;
    851852        m_widgetIsLatched = result.isOverWidget();
Note: See TracChangeset for help on using the changeset viewer.