requestIdleCallback without a timeout never fires in the layout test runner. |
||||||||
Issue description
This:
<!DOCTYPE html>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
requestIdleCallback(() => {
if (window.testRunner)
testRunner.notifyDone()
});
</script>
... will hang the test runner:
$ content_shell --run-layout-test hang.html
Using a zero timeout will also hang:
requestIdleCallback(() => {
if (window.testRunner)
testRunner.notifyDone();
}, {timeout: 0});
... but using a non-zero timeout works:
requestIdleCallback(() => {
if (window.testRunner)
testRunner.notifyDone();
}, {timeout: 1});
,
Mar 16 2016
As discussed on issue 595155 , I think this is related to layout tests not running with a threaded compositor. Does the test work if you pass --enable-threaded-compositing?
,
Mar 16 2016
Yes, --enabled-threaded-compositing fixes it. This seems important to fix. It's another case where our tests are running different code from what we ship.
,
Mar 16 2016
Please don't leave the 'Blink' component on bugs.
,
Mar 17 2016
As a workaround until we fix issue 595484 , you could put your layout tests in the LayoutTests/virtual/threaded virtual test suite (like the rIC layout tests) so that they run with the threaded compositor.
,
Mar 21 2016
,
Mar 21 2016
Issue 595484 has been merged into this issue.
,
Mar 21 2016
I dug around a bit and it looks like layout tests (i.e., WebTestProxyBase[1]) are completely bypassing cc/ (not just cc/scheduler) for painting and animations. There is a valid renderer scheduler, but since cc is not involved, it has no idea about what's going on and hence no idle tasks. I think there are two options: either make layout tests exercise the complete cc machinery like the real browser does or add some test hook for running pending idle tasks. I'd prefer the former but I'm not completely sure how feasible it is. [1] https://code.google.com/p/chromium/codesearch#chromium/src/components/test_runner/web_test_proxy.cc&rcl=1458545209&l=649
,
Mar 21 2016
I had the same idea as your second suggestion (add a test runner hook). I should think that would be relatively straightforward to implement, and it serves the immediate purpose of enabling layout tests. Enabling the cc code for the layout tests sounds like an ambitious (and dubious) undertaking.
,
Mar 21 2016
BTW, if you plan to do this, I would suggest either taking a callback argument to run after all idle tasks have completed, e.g.: test_runner.runIdleTasks(finishJSTest); ... or make it return a promise: test_runner.runIdleTasks().then(finishJSTest);
,
Mar 21 2016
+enne, who I think has looked into option 1 extensively.
,
Mar 21 2016
As I mentioned in the duped https://bugs.chromium.org/p/chromium/issues/detail?id=595484, I don't think any of the idle callback tests should use threaded compositing or make any other test using that machinery have to use that virtual suite. Option #2 sounds much more feasible than option #1 to me.
,
Mar 21 2016
To avoid confusion, I wasn't suggesting introducing threaded compositing to layout tests, but rather make the layout tests use the single threaded compositing infrastructure they are already initializing (but not using). The simpler option #2 works for me to if y'all are happy with it.
,
Apr 4 2016
szager@ has a WIP patch for this: https://codereview.chromium.org/1832673002/
,
Apr 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69e3ac0fc139d5e7271987c39b6126f3525d88d9 commit 69e3ac0fc139d5e7271987c39b6126f3525d88d9 Author: szager <szager@chromium.org> Date: Tue Apr 12 03:11:34 2016 Add testRunner.runIdleTasks() to force idle tasks to run. BUG= 595152 R=skyostil@chromium.org,rmcilroy@chromium.org,esprehn@chromium.org Review URL: https://codereview.chromium.org/1832673002 Cr-Commit-Position: refs/heads/master@{#386563} [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/BUILD.gn [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/renderer/renderer_scheduler_impl.cc [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/renderer/renderer_scheduler_impl.h [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/scheduler.gyp [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/scheduler.gypi [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/test/DEPS [add] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/test/renderer_scheduler_test_support.cc [add] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/scheduler/test/renderer_scheduler_test_support.h [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/test_runner/test_runner.cc [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/test_runner/test_runner.h [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/components/test_runner/web_test_delegate.h [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/content/content_tests.gypi [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/content/public/test/layouttest_support.h [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/content/shell/renderer/layout_test/blink_test_runner.h [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/content/test/BUILD.gn [modify] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/content/test/layouttest_support.cc [add] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/third_party/WebKit/LayoutTests/idle-callback/test-runner-run-idle-tasks-expected.txt [add] https://crrev.com/69e3ac0fc139d5e7271987c39b6126f3525d88d9/third_party/WebKit/LayoutTests/idle-callback/test-runner-run-idle-tasks.html
,
Apr 27 2016
,
Apr 27 2016
This bug is now as fixed as it's ever likely to be: a test can (and should) use test_runner.runIdleTasks to force idle tasks to run immediately. The burden is entirely on the test author, but at least there's a way to do it now.
,
Apr 28 2016
Thanks, this is great!
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/479085f864f815a6378913310125b26456c7bc6e commit 479085f864f815a6378913310125b26456c7bc6e Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Tue Aug 01 13:11:50 2017 Remove timed-out test from LeakExpectation The test is now timed out on the flakiness dashboard because idle tasks are never called on tests until testRunner.runIdleTasks explcitly ( crbug.com/595152 ). This doesn't leak now. This CL also fixes a comment regarding crbug.com/726218 . Bug: 595152 , 662477 Change-Id: I757bd5e72063933b28d92dc69cb4a3e42e9e4c56 Reviewed-on: https://chromium-review.googlesource.com/595792 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Cr-Commit-Position: refs/heads/master@{#490959} [modify] https://crrev.com/479085f864f815a6378913310125b26456c7bc6e/third_party/WebKit/LayoutTests/LeakExpectations |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by skyos...@chromium.org
, Mar 16 2016Components: Blink>Scheduling Blink