Closed Bug 778072 Opened 12 years ago Closed 12 years ago

Use <iframe mozbrowser> in reftest harness

Categories

(Testing :: Reftest, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla17

People

(Reporter: cjones, Assigned: ahal)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
See also bug 777871, which is about the same thing, but for mochitest.
Assignee: nobody → ahalberstadt
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?
Attachment #651531 - Flags: feedback?(jones.chris.g)
> 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.
Attached patch Patch 2.0 - Added missing pref (obsolete) — Splinter Review
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.
Attachment #651531 - Attachment is obsolete: true
Attachment #651531 - Flags: feedback?(jones.chris.g)
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.
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
(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.
(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.
We don't run IPC reftests on win32.  I'm a little confused about your results ...
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?
Status: NEW → ASSIGNED
(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.
--> land as pref on for b2g
Attachment #651825 - Attachment is obsolete: true
Attachment #652782 - Flags: review?(jones.chris.g) → review+
Keywords: checkin-needed
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
Keywords: checkin-needed
hmm...the retriggers were green. I'll ask around about this failure and check in if it's known. Sorry for the churn.
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6945a923ce83
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: