1261158 - Convert image/test/mochitest/chrome.ini mochitests to something other than mochitest-chrome where appropriate

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

The tests in image/test/mochitest/chrome.ini generally look like they should run in both the parent and child process in an e10s environment. However, they are currently chrome mochitests, and so the never really run in the child process. We should convert them to mochitest-plain or browser-chrome tests instead.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Convert test_animSVGImage.html

This one was relatively easy

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Convert test_animSVGImage2.html

Little bit harder, but SpecialPowers.wrap to the rescue!

Comment 3

[Kartikaya Gupta (email:[e-mail])]

test_animation.html and test_animation2.html are disabled, so I left those alone. test_background_image_anim.html and test_bullet_animation.html were easy to convert. test_bug415761.html seems a bit harder, since it does some stuff with file copying, and I'm not sure how that will work with http:// URIs instead of chrome:// URIs. It's probably easier to convert it to a reftest instead.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3 - Convert test_background_image_anim.html

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 4 - Convert test_bullet_animation.html

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 5 - Convert test_changeOfSource.html

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 6 - Convert test_changeOfSource2.html

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 7 - Convert test_has_transparency.html

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 8 - Convert test_net_failedtoprocess.html

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 9 - Convert test_removal_ondecode.html

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 10 - Convert test_removal_onload.html

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 11 - Convert test_staticClone.html

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 12 - Convert test_svg_animatedGIF.html

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 13 - Convert test_svg_filter_animation.html

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 14 - Convert test_synchronized_animation.html

Comment 16

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 15 - Convert test_xultree_animation.html

Surprisingly, embedding XUL into these tests works just fine.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 16 - Convert test_bug1132427.html

In this one I removed a load event listener from the main test file because the helper file (the one that is window.open'd) already has a onload=".." that does the same thing. This was causing the test to get run twice, and fail with a "finish() was called twice" error.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4068af45fe9

Comment 19

[Kartikaya Gupta (email:[e-mail])]

I can repro the test_xultree_animation failure when I run that test in isolation - somehow there is a JS error in textbox.xml which causes the code at [1] to call finish() again. This happens during shutdown *after* the test is done running. I'm investigating.

[1] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js?rev=b78ed68af2fb#1587

Comment 20

[Kartikaya Gupta (email:[e-mail])]

With the patch on bug 1261509 (which is needed to make test_xultree_animation.xhtml pass) the try push is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=170818a31237&group_state=expanded. I also updated the patches to remove some of my requestFlakyTimeout additions since they turned out to be unneccessary.

Comment 21

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Convert test_animSVGImage.html

Comment 22

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Convert test_animSVGImage2.html

Comment 23

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3 - Convert test_background_image_anim.html

Comment 24

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 4 - Convert test_bullet_animation.html

Comment 25

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 5 - Convert test_changeOfSource.html

Comment 26

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 6 - Convert test_changeOfSource2.html

Comment 27

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 7 - Convert test_has_transparency.html

Comment 28

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 8 - Convert test_net_failedtoprocess.html

Comment 29

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 9 - Convert test_removal_ondecode.html

Comment 30

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 10 - Convert test_removal_onload.html

Comment 31

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 11 - Convert test_staticClone.html

Comment 32

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 12 - Convert test_svg_animatedGIF.html

Comment 33

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 13 - Convert test_svg_filter_animation.html

Comment 34

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 14 - Convert test_synchronized_animation.html

Comment 35

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 15 - Convert test_xultree_animation.html

Comment 36

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 16 - Convert test_bug1132427.html

Comment 37

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8737755 [details] [diff] [review]
Part 2 - Convert test_animSVGImage2.html

>+[test_animSVGImage2.html]
>+skip-if = buildapp == 'b2g' || os == 'android'

I'm curious, why do we need to skip on b2g and android?

Comment 38

[Kartikaya Gupta (email:[e-mail])]

(In reply to Timothy Nikkel (:tnikkel) from comment #37)
> I'm curious, why do we need to skip on b2g and android?

Well in the chrome.ini it has the skip-if in the DEFAULT section at the top of the file, so I carried it over into mochitest.ini. I suspect chrome mochitests are generally disabled on Fennec/B2G because their architecture is so different. We could probably enable these tests for Fennec at least as plain mochitests, but I didn't want to do that as part of this bug because it's expanding the scope. We can do that as a follow-up if you want.

Comment 39

[Timothy Nikkel (:tnikkel)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #38)
> Well in the chrome.ini it has the skip-if in the DEFAULT section at the top
> of the file, so I carried it over into mochitest.ini.

Oh, I didn't see that.

Comment 40

[Timothy Nikkel (:tnikkel)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> test_animation.html and test_animation2.html are disabled, so I left those
> alone.

I did a try push with all disabled imagelib tests re-enabled. These two didn't fail, same with test_undisplayed_iframe.html. I'm triggering a bunch more jobs to see if they fail intermittently.

Comment 41

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ff0145cecf4
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7e599be4a03
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3da4bfcf9cc
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb3da962e9a1
https://hg.mozilla.org/integration/mozilla-inbound/rev/1efd3457c427
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f047928cb92
https://hg.mozilla.org/integration/mozilla-inbound/rev/9609b68d0358
https://hg.mozilla.org/integration/mozilla-inbound/rev/134a4dbaad4b
https://hg.mozilla.org/integration/mozilla-inbound/rev/916c6799c2bb
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae6093a8aeaf
https://hg.mozilla.org/integration/mozilla-inbound/rev/67dacff9db6e
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdfe660df0e6
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1b4e8a0d9e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/46746eac69d5
https://hg.mozilla.org/integration/mozilla-inbound/rev/3911b874e8d1
https://hg.mozilla.org/integration/mozilla-inbound/rev/73b3beeddb7b

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6ff0145cecf4
https://hg.mozilla.org/mozilla-central/rev/a7e599be4a03
https://hg.mozilla.org/mozilla-central/rev/a3da4bfcf9cc
https://hg.mozilla.org/mozilla-central/rev/bb3da962e9a1
https://hg.mozilla.org/mozilla-central/rev/1efd3457c427
https://hg.mozilla.org/mozilla-central/rev/8f047928cb92
https://hg.mozilla.org/mozilla-central/rev/9609b68d0358
https://hg.mozilla.org/mozilla-central/rev/134a4dbaad4b
https://hg.mozilla.org/mozilla-central/rev/916c6799c2bb
https://hg.mozilla.org/mozilla-central/rev/ae6093a8aeaf
https://hg.mozilla.org/mozilla-central/rev/67dacff9db6e
https://hg.mozilla.org/mozilla-central/rev/cdfe660df0e6
https://hg.mozilla.org/mozilla-central/rev/c1b4e8a0d9e1
https://hg.mozilla.org/mozilla-central/rev/46746eac69d5
https://hg.mozilla.org/mozilla-central/rev/3911b874e8d1
https://hg.mozilla.org/mozilla-central/rev/73b3beeddb7b

Comment 43

[Timothy Nikkel (:tnikkel)]

Re-enabled the three disabled mochitest-chrome tests in bug 1262269. They will need to be converted too, I'll do that in another bug.

Comment 44

[Kartikaya Gupta (email:[e-mail])]

Thanks! I also uplifted these patches to aurora since the e10s folks prefer that any e10s-related test changes be uplifted to 47 as well.

https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=f5f4bbaf8f02