At the moment, we run the <iframe mozbrowser> tests OOP on *nix and in-process on Windows. With bug 761939 fixed, we should be able to run the tests OOP on Windows. We should rejigger the test framework so we run all the tests both OOP and in-process on both Windows and *nix.

These patches work, but they rely on bug 757182. Since the patches will rot as soon as anyone lands a test or test change to mozbrowser, I'm not going to mark for review until all the dependencies are met here. Although this is a lot of code churn, this should be a simple review; the changes are all mechanical.

Comment on attachment 630787 [details] [diff] [review] Bug 762049 - Part 1: Rename test_browserFrame{1,2,3}.html. All dependencies landed (\o/). Mounir, would you like to do the honors?

Comment on attachment 630788 [details] [diff] [review] Bug 762049 - Part 2: Make remaining tests both in-process and OOP. The technique here is to create three files for every test: 1. test_browserElement_oop_Foo.html 2. test_browserElement_inproc_Foo.html 3. browserElement_Foo.js Files (1) and (2) are identical, and all of the test code is in (3). browserFrameHelpers (renamed to browserElementTestHelpers) sets the in/out-of process pref according to the filename. I didn't do this for the tests touched in part 1 because those tests are checking the security prefs, which should work the same in and out-of process. I didn't modify any of the test code, except for: * one test which relied on strict mode not being set; I fixed the test so we can run it with strict mode. * two tests which relied on an <iframe> being present in the html file. I want these html files to be mere shells, and it was easy enough to use appendElement to the same effect. I'm going to post a fourth patch adding PD licenses to all these files.

Comment on attachment 630789 [details] [diff] [review] Bug 762049 - Part 3: Rename browserFrameHelpers to browserElementTestHelpers. Slowly getting rid of "browser frame" in favor of "browser element", which I suspect will get a fast r+ from you. :)

Comment on attachment 630789 [details] [diff] [review] Bug 762049 - Part 3: Rename browserFrameHelpers to browserElementTestHelpers. Review of attachment 630789 [details] [diff] [review]: ----------------------------------------------------------------- r=me, thank you sed.

Comment on attachment 630788 [details] [diff] [review] Bug 762049 - Part 2: Make remaining tests both in-process and OOP. Review of attachment 630788 [details] [diff] [review]: ----------------------------------------------------------------- r=me You could have given nicer <title> to your tests as long as you were here but that's not very important I believe ;) ::: dom/browser-element/mochitest/test_browserFramePromptConfirm.html @@ +82,5 @@ > </scr' + 'ipt></body></html>'; > } > > runTest(); > + nit: remove that empty line.

Comment on attachment 631038 [details] [diff] [review] Part 4: Add CC0 license to all test JS files. Review of attachment 631038 [details] [diff] [review]: ----------------------------------------------------------------- r=me, assuming you didn't forget a file. BTW, why not html files?

> BTW, why not html files? Mostly because most of our HTML mochitests don't have licenses. Is that a thing we're doing these days?

(In reply to Justin Lebar [:jlebar] from comment #12) > > BTW, why not html files? > > Mostly because most of our HTML mochitests don't have licenses. Is that a > thing we're doing these days? AFAIK, we don't. But I doubt a lot of people take time to put licenses boilerplate in tests in general.

Landed for FF16. https://hg.mozilla.org/integration/mozilla-inbound/rev/840a820e589c https://hg.mozilla.org/integration/mozilla-inbound/rev/b40369a421bd https://hg.mozilla.org/integration/mozilla-inbound/rev/e129ca091029 https://hg.mozilla.org/integration/mozilla-inbound/rev/dd157a5da52f

And backed out because of Windows M2 bustage (I didn't notice on try because it manifests as a crash *after* all the browser element tests run). Windows OOP is apparently not all it's cracked up to be. But I still want this change, because it makes testing on *nix much better. I'll disable the OOP tests on Windows and file a follow-up bug for figuring out what's going on there. https://hg.mozilla.org/integration/mozilla-inbound/rev/2982a120fef5 https://hg.mozilla.org/integration/mozilla-inbound/rev/7219414f43ce https://hg.mozilla.org/integration/mozilla-inbound/rev/5d64c6f58785 https://hg.mozilla.org/integration/mozilla-inbound/rev/e4c8f23e0f49

It seems this also caused a gc crash during shutdown of a debug b2g desktop build. I just tried with the backout again and it doesn't crash any more.

(In reply to Gregor Wagner [:gwagner] from comment #17) > It seems this also caused a gc crash during shutdown of a debug b2g desktop > build. > I just tried with the backout again and it doesn't crash any more. Are you sure? This was a test-only change.

(In reply to Justin Lebar [:jlebar] from comment #18) > (In reply to Gregor Wagner [:gwagner] from comment #17) > > It seems this also caused a gc crash during shutdown of a debug b2g desktop > > build. > > I just tried with the backout again and it doesn't crash any more. > > Are you sure? This was a test-only change. Ah wrong folder... It's still crashing :(

> Ah wrong folder... It's still crashing :( You mean, it crashes without this change too? Could you please file a bug?

New try push: https://tbpl.mozilla.org/?tree=Try&rev=20fac72a35ae

> And backed out because of Windows M2 bustage Here's a link to a busted log: https://tbpl.mozilla.org/php/getParsedLog.php?id=12465092&tree=Mozilla-Inbound

Comment on attachment 631537 [details] [diff] [review] Part 5: Don't use OOP on Windows. Review of attachment 631537 [details] [diff] [review]: ----------------------------------------------------------------- r=me even you keep your highly verbose and not clearer version ;) ::: dom/browser-element/mochitest/browserElementTestHelpers.js @@ +127,3 @@ > var oop; > +if (navigator.platform.indexOf('Win') != -1 || > + location.pathname.indexOf("_inproc_") != -1) { I would have done something like: // Enable or disable OOP depending on the test's filename. oop = location.pathname.indexOf("_inproc_") == -1; // Always disable OOP on Windows (bug 763081). oop = navigator.platform.indexOf('Win') != -1 ? false : oop;

> r=me even you keep your highly verbose and not clearer version ;) You're right, mine is pretty ugly, isn't it?

Landed again for FF16: https://hg.mozilla.org/integration/mozilla-inbound/rev/f9f138abe4c3 https://hg.mozilla.org/integration/mozilla-inbound/rev/3a827c12da43 https://hg.mozilla.org/integration/mozilla-inbound/rev/830251d84ccd https://hg.mozilla.org/integration/mozilla-inbound/rev/a53c96e72649 https://hg.mozilla.org/integration/mozilla-inbound/rev/ee3d46a285e6

https://hg.mozilla.org/mozilla-central/rev/f9f138abe4c3 https://hg.mozilla.org/mozilla-central/rev/3a827c12da43 https://hg.mozilla.org/mozilla-central/rev/830251d84ccd https://hg.mozilla.org/mozilla-central/rev/a53c96e72649 https://hg.mozilla.org/mozilla-central/rev/ee3d46a285e6