[webkit-changes] [WebKit/WebKit] cb3478: requestIdleCallback can continuously schedule Wind...

[Ryosuke Niwa]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: cb3478b28c691980974f822d1e662be89a504cf0
      https://github.com/WebKit/WebKit/commit/cb3478b28c691980974f822d1e662be89a504cf0
  Author: Ryosuke Niwa <rniwa at webkit.org>
  Date:   2024-10-30 (Wed, 30 Oct 2024)

  Changed paths:
    M LayoutTests/requestidlecallback/requestidlecallback-deadline-cap-by-rendering-update-expected.txt
    M LayoutTests/requestidlecallback/requestidlecallback-deadline-cap-by-rendering-update.html
    M Source/WebCore/dom/Document.cpp
    M Source/WebCore/dom/Document.h
    M Source/WebCore/dom/IdleCallbackController.cpp
    M Source/WebCore/dom/WindowEventLoop.cpp
    M Source/WebCore/dom/WindowEventLoop.h
    M Source/WebCore/page/OpportunisticTaskScheduler.cpp
    M Source/WebCore/page/OpportunisticTaskScheduler.h
    M Source/WebCore/page/Page.cpp
    M Source/WebCore/page/Page.h

  Log Message:
  -----------
  requestIdleCallback can continuously schedule WindowEventLoop's timer
https://bugs.webkit.org/show_bug.cgi?id=282133

Reviewed by Wenson Hsieh.

The bug was most likely caused by WindowEventLoop::didReachTimeToRun scheduling itself when exiting early
for shouldEndIdlePeriod returning true. While I have not been able to reproduce the issue, this PR
refactors the code in such a way that a bug like this could never happen.

Prior to this PR, WindowEventLoop::didReachTimeToRun() was responsible for re-scheduling itself. The idea
was that WindowEventLoop will utilize the WindowEventLoop's main timer to drive the idle callbacks when
there is an idle period. In practice, this led to a fragile design where shouldEndIdlePeriod returning
true can end up in a busy loop between WindowEventLoop::didReachTimeToRun() and Timer.

This PR refactors this code so that there is a dedicated m_idleTimer timer for scheduling idle callback,
and WindowEventLoop::opportunisticallyRunIdleCallbacks and scheduleIdlePeriod are responsible for
scheduling this timer. opportunisticallyRunIdleCallbacks specifically schedules the timer at 1ms after
the next rendering update or next timer which ever is sooner if either is scheduled. Neither is scheduled,
it would schedule immediately trigger a 0s timer.

This PR also removes the knowledge of rendering update time in WindowEventLoop. Instead, WindowEventLoop
will dynamically consults each Document's Page object for the next rendering update time. It also removes
the flag in OpportunisticTaskScheduler indicating there is an idle callback or not. Instead,
OpportunisticTaskScheduler will dynamically calls document's hasPendingIdleCallback.

* LayoutTests/requestidlecallback/requestidlecallback-deadline-cap-by-rendering-update.html:

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::hasPendingIdleCallback const): Added.

* Source/WebCore/dom/Document.h:
(WebCore::Document::idleCallbackController const):

* Source/WebCore/dom/IdleCallbackController.cpp:
(WebCore::IdleCallbackController::queueIdleCallback): Always schedule idle period when requestIdleCallback
is called.

* Source/WebCore/dom/WindowEventLoop.cpp:
(WebCore::WindowEventLoop::WindowEventLoop):
(WebCore::WindowEventLoop::scheduleIdlePeriod): Schedule m_idleTimer in 0ms. This is okay because
WindowEventLoop::opportunisticallyRunIdleCallbacks exits early when there are other (higher priority)
tasks and microtasks scheduled in the event loop.
(WebCore::WindowEventLoop::didScheduleRenderingUpdate): Deleted.
(WebCore::WindowEventLoop::didStartRenderingUpdate): Deleted.
(WebCore::WindowEventLoop::opportunisticallyRunIdleCallbacks):
(WebCore::WindowEventLoop::shouldEndIdlePeriod): Now returns true when there are (higher priority) tasks
and microtasks scheduled in the event loop.
(WebCore::WindowEventLoop::computeIdleDeadline):
(WebCore::WindowEventLoop::nextScheduledWorkTime const): Extracted from
(WebCore::WindowEventLoop::nextRenderingTime const): Now polls rendering update time from each Page.
(WebCore::WindowEventLoop::didReachTimeToRun): Removed the much of logic for idle callback.
(WebCore::WindowEventLoop::didFireIdleTimer): Added.

* Source/WebCore/dom/WindowEventLoop.h:

* Source/WebCore/page/OpportunisticTaskScheduler.cpp:
(WebCore::OpportunisticTaskScheduler::rescheduleIfNeeded):
(WebCore::OpportunisticTaskScheduler::runLoopObserverFired):

* Source/WebCore/page/OpportunisticTaskScheduler.h:

* Source/WebCore/page/Page.cpp:
(WebCore::Page::scheduleRenderingUpdateInternal):
(WebCore::Page::nextRenderingUpdateTimestamp const): Added.
(WebCore::Page::updateRendering):
(WebCore::Page::findMatchingLocalDocument const): Added.
(WebCore::Page::opportunisticallyRunIdleCallbacks):

* Source/WebCore/page/Page.h:
(WebCore::Page::nextRenderingUpdateTimestamp const):

Canonical link: https://commits.webkit.org/285933@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications