The current version of Gaia causes some mochitests to fail: 15:50:30 INFO - 5 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/alarm/test/test_alarm_non_permitted_app.html | navigator.mozAlarms should return null - got [xpconnect wrapped nsIDOMMozAlarmsManager], expected null 16:31:52 INFO - 3901 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/power/test/test_power_basics.html | Shouldn't be able to access power manager without permission. 16:38:43 INFO - I/GeckoDump( 826): 5 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/alarm/test/test_alarm_non_permitted_app.html | navigator.mozAlarms should return null - got [xpconnect wrapped nsIDOMMozAlarmsManager], expected null 16:38:45 INFO - I/GeckoDump( 826): 3901 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/power/test/test_power_basics.html | Shouldn't be able to access power manager without permission. 16:38:46 ERROR - F/libc ( 794): Fatal signal 11 (SIGSEGV) at 0x6f72706c (code=1) 16:38:46 ERROR - This usually indicates the B2G process has crashed https://tbpl.mozilla.org/php/getParsedLog.php?id=19969020&tree=Mozilla-Inbound&full=1 Because of this problem, we can't update the emulator used in TBPL; we're forced to use an old emulator which has a version of Gaia that doesn't cause this conflict (commit 3df466a3a11c898adc1874c59784ce83a3bab300). This was also seen in bug 841836. Not being able to update the emulator will prevent other fixes from landing that require new emulator builds.

overholt, scravag, I'm not sure if this a Gaia issue or a DOM issue...can you get someone to investigate? This is blocking a couple of other issues and will likely start blocking more soon.

Andrew, is this something you can get someone to look at? It's blocking more and more things from landing, since it prevents us from updating the emulator that's being used in TBPL.

Thanks for the needinfo (sorry, I missed the CC). I'll find someone.

I've asked Andrea to take a look.

David, this sounds like exactly the thing you were working with Luqman on today. Any comments?

So David and I were trying to figure out yesterday why the alarm tests were failing a und it came down how they were using the SpecialPowers permissions functions: > SpecialPowers.addPermission > SpecialPowers.removePermission So as far as I can tell, once a permission is set with those functions it won't be propagated to the already initialized DOM objects. I know this is true of the AlarmsApi which seems to only check the permission on page load. http://mxr.mozilla.org/mozilla-central/source/dom/alarm/AlarmsManager.js#149 The approach we took to get around this was to add a way to also check permissions with SpecialPowers and wrote a little helper which would take a callback, wrap it in an if statement for actually testing if you have the permission you want set and if not setting it then reloading the page.

Actually, looking at the power API test, it already seems to do the set-then-reload trick. The reason it's probably failing is because it expects to start in a state where the 'power' permission is not set. AFAIK, mochitests on b2g run initially with all permissions enabled. A quick fix could be to just reverse the 2 tests or remove the permission at the start of the test.

Sounds like this might be related to the decision to have B2G mochitests run with wide-open permissions so that functional tests weren't hamstrung. At that time, we also discussed the need to have a mode when it ran with closed permissions so that permissions tests had the proper fixture to run from. I think the proposal with the most legs was actually two distinct mochitest runs, one under a permissive manifest and one under a conservative manifest.

This sounds like the exact same problem that's hitting bug 826058, namely that nobody ever implemented bug 685652. addPermission and setXPreference are footgun APIs in SpecialPowers, unfortunately.

footgun only with OOP enabled, that is. If OOP isn't in play here, you can ignore comment 10.

The confusing thing about this is that it is apparently sensitive to Gaia. With https://github.com/mozilla-b2g/gaia/commit/3df466a3a11c898adc1874c59784ce83a3bab300 (from 25 days ago), this problem doesn't occur, but with current Gaia it does. There have not been any changes to the mochitest test container app in that timeframe.

geo: all is as described by luqman, would recommend we work on a quick patch to fix this functionality. I'll see if i can work on a patch to get these running as is

David, Sure, agreed re: this fix. I can spin out the other discussion, but there will be a need to settle what the start-state of a B2G mochitest is and at least communicate it. We'll also need a way to start a mochitest w/o all permissions set, if we're to do valid perms testing.

Also, comment 12 is salient. This change went in awhile back, so I'm also curious what else changed to make the failures surface.

Previous patch did not include the new file under power/test/

Comment on attachment 720792 [details] [diff] [review] power & alarm api tests that should work regardless of the state of the mochitest app Review of attachment 720792 [details] [diff] [review]: ----------------------------------------------------------------- Thanks David! Looks great! I don't have strong opinion but just wondering why the file names are slightly different: test_power_non_permitted.html test_alarm_non_permitted_app.html Just a picky reminder. It's up to you. :) Btw, please remove the temporal dump() functions and run the test before check-in. Thanks!

Is there a general ETA on this at this point? It's blocking DOM performance work from going forward...

Boris, does this need to land on the v1 train ?

If this bug is well under way, can't we just temporarily disable the affected tests? Assuming this is still a while away from completion.

Looked like things are more or less done, with an r+-with-changes. It's a question of landing now.

Comment on attachment 722450 [details] [diff] [review] mercurial patch with fixes. NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): 843893 User impact if declined: mochitest run won't pass Testing completed: mochitest fixes Risk to taking this patch (and alternatives if risky): 0 String or UUID changes made by this patch:

We're waiting for review before considering for uplift.

Gene - does your r+ carry over from comment 18?

> Boris, does this need to land on the v1 train ? For my purposes it just needs to land in whatever we run on tbpl for inbound/m-c/try.... I have DOM patches that expose preexisting bugs in gaia that are now fixed but the snapshot we use on tbpl is still buggy and can't be upgraded due to this bug.

Comment on attachment 722450 [details] [diff] [review] mercurial patch with fixes. Sorry for the delayed response. I'm just aware of this. Asking for approval-mozilla-b2g-18 doesn't need to get a review+ again if the patch used to get a review+ before. Marking review+ anyway.

Needs to land on v1-train, trunk

guys, we need to fix this asap. It is blocking work.

Pushed to try along with the latest emulator (to verify that the alarm and power API failures go away): https://tbpl.mozilla.org/?tree=Try&rev=cffc4618bf7f Pushed a second version to try running against the current emulator to make sure it's harmless in this context (so we can land independently of an emulator update): https://tbpl.mozilla.org/?tree=Try&rev=70011fc56183

Comment on attachment 722450 [details] [diff] [review] mercurial patch with fixes. Review of attachment 722450 [details] [diff] [review]: ----------------------------------------------------------------- This patch adds "test_power_non_permitted_app.html" to Makefile.in, but doesn't actually add the file itself.

Comment on attachment 726390 [details] [diff] [review] second try Review of attachment 726390 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, I'll run this through try again.

pushed to try with current emulator: https://tbpl.mozilla.org/?tree=Try&rev=7900b93ec575

And pushed to try with a new emulator: https://tbpl.mozilla.org/?tree=Try&rev=a60c10e052c7

(In reply to Jonathan Griffin (:jgriffin) from comment #36) > And pushed to try with a new emulator: > https://tbpl.mozilla.org/?tree=Try&rev=a60c10e052c7 There are failures related to the new emulator, but none in alarm and power API's.

(In reply to Jonathan Griffin (:jgriffin) from comment #35) > pushed to try with current emulator: > https://tbpl.mozilla.org/?tree=Try&rev=7900b93ec575 This passed too. This means we can land this patch now, prior to an emulator update.

Thanks John.

This also needs to land on b2g18 as well.

Comment on attachment 726390 [details] [diff] [review] second try NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed: Risk to taking this patch (and alternatives if risky): We need to take this patch as a precondition for updating the emulator used in TBPL to run tests on mozilla-b2g18. Without this patch, we are stuck with an old emulator. This patch only changes some mochitests, so there is no risk. String or UUID changes made by this patch:

This bug was already b2g18+, it is on the previous patch.

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/42caf4845677 - at least, so far, Linux and Mac agree that the changes to test_power_basics.html make power not ok().

Phil: why do power tests get run on linux / mac again ? do you have a view into what those results were so i can try to repro. Jgriffin: i think the last try build did not run the the power and alarm tests.. do you know why ?

(In reply to dclarke@mozilla.com [:onecyrenus] from comment #45) > Phil: > > why do power tests get run on linux / mac again ? do you have a view into > what those results were so i can try to repro. > We need to prevent these tests from running on desktop, either via Makefile.in or in the tests themselves. > Jgriffin: > > i think the last try build did not run the the power and alarm tests.. do > you know why ? It seems they are excluded in http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/b2g.json due to earlier test failures.

jgriffin: remixing a new patch to only run when running as gonk, and will remove from b2g.json file so we actually see the result.

Need to verify that b2g.json has been disabled, the tests run on gonk but do not run on linux, mac, windows.

Latest version pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=1cf991e7948e

The alarm section passed in the previous run, just one test failed for power, fixed. Verified on my machine.

https://bugzilla.mozilla.org/show_bug.cgi?id=853206 still waiting for tryserver access to come back online for the moment

If you want to push this to try yourself, the correct syntax is: try: -b o -p ics_armv7a_gecko,linux64,macosx64,win32 -u mochitests -t none

Comment on attachment 727534 [details] [diff] [review] updated patch I don't know the details of how the power API works or should be tested, so I'm punting this review to someone who might.

https://tbpl.mozilla.org/?tree=Try&rev=3faef0b34234 I had kicked this off last night. Is it sufficient ?

Comment on attachment 727534 [details] [diff] [review] updated patch Review of attachment 727534 [details] [diff] [review]: ----------------------------------------------------------------- This doesn't need a re-review, it's a one line change, and doesn't change the semantics of the test.

Did you push the right patch to try? It looks like it can't find test_power_non_permitted_app.html: 02:07:43 ERROR - make[8]: *** No rule to make target `test_power_non_permitted_app.html', needed by `libs'. Stop.

https://tbpl.mozilla.org/?tree=Try&rev=46b0a2692145 Finally got the patch right, lots of goofs

Basically it looks like a few days ago moz-central was changed for alarm api. I have made it so the power api does not run on desktop, alarm however runs across all, and passes.

Try run was green, but it didn't include any Android runs. I'm thinking we should check it there too.

just kicked off a run: https://tbpl.mozilla.org/?tree=Try&rev=daaac9ca1159

try seems to have passed, but random orange showed up on unrelated testcase.

Comment on attachment 722450 [details] [diff] [review] mercurial patch with fixes. Please get approval on the patch that was actually landed if you want this uplifted to b2g18.