1366441 - Permaorange in test_animation_observers_sync.html when Gecko 55 merges to mozilla-beta on 2017-06-12

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect)

Description

[Phil Ringnalda (:philor)]

Apparently https://hg.mozilla.org/mozilla-central/rev/2a0949cc856f added the first use of anything behind dom.animations-api.core.enabled to test_animation_observers_sync.html, since it was passing in simulations of trunk as though it was already merged to beta until that landed, and now it fails like https://treeherder.mozilla.org/logviewer.html#?job_id=100611558&repo=try. Looks like now it needs to set it like the other tests in that directory do.

[Tracking Requested - why for this release]: merge bustage which would result in a closed tree and a delayed beta 1.

Comment 1

[Boris Chiou [:boris]]

Thanks for catching this. We should enable dom.animations-api.core.enabled in this test case.

Comment 2

[Boris Chiou [:boris]]

Attachment: Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

Review commit: https://reviewboard.mozilla.org/r/141260/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141260/

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

https://reviewboard.mozilla.org/r/141260/#review144856

I don't think we need to open a sub window in this test case, since it runs on chrom privilege.
We can just call pushPrefEnv just like other tests (e.g. test_animation_properties.html).

Comment 4

[Boris Chiou [:boris]]

Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141260/diff/1-2/

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

https://reviewboard.mozilla.org/r/141260/#review144860

Thank you for the quick fix!

::: dom/animation/test/chrome/test_animation_observers_sync.html:1000
(Diff revision 2)
> -  assert_equals_records(observer.takeRecords(),
> +    assert_equals_records(observer.takeRecords(),
> -    [{ added: [], changed: [], removed: [anim] }],
> +      [{ added: [], changed: [], removed: [anim] }],
> -    "records after animation is finished");
> +      "records after animation is finished");
> -}, "exclude_animations_targeting_pseudo_elements");
> +  }, "exclude_animations_targeting_pseudo_elements");
> +}
> +

We need to call setup({explicit_done: true}) here since pushPrefEnv() is asynchronous call.  This is the reason why done() is called after runTest()?
Oh, I just realized some tests in dom/animation/test/chrome/ don't use explicit_done at all..

Comment 6

[Boris Chiou [:boris]]

Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

https://reviewboard.mozilla.org/r/141260/#review144860

> We need to call setup({explicit_done: true}) here since pushPrefEnv() is asynchronous call.  This is the reason why done() is called after runTest()?
> Oh, I just realized some tests in dom/animation/test/chrome/ don't use explicit_done at all..

Yes. That's why I didn't add it because I just copy from them. Anyway, I will add setup({explicit_done: true}). Thanks a lot.

Comment 7

[Boris Chiou [:boris]]

Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141260/diff/2-3/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df9c4ed9ec08
Enable dom.animations-api.core.enabled for test_animation_observers_sync.html. r=hiro

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/df9c4ed9ec08