Skip to content
Permalink
Browse files
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
  • Loading branch information
smfr committed Apr 20, 2022
1 parent 62f6817 commit f335db6278dcf8c8670cb792956510a2edb8c1d9
Showing 4 changed files with 77 additions and 46 deletions.
@@ -1,3 +1,27 @@
2022-04-20 Simon Fraser <simon.fraser@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-04-20 Yacine Bandou <yacine.bandou@softathome.com>

[GStreamer] REGRESSION(r285586): we never end the playback of some videos
@@ -56,14 +56,12 @@ static Frame* getMainFrame(Frame* frame)

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,32 +74,34 @@ void AutoscrollController::startAutoscrollForSelection(RenderObject* renderer)
// 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;

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;
}

if (!rendererIsBeingDestroyed)
scrollable->stopAutoscroll();

#if ENABLE(PAN_SCROLLING)
if (panScrollInProgress()) {
FrameView& frameView = scrollable->view().frameView();
@@ -114,8 +114,8 @@ void AutoscrollController::stopAutoscrollTimer(bool rendererIsBeingDestroyed)

#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 @@ void AutoscrollController::updateAutoscrollRenderer()
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 = dynamicDowncast<RenderBox>(renderer);
}

void AutoscrollController::updateDragAndDrop(Node* dropTargetNode, const IntPoint& eventPosition, WallTime eventTime)
{
if (!dropTargetNode) {
stopAutoscrollTimer();
return;
}

RenderBox* scrollable = RenderBox::findAutoscrollable(dropTargetNode->renderer());
if (!scrollable) {
stopAutoscrollTimer();
if (!is<RenderBox>(renderer)) {
m_autoscrollRenderer = nullptr;
return;
}

Frame& frame = scrollable->frame();

Page* page = frame.page();
if (!page || !page->settings().autoscrollForDragAndDropEnabled()) {
stopAutoscrollTimer();
return;
}
m_autoscrollRenderer = WeakPtr { downcast<RenderBox>(*renderer) };
}

IntSize offset = scrollable->calculateAutoscrollDirection(eventPosition);
if (offset.isZero()) {
void AutoscrollController::updateDragAndDrop(Node* dropTargetNode, const IntPoint& eventPosition, WallTime eventTime)
{
IntSize offset;
auto findDragAndDropScroller = [&]() -> RenderBox* {
if (!dropTargetNode)
return nullptr;

auto* scrollable = RenderBox::findAutoscrollable(dropTargetNode->renderer());
if (!scrollable)
return nullptr;

auto& frame = scrollable->frame();
auto* page = frame.page();
if (!page || !page->settings().autoscrollForDragAndDropEnabled())
return nullptr;

offset = scrollable->calculateAutoscrollDirection(eventPosition);
if (offset.isZero())
return nullptr;

return scrollable;
};

RenderBox* scrollable = findDragAndDropScroller();
if (!scrollable) {
stopAutoscrollTimer();
return;
}
@@ -170,12 +178,12 @@ void AutoscrollController::updateDragAndDrop(Node* dropTargetNode, const IntPoin

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,19 +218,20 @@ bool AutoscrollController::panScrollInProgress() const
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())
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
@@ -66,7 +66,7 @@ class AutoscrollController {
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 @@ class AutoscrollController {
#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)
@@ -1127,12 +1127,10 @@ void EventHandler::didPanScrollStop()

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)

0 comments on commit f335db6

Please sign in to comment.