Skip to content

Commit

Permalink
Unreviewed, reverting 266622@main.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259875

Caused ~1% Speedometer 2.1 regression on macOS Sonoma

Reverted changeset:

"Integrate requestIdleCallback with HTML5 event loop and OpportunisticTaskScheduler"
https://bugs.webkit.org/show_bug.cgi?id=259850
https://commits.webkit.org/266622@main

Canonical link: https://commits.webkit.org/266626@main
  • Loading branch information
webkit-commit-queue authored and rniwa committed Aug 6, 2023
1 parent f818e6d commit 33c467b
Show file tree
Hide file tree
Showing 20 changed files with 61 additions and 155 deletions.
7 changes: 3 additions & 4 deletions LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1233,10 +1233,6 @@ http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html
# Content encoding sniffing is only supported by CFNetwork.
http/tests/xmlhttprequest/gzip-content-type-no-content-encoding.html [ Skip ]

# Only supported on CF (Apple) WebKit2 ports for now.
requestidlecallback [ Skip ]
imported/w3c/web-platform-tests/requestidlecallback [ Skip ]

# Only supported in WebKit2.
http/wpt/cross-origin-resource-policy/ [ Skip ]

Expand Down Expand Up @@ -6207,6 +6203,9 @@ imported/w3c/web-platform-tests/payment-request/payment-is-showing.https.html [
imported/w3c/web-platform-tests/permissions/permissions-query-feature-policy-attribute.https.sub.html [ Skip ]
imported/w3c/web-platform-tests/pointerevents/pointerlock/pointerevent_pointermove_in_pointerlock.html [ Skip ]
imported/w3c/web-platform-tests/pointerlock/mouse_buttons_back_forward.html [ Skip ]
imported/w3c/web-platform-tests/requestidlecallback/callback-idle-periods.html [ Skip ]
imported/w3c/web-platform-tests/requestidlecallback/callback-multiple-calls.html [ Skip ]
imported/w3c/web-platform-tests/requestidlecallback/callback-xhr-sync.html [ Skip ]
imported/w3c/web-platform-tests/resize-observer/observe.html [ Skip ]
imported/w3c/web-platform-tests/resize-observer/svg.html [ Skip ]
imported/w3c/web-platform-tests/resource-timing/entry-attributes.html [ Skip ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@ Test of requestIdleCallback idle period behavior
This test validates that window.requestIdleCallback deals with callbacks during idle periods correctly.


PASS Check that if an idle callback calls requestIdleCallback the new callback doesn't run in the current idle period.
Harness Error (TIMEOUT), message = null

TIMEOUT Check that if an idle callback calls requestIdleCallback the new callback doesn't run in the current idle period. Test timed out

Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@

Harness Error (TIMEOUT), message = null

PASS requestIdleCallback callbacks should be invoked in order (called iteratively)
PASS requestIdleCallback callbacks should be invoked in order (called recursively)
TIMEOUT requestIdleCallback callbacks should be invoked in order (called recursively) Test timed out

Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@

PASS re-schedule idle callbacks after sync xhr
Harness Error (TIMEOUT), message = null

TIMEOUT re-schedule idle callbacks after sync xhr Test timed out

5 changes: 2 additions & 3 deletions LayoutTests/platform/ios-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@ imported/w3c/web-platform-tests/file-system-access/ [ Pass ]
imported/w3c/web-platform-tests/fs/ [ Pass ]
storage/filesystemaccess/ [ Pass ]

requestidlecallback [ Pass ]
imported/w3c/web-platform-tests/requestidlecallback [ Pass ]

#//////////////////////////////////////////////////////////////////////////////////////////
# End platform-specific directories.
#//////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -2123,6 +2120,8 @@ webkit.org/b/231638 [ Debug ] webgl/pending/conformance/glsl/misc/shader-with-re

webkit.org/b/228127 imported/w3c/web-platform-tests/html/cross-origin-opener-policy/resource-popup.https.html [ Pass Crash Failure ]

webkit.org/b/231684 [ Debug ] requestidlecallback/requestidlecallback-document-gc.html [ Pass Crash ]

webkit.org/b/230509 fast/events/ios/dom-update-on-keydown-quirk.html [ Pass Failure ]

webkit.org/b/231714 fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html [ Pass Timeout ]
Expand Down
3 changes: 0 additions & 3 deletions LayoutTests/platform/mac-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,6 @@ imported/w3c/web-platform-tests/file-system-access/ [ Pass ]
imported/w3c/web-platform-tests/fs/ [ Pass ]
storage/filesystemaccess/ [ Pass ]

requestidlecallback [ Pass ]
imported/w3c/web-platform-tests/requestidlecallback [ Pass ]

#//////////////////////////////////////////////////////////////////////////////////////////
# End platform-specific directories.
#//////////////////////////////////////////////////////////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE


PASS event.persisted is true
PASS logsA.length is 0
PASS logsB.length is 0
PASS logsA.length is 4
PASS logsA.join(", ") is "A1, A2, A3, A4"
PASS logsB.length is 3
PASS logsB.join(", ") is "B1, B2, B3"
PASS logs.length is 0
PASS logs.length is 7
PASS logs.join(", ") is "A1, B1, A2, B2, A3, B3, A4"
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
document.body.appendChild(iframe);

let isInitialLoad = true;
const logsA = [];
const logsB = [];
const logs = [];
if (window.testRunner)
setTimeout(() => testRunner.notifyDone(), 3000);

Expand All @@ -26,25 +25,22 @@
setTimeout(() => testRunner.notifyDone(), 3000);

shouldBeTrue('event.persisted');
shouldBe('logsA.length', '0');
shouldBe('logsB.length', '0');
iframe.contentWindow.requestIdleCallback(() => logsB.push('B3'));
requestIdleCallback(() => logsA.push('A4'));
shouldBe('logs.length', '0');
iframe.contentWindow.requestIdleCallback(() => logs.push('B3'));
requestIdleCallback(() => logs.push('A4'));
requestIdleCallback(() => {
shouldBe('logsA.length', '4');
shouldBeEqualToString('logsA.join(", ")', 'A1, A2, A3, A4');
shouldBe('logsB.length', '3');
shouldBeEqualToString('logsB.join(", ")', 'B1, B2, B3');
shouldBe('logs.length', '7');
shouldBeEqualToString('logs.join(", ")', 'A1, B1, A2, B2, A3, B3, A4');
finishJSTest();
});
});

window.addEventListener("pagehide", function(event) {
requestIdleCallback(() => logsA.push('A1'));
iframe.contentWindow.requestIdleCallback(() => logsB.push('B1'));
requestIdleCallback(() => logsA.push('A2'));
iframe.contentWindow.requestIdleCallback(() => logsB.push('B2'));
requestIdleCallback(() => logsA.push('A3'));
requestIdleCallback(() => logs.push('A1'));
iframe.contentWindow.requestIdleCallback(() => logs.push('B1'));
requestIdleCallback(() => logs.push('A2'));
iframe.contentWindow.requestIdleCallback(() => logs.push('B2'));
requestIdleCallback(() => logs.push('A3'));
});

onload = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE


PASS requestIdleCallbackIsCalled is true
PASS logsA.length is 2
PASS logsA.join(", ") is "1.A1, 4.A2"
PASS logsB.length is 2
PASS logsB.join(", ") is "2.B1, 3.B2"
PASS logs.length is 4
PASS logs.join(", ") is "1.A1, 2.B1, 4.A2, 3.B2"
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
17 changes: 7 additions & 10 deletions LayoutTests/requestidlecallback/requestidlecallback-is-called.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,38 +10,35 @@

requestIdleCallbackIsCalled = false;
const iframe = document.createElement('iframe');
const logsA = [];
const logsB = [];
const logs = [];

iframe.onload = () => {
requestIdleCallback(() => {
requestIdleCallbackIsCalled = true;
logsA.push('1.A1');
logs.push('1.A1');
});

iframe.contentWindow.requestIdleCallback(() => {
requestIdleCallbackIsCalled = true;
logsB.push('2.B1');
logs.push('2.B1');
});

iframe.contentWindow.requestIdleCallback(() => {
requestIdleCallbackIsCalled = true;
logsB.push('3.B2');
logs.push('3.B2');
});

requestIdleCallback(() => {
requestIdleCallbackIsCalled = true;
logsA.push('4.A2');
logs.push('4.A2');
});
}
document.body.appendChild(iframe);

setTimeout(() => {
shouldBeTrue('requestIdleCallbackIsCalled');
shouldBe('logsA.length', '2');
shouldBeEqualToString('logsA.join(", ")', '1.A1, 4.A2');
shouldBe('logsB.length', '2');
shouldBeEqualToString('logsB.join(", ")', '2.B1, 3.B2');
shouldBe('logs.length', '4');
shouldBeEqualToString('logs.join(", ")', '1.A1, 2.B1, 4.A2, 3.B2');
finishJSTest();
}, 200);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE


PASS a3 is a2 + 1
PASS logsA.length is 2
PASS logsB.length is 1
PASS logsA.join(", ") is "A1, A3"
PASS logsB.join(", ") is "B1"
PASS logs.length is 3
PASS logs.join(", ") is "1.A1, 3.A2, 4.A3"
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,26 @@
jsTestIsAsync = true;

const iframe = document.createElement('iframe');
const logsA = [];
const logsB = [];
const logs = [];

iframe.onload = () => {
requestIdleCallback(() => {
logsA.push('A1');
cancelIdleCallback(b1);
logs.push('1.A1');
iframe.contentWindow.cancelIdleCallback(b1);
});

const b1 = iframe.contentWindow.requestIdleCallback(() => {
logsB.push('B1');
iframe.contentWindow.cancelIdleCallback(a3);
iframe.contentWindow.cancelIdleCallback(b2);
});
const b2 = iframe.contentWindow.requestIdleCallback(() => logsB.push('B2'));
const b1 = iframe.contentWindow.requestIdleCallback(() => logs.push('2.B1'));

window.a2 = requestIdleCallback(() => logsA.push('A2'));
window.a2 = requestIdleCallback(() => logs.push('3.A2'));
cancelIdleCallback(a2 + 1);
window.a3 = requestIdleCallback(() => logsA.push('A3'));
window.a3 = requestIdleCallback(() => logs.push('4.A3'));
shouldBe('a3', 'a2 + 1');
cancelIdleCallback(a2);
}
document.body.appendChild(iframe);

setTimeout(() => {
shouldBe('logsA.length', '2');
shouldBe('logsB.length', '1');
shouldBeEqualToString('logsA.join(", ")', 'A1, A3');
shouldBeEqualToString('logsB.join(", ")', 'B1');
shouldBe('logs.length', '3');
shouldBeEqualToString('logs.join(", ")', '1.A1, 3.A2, 4.A3');
finishJSTest();
}, 200);

Expand Down
3 changes: 0 additions & 3 deletions Source/WebCore/dom/EventLoop.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ class EventLoop : public RefCounted<EventLoop>, public CanMakeWeakPtr<EventLoop>
void run();
void clearAllTasks();

// FIXME: Account for fully-activeness of each document.
bool hasTasksForFullyActiveDocument() const { return !m_tasks.isEmpty(); }

private:
void scheduleToRunIfNeeded();
virtual void scheduleToRun() = 0;
Expand Down
12 changes: 8 additions & 4 deletions Source/WebCore/dom/IdleCallbackController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "Document.h"
#include "FrameDestructionObserverInlines.h"
#include "IdleDeadline.h"
#include "Timer.h"
#include "WindowEventLoop.h"

namespace WebCore {
Expand All @@ -41,11 +40,18 @@ IdleCallbackController::IdleCallbackController(Document& document)
}
int IdleCallbackController::queueIdleCallback(Ref<IdleRequestCallback>&& callback, Seconds)
{
bool startIdlePeriod = m_idleRequestCallbacks.isEmpty() && m_runnableIdleCallbacks.isEmpty();

++m_idleCallbackIdentifier;
auto handle = m_idleCallbackIdentifier;

m_idleRequestCallbacks.append({ handle, WTFMove(callback) });

if (startIdlePeriod)
queueTaskToStartIdlePeriod();

// FIXME: Queue a task if timeout is positive.

return handle;
}

Expand Down Expand Up @@ -87,9 +93,7 @@ void IdleCallbackController::startIdlePeriod()
m_runnableIdleCallbacks.append({ request.identifier, WTFMove(request.callback) });
m_idleRequestCallbacks.clear();

if (m_runnableIdleCallbacks.isEmpty())
return;

ASSERT(!m_runnableIdleCallbacks.isEmpty());
queueTaskToInvokeIdleCallbacks(deadline);

m_lastDeadline = deadline;
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/dom/IdleCallbackController.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,9 @@ class IdleCallbackController {
int queueIdleCallback(Ref<IdleRequestCallback>&&, Seconds timeout);
void removeIdleCallback(int);

void startIdlePeriod();

private:
void queueTaskToStartIdlePeriod();
void startIdlePeriod();
void queueTaskToInvokeIdleCallbacks(MonotonicTime deadline);
void invokeIdleCallbacks(MonotonicTime deadline);

Expand Down
2 changes: 0 additions & 2 deletions Source/WebCore/dom/Microtasks.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ class MicrotaskQueue final {

WEBCORE_EXPORT void addCheckpointTask(std::unique_ptr<EventLoopTask>&&);

bool isEmpty() const { return m_microtaskQueue.isEmpty(); }

private:
JSC::VM& vm() const { return m_vm.get(); }

Expand Down
28 changes: 0 additions & 28 deletions Source/WebCore/dom/WindowEventLoop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include "CustomElementReactionQueue.h"
#include "Document.h"
#include "HTMLSlotElement.h"
#include "IdleCallbackController.h"
#include "Microtasks.h"
#include "MutationObserver.h"
#include "SecurityOrigin.h"
Expand Down Expand Up @@ -116,33 +115,6 @@ MicrotaskQueue& WindowEventLoop::microtaskQueue()
return *m_microtaskQueue;
}

void WindowEventLoop::opportunisticallyRunIdleCallbacks()
{
if (shouldEndIdlePeriod())
return;

forEachAssociatedContext([](ScriptExecutionContext& context) {
RefPtr document = dynamicDowncast<Document>(context);
if (!document)
return;
auto* idleCallbackController = document->idleCallbackController();
if (!idleCallbackController)
return;
idleCallbackController->startIdlePeriod();
});
}

bool WindowEventLoop::shouldEndIdlePeriod()
{
if (hasTasksForFullyActiveDocument())
return true;
if (!microtaskQueue().isEmpty())
return true;
if (m_hasARenderingOpportunity)
return true;
return false;
}

void WindowEventLoop::didReachTimeToRun()
{
Ref protectedThis { *this }; // Executing tasks may remove the last reference to this WindowEventLoop.
Expand Down
7 changes: 0 additions & 7 deletions Source/WebCore/dom/WindowEventLoop.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ class WindowEventLoop final : public EventLoop {

CustomElementQueue& backupElementQueue();

void didScheduleRenderingUpdate() { m_hasARenderingOpportunity = true; }
void didFinishRenderingUpdate() { m_hasARenderingOpportunity = false; }
void opportunisticallyRunIdleCallbacks();
bool shouldEndIdlePeriod();

WEBCORE_EXPORT static void breakToAllowRenderingUpdate();

private:
Expand Down Expand Up @@ -87,8 +82,6 @@ class WindowEventLoop final : public EventLoop {

std::unique_ptr<CustomElementQueue> m_customElementQueue;
bool m_processingBackupElementQueue { false };

bool m_hasARenderingOpportunity { false };
};

} // namespace WebCore
Loading

0 comments on commit 33c467b

Please sign in to comment.