⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 293094 in webkit


Ignore:
Timestamp:
Apr 20, 2022, 8:04:25 AM (4 years ago)
Author:
Simon Fraser
Message:

Some AutoscrollController cleanup
​https://bugs.webkit.org/show_bug.cgi?id=239512

Reviewed by Alan Bujtas.

Have AutoscrollController store a WeakPtr to the render object. Address an apparent
null de-ref in AutoscrollController::stopAutoscrollTimer() where the Frame can null.

Refactor updateDragAndDrop() with a lambda so that all the code paths that exit early
clearly call stopAutoscrollTimer() which nulls out the renderer.

  • page/AutoscrollController.cpp:

(WebCore::AutoscrollController::autoscrollRenderer const):
(WebCore::AutoscrollController::startAutoscrollForSelection):
(WebCore::AutoscrollController::stopAutoscrollTimer):
(WebCore::AutoscrollController::updateAutoscrollRenderer):
(WebCore::AutoscrollController::updateDragAndDrop):
(WebCore::AutoscrollController::startPanScrolling):

  • page/AutoscrollController.h:
  • page/EventHandler.cpp:

(WebCore::EventHandler::startPanScrolling):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r293091 r293094  
     12022-04-20  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Some AutoscrollController cleanup
     4        https://bugs.webkit.org/show_bug.cgi?id=239512
     5
     6        Reviewed by Alan Bujtas.
     7
     8        Have AutoscrollController store a WeakPtr to the render object. Address an apparent
     9        null de-ref in AutoscrollController::stopAutoscrollTimer() where the Frame can null.
     10       
     11        Refactor updateDragAndDrop() with a lambda so that all the code paths that exit early
     12        clearly call stopAutoscrollTimer() which nulls out the renderer.
     13
     14        * page/AutoscrollController.cpp:
     15        (WebCore::AutoscrollController::autoscrollRenderer const):
     16        (WebCore::AutoscrollController::startAutoscrollForSelection):
     17        (WebCore::AutoscrollController::stopAutoscrollTimer):
     18        (WebCore::AutoscrollController::updateAutoscrollRenderer):
     19        (WebCore::AutoscrollController::updateDragAndDrop):
     20        (WebCore::AutoscrollController::startPanScrolling):
     21        * page/AutoscrollController.h:
     22        * page/EventHandler.cpp:
     23        (WebCore::EventHandler::startPanScrolling):
     24
    1252022-04-20  Yacine Bandou  <yacine.bandou@softathome.com>
    226

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

    r288069 r293094  
    5757AutoscrollController::AutoscrollController()
    5858    : m_autoscrollTimer(*this, &AutoscrollController::autoscrollTimerFired)
    59     , m_autoscrollRenderer(nullptr)
    60     , m_autoscrollType(NoAutoscroll)
    6159{
    6260}
    … …  
    6462RenderBox* AutoscrollController::autoscrollRenderer() const
    6563{
    66     return m_autoscrollRenderer;
     64    return m_autoscrollRenderer.get();
    6765}
    6866
    … …  
    7775    if (m_autoscrollTimer.isActive())
    7876        return;
    79     RenderBox* scrollable = RenderBox::findAutoscrollable(renderer);
     77    auto* scrollable = RenderBox::findAutoscrollable(renderer);
    8078    if (!scrollable)
    8179        return;
    8280    m_autoscrollType = AutoscrollForSelection;
    83     m_autoscrollRenderer = scrollable;
     81    m_autoscrollRenderer = WeakPtr { *scrollable };
    8482    startAutoscrollTimer();
    8583}
    … …  
    8785void AutoscrollController::stopAutoscrollTimer(bool rendererIsBeingDestroyed)
    8886{
    89     RenderBox* scrollable = m_autoscrollRenderer;
     87    auto scrollable = m_autoscrollRenderer;
     88
    9089    m_autoscrollTimer.stop();
    9190    m_autoscrollRenderer = nullptr;
    … …  
    9493        return;
    9594
    96     Frame& frame = scrollable->frame();
    97     if (autoscrollInProgress() && frame.eventHandler().mouseDownWasInSubframe()) {
    98         if (auto subframe = frame.eventHandler().subframeForTargetNode(frame.eventHandler().mousePressNode()))
     95    auto* frame = scrollable->document().frame();
     96    if (autoscrollInProgress() && frame && frame->eventHandler().mouseDownWasInSubframe()) {
     97        if (auto subframe = frame->eventHandler().subframeForTargetNode(frame->eventHandler().mousePressNode()))
    9998            subframe->eventHandler().stopAutoscrollTimer(rendererIsBeingDestroyed);
    10099        return;
    … …  
    103102    if (!rendererIsBeingDestroyed)
    104103        scrollable->stopAutoscroll();
     104
    105105#if ENABLE(PAN_SCROLLING)
    106106    if (panScrollInProgress()) {
    … …  
    115115#if ENABLE(PAN_SCROLLING)
    116116    // If we're not in the top frame we notify it that we are not doing a panScroll any more.
    117     if (!frame.isMainFrame())
    118         frame.mainFrame().eventHandler().didPanScrollStop();
     117    if (frame && !frame->isMainFrame())
     118        frame->mainFrame().eventHandler().didPanScrollStop();
    119119#endif
    120120}
    … …  
    125125        return;
    126126
    127     RenderObject* renderer = m_autoscrollRenderer;
     127    RenderObject* renderer = m_autoscrollRenderer.get();
    128128
    129129#if ENABLE(PAN_SCROLLING)
    … …  
    131131    HitTestResult hitTest = m_autoscrollRenderer->frame().eventHandler().hitTestResultAtPoint(m_panScrollStartPos, hitType);
    132132
    133     if (Node* nodeAtPoint = hitTest.innerNode())
     133    if (auto* nodeAtPoint = hitTest.innerNode())
    134134        renderer = nodeAtPoint->renderer();
    135135#endif
    … …  
    137137    while (renderer && !(is<RenderBox>(*renderer) && downcast<RenderBox>(*renderer).canAutoscroll()))
    138138        renderer = renderer->parent();
    139     m_autoscrollRenderer = dynamicDowncast<RenderBox>(renderer);
     139
     140    if (!is<RenderBox>(renderer)) {
     141        m_autoscrollRenderer = nullptr;
     142        return;
     143    }
     144
     145    m_autoscrollRenderer = WeakPtr { downcast<RenderBox>(*renderer) };
    140146}
    141147
    142148void AutoscrollController::updateDragAndDrop(Node* dropTargetNode, const IntPoint& eventPosition, WallTime eventTime)
    143149{
    144     if (!dropTargetNode) {
    145         stopAutoscrollTimer();
    146         return;
    147     }
    148 
    149     RenderBox* scrollable = RenderBox::findAutoscrollable(dropTargetNode->renderer());
     150    IntSize offset;
     151    auto findDragAndDropScroller = [&]() -> RenderBox* {
     152        if (!dropTargetNode)
     153            return nullptr;
     154
     155        auto* scrollable = RenderBox::findAutoscrollable(dropTargetNode->renderer());
     156        if (!scrollable)
     157            return nullptr;
     158
     159        auto& frame = scrollable->frame();
     160        auto* page = frame.page();
     161        if (!page || !page->settings().autoscrollForDragAndDropEnabled())
     162            return nullptr;
     163
     164        offset = scrollable->calculateAutoscrollDirection(eventPosition);
     165        if (offset.isZero())
     166            return nullptr;
     167
     168        return scrollable;
     169    };
     170   
     171    RenderBox* scrollable = findDragAndDropScroller();
    150172    if (!scrollable) {
    151173        stopAutoscrollTimer();
    … …  
    153175    }
    154176
    155     Frame& frame = scrollable->frame();
    156 
    157     Page* page = frame.page();
    158     if (!page || !page->settings().autoscrollForDragAndDropEnabled()) {
    159         stopAutoscrollTimer();
    160         return;
    161     }
    162 
    163     IntSize offset = scrollable->calculateAutoscrollDirection(eventPosition);
    164     if (offset.isZero()) {
    165         stopAutoscrollTimer();
    166         return;
    167     }
    168 
    169177    m_dragAndDropAutoscrollReferencePosition = eventPosition + offset;
    170178
    171179    if (m_autoscrollType == NoAutoscroll) {
    172180        m_autoscrollType = AutoscrollForDragAndDrop;
    173         m_autoscrollRenderer = scrollable;
     181        m_autoscrollRenderer = WeakPtr { *scrollable };
    174182        m_dragAndDropAutoscrollStartTime = eventTime;
    175183        startAutoscrollTimer();
    176184    } else if (m_autoscrollRenderer != scrollable) {
    177185        m_dragAndDropAutoscrollStartTime = eventTime;
    178         m_autoscrollRenderer = scrollable;
     186        m_autoscrollRenderer = WeakPtr { *scrollable };
    179187    }
    180188}
    … …  
    211219}
    212220
    213 void AutoscrollController::startPanScrolling(RenderBox* scrollable, const IntPoint& lastKnownMousePosition)
     221void AutoscrollController::startPanScrolling(RenderBox& scrollable, const IntPoint& lastKnownMousePosition)
    214222{
    215223    // We don't want to trigger the autoscroll or the panScroll if it's already active
    … …  
    218226
    219227    m_autoscrollType = AutoscrollForPan;
    220     m_autoscrollRenderer = scrollable;
     228    m_autoscrollRenderer = WeakPtr { scrollable };
    221229    m_panScrollStartPos = lastKnownMousePosition;
    222230
    223     if (FrameView* view = scrollable->frame().view())
     231    if (auto* view = scrollable.frame().view())
    224232        view->addPanScrollIcon(lastKnownMousePosition);
    225     scrollable->frame().eventHandler().didPanScrollStart();
     233
     234    scrollable.frame().eventHandler().didPanScrollStart();
    226235    startAutoscrollTimer();
    227236}

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

    r222392 r293094  
    6767    void handleMouseReleaseEvent(const PlatformMouseEvent&);
    6868    void setPanScrollInProgress(bool);
    69     void startPanScrolling(RenderBox*, const IntPoint&);
     69    void startPanScrolling(RenderBox&, const IntPoint&);
    7070#endif
    7171
    … …  
    7878
    7979    Timer m_autoscrollTimer;
    80     RenderBox* m_autoscrollRenderer;
    81     AutoscrollType m_autoscrollType;
     80    WeakPtr<RenderBox> m_autoscrollRenderer;
     81    AutoscrollType m_autoscrollType { NoAutoscroll };
    8282    IntPoint m_dragAndDropAutoscrollReferencePosition;
    8383    WallTime m_dragAndDropAutoscrollStartTime;

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

    r293054 r293094  
    11281128void EventHandler::startPanScrolling(RenderElement& renderer)
    11291129{
    1130 #if !PLATFORM(IOS_FAMILY)
    11311130    if (!is<RenderBox>(renderer))
    11321131        return;
    1133     m_autoscrollController->startPanScrolling(&downcast<RenderBox>(renderer), lastKnownMousePosition());
     1132    m_autoscrollController->startPanScrolling(downcast<RenderBox>(renderer), lastKnownMousePosition());
    11341133    invalidateClick();
    1135 #endif
    11361134}
    11371135
Note: See TracChangeset for help on using the changeset viewer.