Closed
Bug 1415734
Opened 7 years ago
Closed 7 years ago
Make test_composite.html and test_missing-keyframe-on-compositor work with conformant Promise handling on Android
Categories
(Core :: DOM: Animation, defect)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: hiro, Assigned: hiro)
References
Details
Attachments
(1 file)
It did not appear in Bevis' try in bug 1193394 comment 48, but it failed on Android.
https://treeherder.mozilla.org/logviewer.html#?job_id=143136809&repo=try&lineNumber=2106
It seems waitForDocumentLoad() was timed out. I have no clue yet.
Assignee | ||
Comment 1•7 years ago
|
||
test_missing-keyframe-on-compositor.html also uses waitForDocumentLoad(). This test is mochitest-3 on Android, I guess.
Summary: Make test_composite.html work with conformant Promise handling → Make test_composite.html and test_missing-keyframe-on-compositor work with conformant Promise handling
Assignee | ||
Comment 2•7 years ago
|
||
It seems that opening a new window and waiting the new window loaded is a problematic one?
Assignee | ||
Comment 3•7 years ago
|
||
After looking into the test closely, I noticed that the test does not handle test refresh mode properly. I have no idea why it's been working without any problems.
Assignee | ||
Comment 4•7 years ago
|
||
Apart from the mis-usage of test refresh mode, on Android it seems that promise_test proceeds the next promise_test even if the previous promise_test has not finished yet.
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> Apart from the mis-usage of test refresh mode, on Android it seems that
> promise_test proceeds the next promise_test even if the previous
> promise_test has not finished yet.
This is wrong. promise_test works properly there.
It seems our test code using test refresh mode rely on non-conformant Promise handling. test_animations_omta.html also fails on Android.
What we should do is to advance the time before waiting for a paint. This is what I noticed in comment 3.
Assignee | ||
Comment 6•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Summary: Make test_composite.html and test_missing-keyframe-on-compositor work with conformant Promise handling → Make test_composite.html and test_missing-keyframe-on-compositor work with conformant Promise handling on Android
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 7•7 years ago
|
||
The try looks good. Though I did forget to add advanceTimeAndRefresh in some places. e.g. https://hg.mozilla.org/try/rev/4944bacafb87d9aae4facecb4d41ea2126d8c450#l2.281 , and I have no idea it worked fine. Anyway I will put advanceTimeAndRefresh there too. (What I don't quite understand is why the test have been working though.)
Another try without the conformant Promise handling.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2994eb4c50203d8b7d8721bd5af9d6de105307da
Assignee | ||
Comment 8•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8927024 [details]
Bug 1415734 - Don't start test refresh mode if there is any suppressed paints.
https://reviewboard.mozilla.org/r/198222/#review203650
::: commit-message-41a66:10
(Diff revision 1)
> +So what happened in the failure case;
> +
> +1) In the first promise_test we wait for document load
> +2) The paintingSuppressed flag is set in the first promise_test and create the
> + timer
> +3) When the document has been loaded but the timer has not yet been fired,
> + start test refresh mode and create an element in the subsequent promise_test
> +4) Creating the element triggers a reflow
> +5) waitForPaints() is called
> +6) The timer is fired, but there is a pending reflow, so skip clearing the flag
> +7) Now it's in the test refresh mode, the pending flow will never be processed
> + until some triggers happen (i.e. mouse movement, calling
> + advanceTimeAndRefresh, etc.)
> +8) The test is timed out
Thank you for this detailed explanation and analysis!
Attachment #8927024 -
Flags: review?(bbirtles) → review+
Comment 11•7 years ago
|
||
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e4af3344015d
Don't start test refresh mode if there is any suppressed paints. r=birtles
Comment 12•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•