From 141b34f152f36d970d4ff78abe53f563555a8e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Thu, 19 Dec 2019 13:26:44 +0000 Subject: [PATCH] [FIX] web: tests: nextTick awaits render with requestAnimationFrame Revision on [1]. Commit above made changes to test helper `nextTick()`, so that it waits for `window.nextAnimationFrame()` (shorten, 'rAF'). This was useful to wait for OWL rendering, since OWL renders on next animation frame. From a theoretical standpoint, the helper should wait at least as much time as before, as it adds either no slowdown (rAF is readily available), or a few milliseconds wait (rAF waits for next available animation frame). However, some old tests show non-deterministic behaviours, resulting to crashes [2]. It suggested, for some reasons, that `nextTick()` did not wait long enough for re-render... Which is surprising considering explanation above! Some of these tests have been fixed, assuming it was caused by un- guaranteed order of successive `setTimeout(0)` [3]. Unfortunately, this does not explain why it never crashed before the change on `nextTick()`... After investigation, we think that this behaviour comes from rAFs having their own task queue, which no longer guarantees that queued tasks happen after queued micro-tasks: When the requested next animation frame is readily available, the callback of `requestAnimationFrame()` is immediately called and waits for `setTimeout(0)`. The `setTimeout(0)` await is resolved before any other queued async micro-tasks, so that `nextTick()` is resolved sooner than expected. This would explain how `nextTick()` does not wait long enough for rendering from the loads of resolved Promises, even though there is a promise resolution after `setTimeout(0)`. This commit fixes the issue by moving the await `setTimeout(0)` before the rAF call, so that it correctly waits render from chain of resolved promises (since the `setTimeout(0)` is awaited in the same main task queue). [1] https://github.com/odoo/odoo/commit/b0941d19a07b08fabc64a284b58e57edc46203dd [2] http://runbot.odoo.com/runbot/build/771505 and http://runbot.odoo.com/runbot/build/752855 [3] https://github.com/odoo/odoo/commit/6cbfdbce2d0f1b567b6a6d6f637fe2a476980acc closes odoo/odoo#42123 Signed-off-by: VincentSchippefilt --- addons/web/static/tests/helpers/test_utils_dom.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/addons/web/static/tests/helpers/test_utils_dom.js b/addons/web/static/tests/helpers/test_utils_dom.js index e18726138d1..2d9ed650dd8 100644 --- a/addons/web/static/tests/helpers/test_utils_dom.js +++ b/addons/web/static/tests/helpers/test_utils_dom.js @@ -13,19 +13,17 @@ var concurrency = require('web.concurrency'); */ /** - * Returns a promise that will be resolved after the tick after the - * nextAnimationFrame + * Returns a promise that will be resolved after the nextAnimationFrame after + * the next tick * * This is useful to guarantee that OWL has had the time to render * * @returns {Promise} */ function returnAfterNextAnimationFrame() { - return new Promise((resolve) => { - window.requestAnimationFrame(async () => { - await concurrency.delay(0); - resolve(); - }); + return new Promise(async resolve => { + await concurrency.delay(0); + window.requestAnimationFrame(resolve); }); }