1070810 - Webapps.jsm "accidentally" fails in content-processes and needs a way to purposely fail

Status: RESOLVED INVALID

(Core Graveyard :: DOM: Apps, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

Webapps.jsm has at line 104:

const WEBAPP_RUNTIME = Services.appinfo.ID == "[e-mail]";

This has the side-effect of making this .jsm fail when loaded into a content process - appinfo.ID fails in that context.  However, bug 1067229 exists to allow appinfo.ID and appinfo.vendor to work in content processes.

When bug 1067229 landed, it caused obscure b2g failures (and was subsequently backed out).  The problem is that this "accidental" failure was no longer hit, and caused sandbox violations, such as https://tbpl.mozilla.org/php/getParsedLog.php?id=48445259&tree=Try&full=1#error0

19:51:24    ERROR -  09-19 02:43:36.598 E/Sandbox (  793): seccomp sandbox violation: pid 793, syscall 39, args 1180394744 493 493 3198622220 1084950661 493.  Killing process.
19:51:24    ERROR -  A content process has violated the system call sandbox (bug 790923)

I made a try-push that simulates the error by explicitly throwing if we are not in the main process - https://hg.mozilla.org/try/rev/4109d4563b89 - and the try run here was green (https://tbpl.mozilla.org/?tree=Try&rev=ed872fba2038) - it also shows the call stack for when it is called in the content process (which *looks* alot like it is in the parent process, but I assume it's actually a child process itself acting as a parent).

Anyway, I'm looking for feedback on the best way to resolve this - is an explicit throw like https://hg.mozilla.org/try/rev/4109d4563b89 OK (but presumably closer to the top of the file)?  And if so, do we still want a dump/reportError() to show the stack?

needinfo Fabrice as his fingerprints are all over Webapps.jsm - feel free to forward it to someone else.

Comment 1

[[:fabrice] Fabrice Desré]

This jsm should never be loaded in a content parent. Do we know where the import comes from?

Comment 2

[Mark Hammond [:markh] [:mhammond]]

The patch I made was:

     1.1 --- a/dom/apps/Webapps.jsm
     1.2 +++ b/dom/apps/Webapps.jsm
     1.3 @@ -96,16 +96,25 @@ function supportUseCurrentProfile() {
     1.4  
     1.5  function supportSystemMessages() {
     1.6    return Services.prefs.getBoolPref("dom.sysmsg.enabled");
     1.7  }
     1.8  
     1.9  // Minimum delay between two progress events while downloading, in ms.
    1.10  const MIN_PROGRESS_EVENT_DELAY = 1500;
    1.11  
    1.12 +// XXX - referencing appinfo.ID would previously fail in a content process,
    1.13 +// but bug 1061873 is putting them back.  Sadly though, this causes this
    1.14 +// module to load much further in content processes and cause a sanbox violation.
    1.15 +// So simulate that previous error.
    1.16 +if (Services.appinfo.processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
    1.17 +  dump("\n\n ****** WEBAPPS in content process: " + new Error().stack + "\n\n")
    1.18 +  throw Cr.NS_ERROR_NOT_INITIALIZED;
    1.19 +}
    1.20 +
    1.21  const WEBAPP_RUNTIME = Services.appinfo.ID == "[e-mail]";
    1.22

and the log for the *green* run I then got says:

19:14:31     INFO -   ****** WEBAPPS in content process: @resource://gre/modules/Webapps.jsm:102:53
19:14:31     INFO -  @resource://gre/modules/BrowserElementParent.jsm:24:3
19:14:31     INFO -  XPCU_defineLazyGetter/<.get@resource://gre/modules/XPCOMUtils.jsm:193:33
19:14:31     INFO -  BrowserElementParent.prototype._registerAppManifest@resource://gre/modules/BrowserElementParent.jsm:221:1
19:14:31     INFO -  BrowserElementParent@resource://gre/modules/BrowserElementParent.jsm:186:5
19:14:31     INFO -  create@resource://gre/modules/BrowserElementParent.jsm:64:12
19:14:31     INFO -  BrowserElementParentFactory.prototype._createBrowserElementParent@jar:file:///system/b2g/omni.ja!/components/BrowserElementParent.js:105:36
19:14:31     INFO -  BrowserElementParentFactory.prototype._observeInProcessBrowserFrameShown@jar:file:///system/b2g/omni.ja!/components/BrowserElementParent.js:87:1
19:14:31     INFO -  BrowserElementParentFactory.prototype.observe@jar:file:///system/b2g/omni.ja!/components/BrowserElementParent.js:123:7
19:14:31     INFO -  runTest@http://mochi.test:8888/tests/dom/datastore/tests/test_app_install.html:101:5
19:14:31     INFO -  continueTest@http://mochi.test:8888/tests/dom/datastore/tests/test_app_install.html:38:7
19:14:31     INFO -  WebappsRegistry.prototype.receiveMessage@jar:file:///system/b2g/omni.ja!/components/Webapps.js:51:1

Comment 3

[[:fabrice] Fabrice Desré]

I think the right fix is to finally land bug 902165 - I never found the time to check what bent asks in https://bugzilla.mozilla.org/show_bug.cgi?id=902165#c26

Comment 4

[Mark Hammond [:markh] [:mhammond]]

As per comment 3, this is invalid.