Last Comment Bug 778072 - Use <iframe mozbrowser> in reftest harness
: Use <iframe mozbrowser> in reftest harness
Status: RESOLVED FIXED
:
Product: Testing
Classification: Components
Component: Reftest (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla17
Assigned To: Andrew Halberstadt [:ahal]
:
:
Mentors:
Depends on:
Blocks: b2g-reftest
  Show dependency treegraph
 
Reported: 2012-07-27 02:12 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-08-22 02:46 PDT (History)
5 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch 1.0 - use <iframe mozbrowser> (2.85 KB, patch)
2012-08-13 14:17 PDT, Andrew Halberstadt [:ahal]
no flags Details | Diff | Splinter Review
Patch 2.0 - Added missing pref (3.14 KB, patch)
2012-08-14 10:51 PDT, Andrew Halberstadt [:ahal]
no flags Details | Diff | Splinter Review
Patch 3.0 - Use pref to enable iframe mozbrowser in b2g (4.39 KB, patch)
2012-08-17 08:47 PDT, Andrew Halberstadt [:ahal]
cjones.bugs: review+
Details | Diff | Splinter Review

Description User image Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-27 02:12:36 PDT
Currently we use <browser remote>, but that's going to ship again in any foreseeable future, whereas <iframe mozbrowser> is.  <iframe mozbrowser> is also being used now to run OOP mochitests.

Since cross-process reftest/crashtest failures are soon only going to pertain to b2g, and b2g only uses <iframe mozbrowser>, I think it makes sense to change this globally, not just #ifdef B2G.
Comment 1 User image Justin Lebar (not reading bugmail) 2012-07-27 07:28:17 PDT
See also bug 777871, which is about the same thing, but for mochitest.
Comment 2 User image Andrew Halberstadt [:ahal] 2012-08-13 14:17:49 PDT
Created attachment 651531 [details] [diff] [review]
Patch 1.0 - use <iframe mozbrowser>

Is something like this what you are talking about? I'm not really sure which of the prefs should be enabled on all platforms vs b2g only (or if they aren't required at all). Things appear to run the same with or without most of them. Any tips for determining whether reftests are actually running in a separate process?
Comment 3 User image Justin Lebar (not reading bugmail) 2012-08-13 14:24:51 PDT
> Any tips for determining whether reftests are actually running in a separate process?

* Is iframe.contentWindow null?  If so, we're oop.
* Do you see a plugin-container process?  Is it using CPU?  If so, we're oop.
Comment 4 User image Andrew Halberstadt [:ahal] 2012-08-14 10:51:42 PDT
Created attachment 651825 [details] [diff] [review]
Patch 2.0 - Added missing pref

I'm now seeing a separate plugin container alongside the /system/b2g/b2g process. I was missing a pref to set remote to true, this should only be needed until bug 756376 lands.

Tree is currently closed, so I'll wait until I can push to try since I didn't #ifdef b2g.
Comment 5 User image Justin Lebar (not reading bugmail) 2012-08-14 11:03:08 PDT
See also bug 781355.
Comment 6 User image Andrew Halberstadt [:ahal] 2012-08-14 11:25:02 PDT
pushed to try: https://tbpl.mozilla.org/?noignore=1&tree=Try&rev=2e2df603ecac
Comment 7 User image Andrew Halberstadt [:ahal] 2012-08-15 07:11:47 PDT
I think the patch regresses reftests ;)

Rev3 WINNT 5.1:
REFTEST FINISHED: Slowest test took 3080ms (file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/413292-1.html)
REFTEST INFO | Result summary:
REFTEST INFO | Successful: 7391 (7372 pass, 19 load only)
REFTEST INFO | Unexpected: 437 (435 unexpected fail, 0 unexpected pass, 0 unexpected asserts, 0 unexpected fixed asserts, 2 failed load, 0 exception)
REFTEST INFO | Known problems: 296 (182 known fail, 0 known asserts, 67 random, 47 skipped, 0 slow)
REFTEST INFO | Total canvas count = 14
REFTEST TEST-START | Shutdown
INFO | automation.py | Application ran for: 0:45:15.578000

(Linux opt had 457 failures)

If this is really important to land, I can ifdef B2G it for now. I imagine many of the same tests will regress in B2G, but it's not like they are stable on that platform anyway.
Comment 8 User image Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 10:35:26 PDT
Sorry, I meant using <iframe mozbrowser remote> to replace <browser remote>, which isn't shipping anymore.

So
 - this should only affect OOP reftests
 - it's ok by me to switch between <browser remote> and <iframe mozbrowser remote> based on a pref
Comment 9 User image Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 10:35:42 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #8)
>  - it's ok by me to switch between <browser remote> and <iframe mozbrowser
> remote> based on a pref

... only on for b2g to start.
Comment 10 User image Andrew Halberstadt [:ahal] 2012-08-15 10:45:10 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #8)
> Sorry, I meant using <iframe mozbrowser remote> to replace <browser remote>,
> which isn't shipping anymore.

I thought that's what I was doing in the above patch?

> So
>  - this should only affect OOP reftests

The above patch causes about 450 failures (see https://tbpl.mozilla.org/?noignore=1&tree=Try&rev=2e2df603ecac). So I think we have some investigation to do.

>  - it's ok by me to switch between <browser remote> and <iframe mozbrowser
> remote> based on a pref

Works for me. I haven't run the full suite of tests on B2G yet, so I'm not sure what effect using <iframe mozbrowser> will have there yet, but I imagine there will be a similar number of regressions as on desktop firefox.
Comment 11 User image Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 11:12:18 PDT
We don't run IPC reftests on win32.  I'm a little confused about your results ...
Comment 12 User image Andrew Halberstadt [:ahal] 2012-08-16 12:12:10 PDT
Yeah, I have no idea what is going on. All I know is the above patch broke a large number of reftests on every platform. I think it would also break a large number of tests on B2G.

Should I go ahead and get this landed as a pref and enable it in B2G anyway? Or should I hold off until we figure out what is going on?
Comment 13 User image Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-16 12:36:26 PDT
(In reply to Andrew Halberstadt [:ahal] from comment #12)
> Should I go ahead and get this landed as a pref and enable it in B2G anyway?
> Or should I hold off until we figure out what is going on?

Yeah, let's do that.
Comment 14 User image Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-16 12:36:39 PDT
--> land as pref on for b2g
Comment 15 User image Andrew Halberstadt [:ahal] 2012-08-17 08:47:57 PDT
Created attachment 652782 [details] [diff] [review]
Patch 3.0 - Use pref to enable iframe mozbrowser in b2g
Comment 16 User image Ryan VanderMeulen [:RyanVM] 2012-08-21 03:27:36 PDT
This is still causing Android reftest failures. Please run your patch through Try before requesting checkin in the future.
https://tbpl.mozilla.org/php/getParsedLog.php?id=14553045&tree=Try&full=1
Comment 17 User image Ryan VanderMeulen [:RyanVM] 2012-08-21 04:54:59 PDT
hmm...the retriggers were green. I'll ask around about this failure and check in if it's known. Sorry for the churn.
Comment 18 User image Ryan VanderMeulen [:RyanVM] 2012-08-21 05:58:45 PDT
Apparently this is a known issue with some of the new tegras. Sorry for the confusion. Green on Try.
https://tbpl.mozilla.org/?tree=Try&rev=cfba471376c9
Comment 19 User image Ryan VanderMeulen [:RyanVM] 2012-08-21 15:26:43 PDT
(In reply to Ryan VanderMeulen from comment #18)
> Green on Try.
> https://tbpl.mozilla.org/?tree=Try&rev=cfba471376c9

https://hg.mozilla.org/integration/mozilla-inbound/rev/6945a923ce83
Comment 20 User image Ed Morley [:emorley] 2012-08-22 02:46:02 PDT
https://hg.mozilla.org/mozilla-central/rev/6945a923ce83

Note You need to log in before you can comment on or make changes to this bug.