callback-removed-frame.html fails with threaded compositing. |
|||
Issue descriptionI have a WIP patch[1] that changes layout tests running in single-threaded mode actually run through the update phases in the compositor, which is closer to production behaviour. external/wpt/requestidlecallback/callback-removed-frame.html is failing here with the following assert: This is a testharness.js-based test. FAIL calling requestIdleCallback on a contentWindow from a removed iframe should not trigger the callback assert_unreached: requestIdleCallback callback should not trigger the callback Reached unreachable code Harness: the test ran to completion. I ran the test with threaded compositing to verify if this was a bug with the patch itself or something in the test and I get the same failure when running with threaded compositing. rmcilroy@, would you be the correct person to own this? And could you confirm if this is a bug with the test? I can temporarily disable it in that case. [1]: https://chromium-review.googlesource.com/c/chromium/src/+/900205
,
Feb 9 2018
It was multi-threaded compositor mode in content shell for layout tests.
,
Feb 9 2018
I tried following the code a bit and I can see that the idle task is still being posted after the frame is disconnected. HTMLFrameOwnerElement:ClearContentFrame is called when the frame is removed, but idle tasks make it through to LocalDomWindow which still has a valid Document and through that to the Scheduler. The only difference from the change above or running the test with threaded compositing is that the RendererScheduler starts getting callbacks from cc, which happen only after a frame is produced, that actually start the idle period.
I modified the test to replace the first idleCallback which is made when the frame is visible with an unreached_func and the test still passes.
<!doctype html>
<meta charset=utf-8>
<title>requestIdleCallback on removed frame shouldn't call back</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
async_test(function (t) {
assert_false(document.hidden, "document.hidden must exist and be false to run this test properly");
function start() {
var frame = document.createElement('iframe');
frame.addEventListener('load', _ => connect(frame), {once:true});
frame.src = "about:blank";
document.body.appendChild(frame);
}
function connect(frame) {
var contentWindow = frame.contentWindow;
contentWindow.requestIdleCallback(t.unreached_func("fist idlecallback happened"));
t.step_timeout(function() { t.done(); }, 1000);
}
function callback0(f, w) {
document.body.removeChild(f);
w.requestIdleCallback(t.unreached_func("requestIdleCallback callback should not trigger the callback"));
}
addEventListener('load', start, {once:true});
}, "calling requestIdleCallback on a contentWindow from a removed iframe should not trigger the callback");
</script>
,
Feb 12 2018
Thanks for taking a look. I've taken a look at this. First-off, with the single-threaded compositor we never run idle tasks, so almost all of these idle-callback tests currently fail with timeouts (see https://bugs.chromium.org/p/chromium/issues/detail?id=666993). We have a seperate set of tests in virtual/threaded which do work (since the threaded compositor is enabled there). As for callback-removed-frame.html - this one is incorrectly passing with the single-threaded compositor because none of the idle callbacks run (and the test calls done after a 1000ms timeout and doesn't check anything was run). With Chrome this test does pass correcly (it runs the first idle callback, but not the second as expected). However with the threaded compositor mode in content-shell for some reason it fails. khushalsagar@ - do you have any ideas for why this behaves differently in threaded compositor content shell compared with threaded compositor Chrome?
,
Feb 12 2018
If I run this test with chrome, I see it fail with the following exception instead:
Uncaught TypeError: w.requestIdleCallback is not a function
TypeError: w.requestIdleCallback is not a function
at callback0 (file:///usr/local/google/home/khushalsagar/code/clankium/src/third_party/WebKit/LayoutTests/external/wpt/requestidlecallback/callback-removed-frame.html:25:9)
at contentWindow.requestIdleCallback._ (file:///usr/local/google/home/khushalsagar/code/clankium/src/third_party/WebKit/LayoutTests/external/wpt/requestidlecallback/callback-removed-frame.html:19:46)
,
Feb 12 2018
And this is weird. Running this with content shell has the same result as above with chrome. But if you run content shell with --run-layout-test, which is the mode that the layout test runner uses, then it fails instead with the callback being invoked. I don't think the difference is stemming from cc, but some other setup that the layout test runner does.
,
Feb 13 2018
I'm disabling the test for now in https://chromium-review.googlesource.com/c/chromium/src/+/900205. Its currently a no-op anyway since idle callbacks don't work for single threaded mode in layout tests.
,
Mar 7 2018
Removing myself as I won't have time to look into this in the short-term.
,
Oct 3
|
|||
►
Sign in to add a comment |
|||
Comment 1 by rmcilroy@chromium.org
, Feb 9 2018Components: Blink>Scheduling