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)

defect
Not set
normal

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.
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
It seems that opening a new window and waiting the new window loaded is a problematic one?
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.
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.
No longer blocks: 1193394
(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.
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
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
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+
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
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.

Attachment

General

Created:
Updated:
Size: