Justin, is this something you can look into? Not sure how severe it is. https://tbpl.mozilla.org/php/getParsedLog.php?id=26218860&tree=Fx-Team#error13 b2g_emulator_vm fx-team opt test mochitest-3 on 2013-08-06 09:25:37 PDT for push 65220b0f2b68 slave: tst-linux64-ec2-308 09:57:49 INFO - 38684 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Got download progress 09:57:49 INFO - 38685 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Got download progress 09:57:49 INFO - 38686 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | App is installed 09:57:49 INFO - 38687 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Checking installed app 09:57:49 INFO - System JS : ERROR resource://gre/modules/BrowserElementParent.jsm:219 09:57:49 INFO - ReferenceError: DOMApplicationRegistry is not defined 09:57:49 INFO - ###################################### forms.js loaded 09:57:49 INFO - ############################### browserElementPanning.js loaded 09:57:49 INFO - ######################## BrowserElementChildPreload.js loaded 09:57:49 INFO - 38688 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Message from app: OK: Launched app 09:57:49 INFO - 38689 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | App is installed 09:57:49 INFO - 38690 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Message from app: OK: Really Rapid Release (cached) == Really Rapid Release (cached) - Manifest name should be correct 09:57:49 INFO - 38691 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Message from app: OK: http://test == http://test - App origin should be correct 09:57:49 INFO - 38692 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Message from app: OK: http://mochi.test:8888 == http://mochi.test:8888 - Install origin should be correct 09:57:49 INFO - 38693 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Version should be correct 09:57:49 INFO - 38694 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Messaging from app complete 09:57:49 INFO - 38695 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Setting callbacks 09:57:49 INFO - 38696 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Checking for updates 09:57:49 INFO - 38697 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | downloadapplied fired. 09:57:49 INFO - 38698 INFO TEST-KNOWN-FAIL | /tests/dom/apps/tests/test_app_update.html | lastUpdateCheck updated appropriately 09:57:49 INFO - -- webapps.js uninstall http://test/tests/dom/apps/tests/file_app.sjs?apptype=cached&getmanifest=true 09:57:49 INFO - 38699 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Uninstalled app 09:57:49 INFO - 38700 INFO TEST-PASS | /tests/dom/apps/tests/test_app_update.html | Checking uninstalled app 09:57:49 INFO - ************************************************************ 09:57:49 INFO - * Call to xpconnect wrapped JSObject produced this error: * 09:57:49 INFO - [Exception... "'[JavaScript Error: "DOMApplicationRegistry is not defined" {file: "resource://gre/modules/BrowserElementParent.jsm" line: 219}]' when calling method: [nsIObserver::observe]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "JS frame :: http://mochi.test:8888/tests/dom/apps/tests/test_app_update.html :: checkAppState :: line 249" data: yes] 09:57:49 INFO - ************************************************************ 09:57:49 INFO - ###################################### forms.js loaded 09:57:49 INFO - ############################### browserElementPanning.js loaded 09:57:49 INFO - ######################## BrowserElementChildPreload.js loaded

This is a bit odd; it looks like this means that importing Webapps.jsm didn't create the DOMApplicationRegistry object? Maybe we hit an exception loading Webapps.jsm and it's being swallowed? Fabrice might know more about this than me...

Is this intermittent? Indeed it looks like we fail either to load Webapps.jsm at all (the exception could come from one of the other jsm it imports), or DOMApplicationRegistry.init() throws. I've never hit that before.

I'm seeing it in every random log I've looked at. We're only noticing it now due to recent TBPL log parser improvements that landed (bug 892958).

Kyle, is it possible that our manual b2g compartment merging is somehow getting us into trouble here? BrowserElementParent.jsm does > XPCOMUtils.defineLazyGetter(this, "DOMApplicationRegistry", function () { > Cu.import("resource://gre/modules/Webapps.jsm"); > return DOMApplicationRegistry; > }); and then it references DOMApplicationRegistry in the BEP constructor...

I hesitate to say it's impossible, but it seems unlikely. I would not expect problems caused by compartment sharing to be intermittent either.

This isn't intermittent. It happens on every run. It just currently isn't a fatal exception.

2:25:27 PM - RyanVM|Sheriff: khuey: any more thoughts on bug 902165 (DOMApplicationRegistry not defined) 2:27:28 PM - khuey: RyanVM|Sheriff: it's possible, yes Back to you, jlebar! :)

I don't think I'm going to get to this before I leave on Friday. Sorry. :(

Back on to khuey's ever-growing heap then, I guess :-\

Kyle, I don't think we gain much by lazy loading the Webapps.jsm in the parent, since it will be loaded anyway quite early by shell.js. So if moving to a simple Cu.import() fixes this bug, r=me

Comment on attachment 797894 [details] [diff] [review] patch bizarre, but if it works, ship it!

That helps with the M3 errors, but that now triggers loading ActivitiesService.jsm in child processes, which doesn't end well. I have no idea yet how this can happen tough :(

Sorry to bug you Fabrice, but any news to report? :)

I finally found the root cause... BrowserElementParent.js creation was triggered by being in the app-startup category, but we create these components in both the parent and child processes. This is actually what you want sometimes, but definitely not in this case: the code ended up trying to load and run a bunch of code in the child that is parent only. The test failure was happening when trying to create a parent process manager in the child. The approach in this patch is to create a new category, named 'app-startup-parent'. Components that only need to run in the parent register themselves to this category. try run at: https://tbpl.mozilla.org/?tree=Try&rev=309f671b0cda

Comment on attachment 8339505 [details] [diff] [review] bep-in-parent-only.patch Tests don't look happy.

Moving that patch in the right bug. Try run at https://tbpl.mozilla.org/?tree=Try&rev=adbc5e34ff11

Comment on attachment 8395059 [details] [diff] [review] rebased patch try doesn't seem happy...

Comment on attachment 8395059 [details] [diff] [review] rebased patch Review of attachment 8395059 [details] [diff] [review]: ----------------------------------------------------------------- Let me know when you have a patch that passes try :)

Hey Fabrice, there was a missing parenthesis in your patch. I've added it and added a break in the RegisterBEP case. The patch looks green on try (so far): https://tbpl.mozilla.org/?tree=Try&rev=d7ba1d2a1577

/me facepalms

(In reply to comment #23) > /me facepalms Blame JS! :)

(In reply to Fabrice Desré [:fabrice] from comment #23) > /me facepalms :D Do you want to re-request review to bent? (or maybe someone else since he's on vacation)

Comment on attachment 8427342 [details] [diff] [review] Patch Review of attachment 8427342 [details] [diff] [review]: ----------------------------------------------------------------- r=me with this fixed: ::: dom/apps/src/Webapps.jsm @@ +1150,5 @@ > break; > case "Webapps:ReplaceReceipt": > this.replaceReceipt(msg, mm); > break; > + case "Webapps:RegisterBEP": I think we should make sure the app has the 'browser' permission before we allow this. If I'm confused and we always have one even for apps without this permission then we could at least make sure that it only does this once I think.

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #26) > I think we should make sure the app has the 'browser' permission before we > allow this. > > If I'm confused and we always have one even for apps without this permission > then we could at least make sure that it only does this once I think. TBH I'm not completely sure what that second comment means, but I did make a change to Fabrice's patch that checks for the "browser" permission, and the tests seem to pass - https://tbpl.mozilla.org/?tree=Try&rev=e9775a1f53c5. Fabrice, does this patch take Ben's comments into account?

Comment on attachment 8493442 [details] [diff] [review] 0002-Bug-902165-System-JS-ERROR-resource-gre-modules-Brow.patch Review of attachment 8493442 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/apps/Webapps.jsm @@ +4294,5 @@ > + return; > + } > + if (!app.hasPermission("browser")) { > + return; > + } That's not how we test permissions in ipc code. You need to use assertPermission)() like in http://mxr.mozilla.org/mozilla-central/source/dom/apps/Webapps.jsm?rev=5f06a3b5fd42#1167

(In reply to Fabrice Desré [:fabrice] from comment #28) > That's not how we test permissions in ipc code. You need to use > assertPermission)() like in > http://mxr.mozilla.org/mozilla-central/source/dom/apps/Webapps. > jsm?rev=5f06a3b5fd42#1167 Like this? Try at https://tbpl.mozilla.org/?tree=Try&rev=a606605886f6

Comment on attachment 8494256 [details] [diff] [review] 0002-Bug-902165-System-JS-ERROR-resource-gre-modules-Brow.patch Review of attachment 8494256 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nit fixed. ::: dom/apps/Webapps.jsm @@ +1170,5 @@ > " from a content process with no 'webapps-manage' privileges."); > return null; > } > } > + // And RegisterBEP requires "browser" priv's... s/priv's/permission

Thanks! https://hg.mozilla.org/integration/fx-team/rev/3f2617daac82