animations/animation-ready-reject-script-forbidden.html times out often. |
||||||
Issue description
animations/animation-ready-reject-script-forbidden.html times out often.
It could be due to the fact that the transition is expected to last 2s.
#element {
transition: transform 2000ms;
}
Filing a bug. Marking it as so for now.
,
Nov 24 2017
,
Nov 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/97820036ee151c27ab83dd9fde90ebdd46fdc984 commit 97820036ee151c27ab83dd9fde90ebdd46fdc984 Author: Naina Raisinghani <nainar@chromium.org> Date: Fri Nov 24 00:19:58 2017 Mark animations/animation-ready-reject-script-forbidden.html as timeout TBR=meade@chromium.org Bug: 788042 Change-Id: I63f54ee54eaf6cbc598db3e51c5b4c19ce3dbe78 Reviewed-on: https://chromium-review.googlesource.com/788472 Reviewed-by: nainar <nainar@chromium.org> Commit-Queue: nainar <nainar@chromium.org> Cr-Commit-Position: refs/heads/master@{#519033} [modify] https://crrev.com/97820036ee151c27ab83dd9fde90ebdd46fdc984/third_party/WebKit/LayoutTests/TestExpectations
,
Jan 31 2018
,
Jan 31 2018
Marking this as available. I am dropping all non P0/1 issues due to transitioning out of Chrome
,
Jun 11 2018
The reason that the test times out is because the ready function no longer throws (it ends up in the 'then' not the catch), so the test finish never happens. I attempted a bisect for when this behavior changed, but ended up with a big list (note that 'bad' here is catch() and 'good' is then()): You are probably looking for a change made after 521189 (known bad), but no later than 521379 (first known good). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/b1abba979fd58e1ba57fb4009517268dc64f1ce8..ad2869c08ee6eda404d8bb2d59705492785f969f Ran an internal bisect and got: https://chromium.googlesource.com/chromium/src/+log/3a13f63d2ad6f26c9aefdd2d2a7f8fa34571f23e..b71f5b7df345f1a8603b7cf7b59edf48288e7d96 Which points at: https://chromium.googlesource.com/chromium/src/+/b71f5b7df345f1a8603b7cf7b59edf48288e7d96 commit b71f5b7df345f1a8603b7cf7b59edf48288e7d96 Author: Naina Raisinghani <nainar@chromium.org> Date: Mon Dec 4 01:01:29 2017 +0000 Remove calls to styleForLayoutObject() in LayoutTreeBuilder::style() This patch stashes all ComputedStyle information about all descendants when an element will need to undergo Reattach. It does so by recursing over all ContainerNode, ShadowRoot and PseudoElement descendants in the case where the node itself needs to Reattach. We then use the ComputedStyle stashed on the Node in LayoutTreeBuilder::style() eliminating the need to call Element::styleForLayoutObject() in Layout Tree Construction. Bug: 595137 Change-Id: I7e6ca2431e602dff80d7d3e84d1bffe1e25d8b2c Reviewed-on: https://chromium-review.googlesource.com/765097 Commit-Queue: nainar <nainar@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#521240} It seems like for whatever reason, this makes the ready function in animations/animation-ready-reject-script-forbidden.html no longer fail, instead it resolves normally (this seems either wrong or scary, whether we reject or resolve should depend on the animation state and I don't get how the blamed CL could have changed that...). However chatting to adithyas@, he believes that the test wasn't particularly guided in its implementation - it was merely trying to get a promise to be rejected whilst in a ScriptForbiddenScope. adithyas - I don't suppose you know the 'normal' times we are inside a ScriptForbiddenScope in blink's lifecycle? If we can work that out, I can maybe put a proper test in place that definitely rejects.
,
Jun 11 2018
The main example I can think of is inside UpdateStyleAndLayoutTree (https://crrev.com/c8c387d2c5095b79f651ee5859b6b45e5281ffa5/third_party/blink/renderer/core/dom/document.cc#2092).
,
Jun 11 2018
After some digging I've been unable to find an obvious way to get the promise to be rejected whilst inside a ScriptForbiddenScope. Since the current test isn't actually testing anything anyway, I'm just going to remove it.
,
Jun 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/30c4e5bc3613142bb4024e7273432a82137177d4 commit 30c4e5bc3613142bb4024e7273432a82137177d4 Author: Stephen McGruer <smcgruer@chromium.org> Date: Mon Jun 11 20:44:35 2018 Remove useless animations LayoutTest This test was originally added to test a specific combination of a promise being rejected whilst in a ScriptForbiddenScope. However a long time ago the code changed so that the promise doesn't get rejected in that case, so the test hangs forever. Since its not testing anything anymore, remove it. Bug: 788042 Change-Id: Ifa5b9d04232f215c022bda7b482cab98760ac536 Reviewed-on: https://chromium-review.googlesource.com/1095646 Reviewed-by: Adithya Srinivasan <adithyas@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#566136} [modify] https://crrev.com/30c4e5bc3613142bb4024e7273432a82137177d4/third_party/WebKit/LayoutTests/TestExpectations [delete] https://crrev.com/91ccd3d10d970881a51bb9cb1497967ea674fac2/third_party/WebKit/LayoutTests/animations/animation-ready-reject-script-forbidden.html
,
Jun 11 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Nov 23 2017