Commit 8f30f322 authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

Revert "Worker: Abort all inflight tasks in Worklet::ContextDestroyed()"

This reverts commit c95749d2.

Reason for revert:
Some web_tests are crashing due to this change.
https://test-results.appspot.com/data/layout_results/WebKit_Linux_Trusty_ASAN/26566/webkit_layout_tests/layout-test-results/results.html

Original change's description:
> Worker: Abort all inflight tasks in Worklet::ContextDestroyed()
> 
> Before this CL, all inflight tasks can be retained in
> Worklet::pending_tasks_set_ even after context destruction, and that causes
> dcheck failures in the destructor of Worklet. This CL fixes it by aborting them
> in Worklet::ContextDestroyed().
> 
> In addition, this CL adds test coverage of addModule() calls on a detached
> iframe. In the tests, LayoutWorklet is used for testing main thread worklets
> instead of PaintWorklet because PaintWorklet will be switched to
> off-the-main-thread worklets (see https://crbug.com/829967).
> 
> Bug: 962355
> Change-Id: I7da71e7f4bdcbfa20125853832122a733c118a7a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626062
> Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#662923}

TBR=falken@chromium.org,nhiroki@chromium.org

Change-Id: I54041a299dfdc74ff298b001cd42609fe2c11901
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 962355
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627843Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662962}
parent c5c90f88
...@@ -94,15 +94,6 @@ void Worklet::ContextDestroyed(ExecutionContext* execution_context) { ...@@ -94,15 +94,6 @@ void Worklet::ContextDestroyed(ExecutionContext* execution_context) {
module_responses_map_->Dispose(); module_responses_map_->Dispose();
for (const auto& proxy : proxies_) for (const auto& proxy : proxies_)
proxy->TerminateWorkletGlobalScope(); proxy->TerminateWorkletGlobalScope();
for (auto iter = pending_tasks_set_.begin();
iter != pending_tasks_set_.end();) {
// Move the iterator forward before calling WorkletPendingTasks::Abort()
// because that modifies |pending_tasks_set_| and invalidates the iterator.
auto current = iter;
++iter;
(*current)->Abort();
}
DCHECK(!HasPendingTasks());
} }
bool Worklet::HasPendingTasks() const { bool Worklet::HasPendingTasks() const {
......
...@@ -17,53 +17,20 @@ function with_iframe(url) { ...@@ -17,53 +17,20 @@ function with_iframe(url) {
}); });
} }
// These tests should not be upstreamed to WPT because the spec does not define // This test should not be upstreamed to WPT because the spec does not define
// behavior in the case where addModule() is called from a detached frame. // behavior in the case where addModule() is called from a detached frame.
promise_test(t => {
promise_test(async t => { const kFrameUrl = 'resources/blank.html';
const frame = await with_iframe('resources/blank.html'); const kScriptUrl = 'resources/empty-worklet-script.js';
const worklet = frame.contentWindow.CSS.layoutWorklet;
frame.remove(); return with_iframe(kFrameUrl)
await promise_rejects( .then(frame => {
t, 'InvalidStateError', let worklet = frame.contentWindow.CSS.paintWorklet;
worklet.addModule('resources/empty-worklet-script.js')); frame.remove();
}, '[main thread worklet] addModule() on a detached iframe should be ' + return worklet.addModule(kScriptUrl);
'rejected.'); })
.then(() => assert_unreached('addModule() should fail.'))
promise_test(async t => { .catch(e => assert_equals(e.name, 'InvalidStateError', e));
const frame = await with_iframe('resources/blank.html'); }, 'addModule() on a detached iframe should be rejected.');
const worklet = frame.contentWindow.CSS.animationWorklet;
frame.remove();
await promise_rejects(
t, 'InvalidStateError',
worklet.addModule('resources/empty-worklet-script.js'));
}, '[off main thread worklet] addModule() on a detached iframe should be ' +
'rejected.');
promise_test(async t => {
const frame = await with_iframe('resources/blank.html');
const worklet = frame.contentWindow.CSS.layoutWorklet;
const promise = worklet.addModule('resources/empty-worklet-script.js');
frame.remove();
// Wait a moment to confirm that asynchronous addModule() operation on the
// detached iframe doesn't crash. We cannot wait for the promise returned by
// addModule() because it's never settled after context destruction.
await new Promise(resolve => setTimeout(resolve, 10));
}, '[main thread worklet] detaching an iframe after addModule() should not ' +
'crash.');
promise_test(async t => {
const frame = await with_iframe('resources/blank.html');
const worklet = frame.contentWindow.CSS.animationWorklet;
const promise = worklet.addModule('resources/empty-worklet-script.js');
frame.remove();
// Wait a moment to confirm that asynchronous addModule() operation on the
// detached iframe doesn't crash. We cannot wait for the promise returned by
// addModule() because it's never settled after context destruction.
await new Promise(resolve => setTimeout(resolve, 10));
}, '[off main thread worklet] detaching an iframe after addModule() should ' +
'not crash.');
</script> </script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment