Title: [294846] branches/safari-613-branch/Source/WebCore
Revision
294846
Author
alanc...@apple.com
Date
2022-05-25 16:50:02 -0700 (Wed, 25 May 2022)

Log Message

Cherry-pick r293094. rdar://problem/91683009

    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):

    Canonical link: https://commits.webkit.org/249805@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293094 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (294845 => 294846)


--- branches/safari-613-branch/Source/WebCore/ChangeLog	2022-05-25 23:49:59 UTC (rev 294845)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog	2022-05-25 23:50:02 UTC (rev 294846)
@@ -1,5 +1,58 @@
 2022-05-19  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r293094. rdar://problem/91683009
+
+    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):
+    
+    Canonical link: https://commits.webkit.org/249805@main
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293094 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-04-20  Simon Fraser  <simon.fra...@apple.com>
+
+            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):
+
+2022-05-19  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r292596. rdar://problem/88862115
 
     Take top layers into account in addLayers/removeLayers

Modified: branches/safari-613-branch/Source/WebCore/page/AutoscrollController.cpp (294845 => 294846)


--- branches/safari-613-branch/Source/WebCore/page/AutoscrollController.cpp	2022-05-25 23:49:59 UTC (rev 294845)
+++ branches/safari-613-branch/Source/WebCore/page/AutoscrollController.cpp	2022-05-25 23:50:02 UTC (rev 294846)
@@ -56,14 +56,12 @@
 
 AutoscrollController::AutoscrollController()
     : m_autoscrollTimer(*this, &AutoscrollController::autoscrollTimerFired)
-    , m_autoscrollRenderer(nullptr)
-    , m_autoscrollType(NoAutoscroll)
 {
 }
 
 RenderBox* AutoscrollController::autoscrollRenderer() const
 {
-    return m_autoscrollRenderer;
+    return m_autoscrollRenderer.get();
 }
 
 bool AutoscrollController::autoscrollInProgress() const
@@ -76,17 +74,18 @@
     // We don't want to trigger the autoscroll or the panScroll if it's already active
     if (m_autoscrollTimer.isActive())
         return;
-    RenderBox* scrollable = RenderBox::findAutoscrollable(renderer);
+    auto* scrollable = RenderBox::findAutoscrollable(renderer);
     if (!scrollable)
         return;
     m_autoscrollType = AutoscrollForSelection;
-    m_autoscrollRenderer = scrollable;
+    m_autoscrollRenderer = WeakPtr { *scrollable };
     startAutoscrollTimer();
 }
 
 void AutoscrollController::stopAutoscrollTimer(bool rendererIsBeingDestroyed)
 {
-    RenderBox* scrollable = m_autoscrollRenderer;
+    auto scrollable = m_autoscrollRenderer;
+
     m_autoscrollTimer.stop();
     m_autoscrollRenderer = nullptr;
 
@@ -93,9 +92,9 @@
     if (!scrollable)
         return;
 
-    Frame& frame = scrollable->frame();
-    if (autoscrollInProgress() && frame.eventHandler().mouseDownWasInSubframe()) {
-        if (auto subframe = frame.eventHandler().subframeForTargetNode(frame.eventHandler().mousePressNode()))
+    auto* frame = scrollable->document().frame();
+    if (autoscrollInProgress() && frame && frame->eventHandler().mouseDownWasInSubframe()) {
+        if (auto subframe = frame->eventHandler().subframeForTargetNode(frame->eventHandler().mousePressNode()))
             subframe->eventHandler().stopAutoscrollTimer(rendererIsBeingDestroyed);
         return;
     }
@@ -102,6 +101,7 @@
 
     if (!rendererIsBeingDestroyed)
         scrollable->stopAutoscroll();
+
 #if ENABLE(PAN_SCROLLING)
     if (panScrollInProgress()) {
         FrameView& frameView = scrollable->view().frameView();
@@ -114,8 +114,8 @@
 
 #if ENABLE(PAN_SCROLLING)
     // If we're not in the top frame we notify it that we are not doing a panScroll any more.
-    if (!frame.isMainFrame())
-        frame.mainFrame().eventHandler().didPanScrollStop();
+    if (frame && !frame->isMainFrame())
+        frame->mainFrame().eventHandler().didPanScrollStop();
 #endif
 }
 
@@ -124,44 +124,52 @@
     if (!m_autoscrollRenderer)
         return;
 
-    RenderObject* renderer = m_autoscrollRenderer;
+    RenderObject* renderer = m_autoscrollRenderer.get();
 
 #if ENABLE(PAN_SCROLLING)
     constexpr OptionSet<HitTestRequest::Type> hitType { HitTestRequest::Type::ReadOnly, HitTestRequest::Type::Active, HitTestRequest::Type::AllowChildFrameContent };
     HitTestResult hitTest = m_autoscrollRenderer->frame().eventHandler().hitTestResultAtPoint(m_panScrollStartPos, hitType);
 
-    if (Node* nodeAtPoint = hitTest.innerNode())
+    if (auto* nodeAtPoint = hitTest.innerNode())
         renderer = nodeAtPoint->renderer();
 #endif
 
     while (renderer && !(is<RenderBox>(*renderer) && downcast<RenderBox>(*renderer).canAutoscroll()))
         renderer = renderer->parent();
-    m_autoscrollRenderer = is<RenderBox>(renderer) ? downcast<RenderBox>(renderer) : nullptr;
+
+    if (!is<RenderBox>(renderer)) {
+        m_autoscrollRenderer = nullptr;
+        return;
+    }
+
+    m_autoscrollRenderer = WeakPtr { downcast<RenderBox>(*renderer) };
 }
 
 void AutoscrollController::updateDragAndDrop(Node* dropTargetNode, const IntPoint& eventPosition, WallTime eventTime)
 {
-    if (!dropTargetNode) {
-        stopAutoscrollTimer();
-        return;
-    }
+    IntSize offset;
+    auto findDragAndDropScroller = [&]() -> RenderBox* {
+        if (!dropTargetNode)
+            return nullptr;
 
-    RenderBox* scrollable = RenderBox::findAutoscrollable(dropTargetNode->renderer());
-    if (!scrollable) {
-        stopAutoscrollTimer();
-        return;
-    }
+        auto* scrollable = RenderBox::findAutoscrollable(dropTargetNode->renderer());
+        if (!scrollable)
+            return nullptr;
 
-    Frame& frame = scrollable->frame();
+        auto& frame = scrollable->frame();
+        auto* page = frame.page();
+        if (!page || !page->settings().autoscrollForDragAndDropEnabled())
+            return nullptr;
 
-    Page* page = frame.page();
-    if (!page || !page->settings().autoscrollForDragAndDropEnabled()) {
-        stopAutoscrollTimer();
-        return;
-    }
+        offset = scrollable->calculateAutoscrollDirection(eventPosition);
+        if (offset.isZero())
+            return nullptr;
 
-    IntSize offset = scrollable->calculateAutoscrollDirection(eventPosition);
-    if (offset.isZero()) {
+        return scrollable;
+    };
+    
+    RenderBox* scrollable = findDragAndDropScroller();
+    if (!scrollable) {
         stopAutoscrollTimer();
         return;
     }
@@ -170,12 +178,12 @@
 
     if (m_autoscrollType == NoAutoscroll) {
         m_autoscrollType = AutoscrollForDragAndDrop;
-        m_autoscrollRenderer = scrollable;
+        m_autoscrollRenderer = WeakPtr { *scrollable };
         m_dragAndDropAutoscrollStartTime = eventTime;
         startAutoscrollTimer();
     } else if (m_autoscrollRenderer != scrollable) {
         m_dragAndDropAutoscrollStartTime = eventTime;
-        m_autoscrollRenderer = scrollable;
+        m_autoscrollRenderer = WeakPtr { *scrollable };
     }
 }
 
@@ -210,7 +218,7 @@
     return m_autoscrollType == AutoscrollForPan || m_autoscrollType == AutoscrollForPanCanStop;
 }
 
-void AutoscrollController::startPanScrolling(RenderBox* scrollable, const IntPoint& lastKnownMousePosition)
+void AutoscrollController::startPanScrolling(RenderBox& scrollable, const IntPoint& lastKnownMousePosition)
 {
     // We don't want to trigger the autoscroll or the panScroll if it's already active
     if (m_autoscrollTimer.isActive())
@@ -217,12 +225,13 @@
         return;
 
     m_autoscrollType = AutoscrollForPan;
-    m_autoscrollRenderer = scrollable;
+    m_autoscrollRenderer = WeakPtr { scrollable };
     m_panScrollStartPos = lastKnownMousePosition;
 
-    if (FrameView* view = scrollable->frame().view())
+    if (auto* view = scrollable.frame().view())
         view->addPanScrollIcon(lastKnownMousePosition);
-    scrollable->frame().eventHandler().didPanScrollStart();
+
+    scrollable.frame().eventHandler().didPanScrollStart();
     startAutoscrollTimer();
 }
 #else

Modified: branches/safari-613-branch/Source/WebCore/page/AutoscrollController.h (294845 => 294846)


--- branches/safari-613-branch/Source/WebCore/page/AutoscrollController.h	2022-05-25 23:49:59 UTC (rev 294845)
+++ branches/safari-613-branch/Source/WebCore/page/AutoscrollController.h	2022-05-25 23:50:02 UTC (rev 294846)
@@ -66,7 +66,7 @@
     void didPanScrollStop();
     void handleMouseReleaseEvent(const PlatformMouseEvent&);
     void setPanScrollInProgress(bool);
-    void startPanScrolling(RenderBox*, const IntPoint&);
+    void startPanScrolling(RenderBox&, const IntPoint&);
 #endif
 
 private:
@@ -77,8 +77,8 @@
 #endif
 
     Timer m_autoscrollTimer;
-    RenderBox* m_autoscrollRenderer;
-    AutoscrollType m_autoscrollType;
+    WeakPtr<RenderBox> m_autoscrollRenderer;
+    AutoscrollType m_autoscrollType { NoAutoscroll };
     IntPoint m_dragAndDropAutoscrollReferencePosition;
     WallTime m_dragAndDropAutoscrollStartTime;
 #if ENABLE(PAN_SCROLLING)

Modified: branches/safari-613-branch/Source/WebCore/page/EventHandler.cpp (294845 => 294846)


--- branches/safari-613-branch/Source/WebCore/page/EventHandler.cpp	2022-05-25 23:49:59 UTC (rev 294845)
+++ branches/safari-613-branch/Source/WebCore/page/EventHandler.cpp	2022-05-25 23:50:02 UTC (rev 294846)
@@ -1125,12 +1125,10 @@
 
 void EventHandler::startPanScrolling(RenderElement& renderer)
 {
-#if !PLATFORM(IOS_FAMILY)
     if (!is<RenderBox>(renderer))
         return;
-    m_autoscrollController->startPanScrolling(&downcast<RenderBox>(renderer), lastKnownMousePosition());
+    m_autoscrollController->startPanScrolling(downcast<RenderBox>(renderer), lastKnownMousePosition());
     invalidateClick();
-#endif
 }
 
 #endif // ENABLE(PAN_SCROLLING)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to