1317101 - Run WebExtension tests in both in-process and out-of-process modes

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Comment 1

[Kris Maglione [:kmag]]

The final implementation of OOP extensions should have them running in a separate process. The purpose of this bug is only to make sure that our code works as expected with out-of-process add-ons, and continues to do so, so it currently simply runs them in the normal web content process.

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 1: Allow about:addons to load remote <browser>s into its content docshell.

Review commit: https://reviewboard.mozilla.org/r/92532/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92532/

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 2: Listen for tab messages only on tabbrowser browsers.

Review commit: https://reviewboard.mozilla.org/r/92534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92534/

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 3: Apply remote-browser binding to all remote="true" <browser>s.

Review commit: https://reviewboard.mozilla.org/r/92536/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92536/

Comment 5

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 4: Deduplicate the handling of context tab and window IDs, and handle <browser> nesting in tabs.

This could still use a fair amount of additional cleanup.

Review commit: https://reviewboard.mozilla.org/r/92538/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92538/

Comment 6

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling.

Review commit: https://reviewboard.mozilla.org/r/92540/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92540/

Comment 7

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 6: Remove or refactor code that prevents extensions from running in the child process.

Review commit: https://reviewboard.mozilla.org/r/92542/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92542/

Comment 8

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 7a: Add a `remote` flag to run an extension out-of-process based on a preference.

Review commit: https://reviewboard.mozilla.org/r/92544/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92544/

Comment 9

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 7b: Run remote extension background pages in a visible window for testing.

Review commit: https://reviewboard.mozilla.org/r/92546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92546/

Comment 10

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 7c: Run extension popups in a remote browser.

Review commit: https://reviewboard.mozilla.org/r/92548/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92548/

Comment 11

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 7d: Load moz-extension: URLs remotely based on a pref.

Review commit: https://reviewboard.mozilla.org/r/92550/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92550/

Comment 12

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 7e: Load extension options pages in a remote browser.

Review commit: https://reviewboard.mozilla.org/r/92552/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92552/

Comment 13

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 8a: Read defaults from ancestor manifests when processing test metadata.

Review commit: https://reviewboard.mozilla.org/r/92554/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92554/

Comment 14

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 8b: Run browser mochitests in both parent-process and remote configurations.

Review commit: https://reviewboard.mozilla.org/r/92556/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92556/

Comment 15

[Kris Maglione [:kmag]]

Attachment: Bug 1317101 - Part 8c: Run plain mochitests in both parent-process and remote configurations.

Review commit: https://reviewboard.mozilla.org/r/92558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92558/

Comment 16

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810138 [details]
Bug 1317101 - Part 2: Listen for tab messages only on tabbrowser browsers.

https://reviewboard.mozilla.org/r/92534/#review92840

::: browser/base/content/tabbrowser.xml:4835
(Diff revision 1)
>              .QueryInterface(Ci.nsILoadContext)
>              .useRemoteTabs;
>            if (remote) {
>              messageManager.addMessageListener("DOMTitleChanged", this);
>              messageManager.addMessageListener("DOMWindowClose", this);
>              messageManager.addMessageListener("contextmenu", this);

I think we might still want "contextmenu" to be hooked up for the whole window.

Comment 17

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810139 [details]
Bug 1317101 - Part 3: Apply remote-browser binding to all remote="true" <browser>s.

https://reviewboard.mozilla.org/r/92536/#review92842

You should be able to remove the specializations for view source, social, and Android. The reason we didn't do this generically was bug 902041, but I don't think that matters anymore. We don't use CPOWs in the binding unless you ask for them.

Comment 18

[Andrew Swan [:aswan]]

Comment on attachment 8810140 [details]
Bug 1317101 - Part 4: Deduplicate the handling of context tab and window IDs, and handle <browser> nesting in tabs.

https://reviewboard.mozilla.org/r/92538/#review92846

::: browser/components/extensions/ext-tabs.js:30
(Diff revision 1)
>  // This function is pretty tightly tied to Extension.jsm.
>  // Its job is to fill in the |tab| property of the sender.
>  function getSender(extension, target, sender) {
> +  let tabId;
>    if ("tabId" in sender) {
>      // The message came from an ExtensionContext. In that case, it should

ExtensionContext -> ContentScriptContext?

::: browser/components/extensions/ext-tabs.js:76
(Diff revision 1)
> -  data.tabId = browser ? TabManager.getBrowserId(browser) : -1;
> +  let tabId = null;
> +  if (browser) {
> +    tabId = getBrowserInfo(browser).tabId;
> +  }
> +
> +  data.tabId = tabId || -1;

nit, you could initialize it to -1 a few lines above and avoid the || here

::: toolkit/components/extensions/ExtensionParent.jsm:342
(Diff revision 1)
>    }
>  
>    get tabId() {
> -    if (!apiManager.global.TabManager) {
> -      return;  // Not yet supported on Android.
> +    let {getBrowserInfo} = apiManager.global;
> +
> +    if (getBrowserInfo) {

Preserve the comment about this not being defined on android?  At first glance it looks strange without knowing about android.

Comment 19

[Andrew Swan [:aswan]]

Comment on attachment 8810141 [details]
Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling.

https://reviewboard.mozilla.org/r/92540/#review92854

Can you give me a little more background to help me understand this one?
The simplifications certainly look like an improvement but I don't understand why the original code worked the way it did, and why we now have the opportunity to simplify it, was it just sloppiness?

Comment 20

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810143 [details]
Bug 1317101 - Part 7a: Add a `remote` flag to run an extension out-of-process based on a preference.

https://reviewboard.mozilla.org/r/92544/#review92844

::: modules/libpref/init/all.js:4716
(Diff revision 1)
>  pref("extensions.webExtensionsMinPlatformVersion", "42.0a1");
>  
>  // Other webextensions prefs
>  pref("extensions.webextensions.keepStorageOnUninstall", false);
>  pref("extensions.webextensions.keepUuidOnUninstall", false);
> +pref("extensions.webextensions.outOfProcess", false);

Maybe "extensions.webextensions.remote"? That fits better with browser.tabs.remote. Although outOfProcess is probably more descriptive.

::: toolkit/components/extensions/Extension.jsm:601
(Diff revision 1)
> +      // Since we're currently extensions in the web content child
> +      // process for testing, we currently only support single-process
> +      // e10s, and the extension child will always run in that process.

This comment is weird. How about: "In single-process e10s, where processCount == 1, we run remote extensions in the content process. In this case, process 0 is the UI process and process 1 is the content/extension process."

::: toolkit/components/extensions/ExtensionParent.jsm:148
(Diff revision 1)
>        // "Extension:Port:Disconnect" and "Extension:Port:PostMessage" for
>        // native messages are handled by NativeApp.
>        return;
>      }
> +
>      let extension = GlobalManager.extensionMap.get(sender.extensionId);

Can we validate that the message actually came from extension.parentProcessManager? That would be kind of nice for security.

::: toolkit/components/extensions/ExtensionParent.jsm:188
(Diff revision 1)
>        return tab && tab.linkedBrowser.messageManager;
>      }
>  
>      // runtime.sendMessage / runtime.connect
> -    if (extensionId) {
> -      // TODO(robwu): map the extensionId to the addon parent process's message
> +    if (extension) {
> +      return extension.parentMessageManager;

This doesn't seem right to me. If two extensions run in separate processes (which would be nice to support eventually), you want to use the destination extension's PPMM. You're using the source here. I know they're the same for now, but I think we should get it right. Can you just use GlobalManager.extensionMap.get(recipient.extensionId)?

Comment 21

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8810148 [details]
Bug 1317101 - Part 8a: Read defaults from ancestor manifests when processing test metadata.

https://reviewboard.mozilla.org/r/92554/#review92856

I think for this to work as intended we would need a patch to the manifestparser itself (in testing/mozbase/manifestparser). This file reads test metadata when running tests locally, but doesn't impact what happens in automation.

Comment 22

[Kris Maglione [:kmag]]

Comment on attachment 8810141 [details]
Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling.

https://reviewboard.mozilla.org/r/92540/#review92854

I don't know why it was done that way before. The webextension-view-type attribute was there before the OOP changes. I don't know why it was replaced with explicit messaging, and loading of the URL in the content process, but neither of them is necessary.

Comment 23

[Kris Maglione [:kmag]]

Comment on attachment 8810140 [details]
Bug 1317101 - Part 4: Deduplicate the handling of context tab and window IDs, and handle <browser> nesting in tabs.

https://reviewboard.mozilla.org/r/92538/#review92846

> ExtensionContext -> ContentScriptContext?

No, an extension page running in a tab, so not a content script.

> nit, you could initialize it to -1 a few lines above and avoid the || here

Nope, because `getBrowserInfo` returns an object without a `tabId` attribute if the browser doesn't belong to a tab.

Comment 24

[Kris Maglione [:kmag]]

Comment on attachment 8810148 [details]
Bug 1317101 - Part 8a: Read defaults from ancestor manifests when processing test metadata.

https://reviewboard.mozilla.org/r/92554/#review92856

As far as I can tell, manifestparser.py already handles this correctly, and it already used to work this way locally prior to bug 1312520, since mozapps/addons uses this same pattern.

Comment 25

[Kris Maglione [:kmag]]

Comment on attachment 8810143 [details]
Bug 1317101 - Part 7a: Add a `remote` flag to run an extension out-of-process based on a preference.

https://reviewboard.mozilla.org/r/92544/#review92844

> Maybe "extensions.webextensions.remote"? That fits better with browser.tabs.remote. Although outOfProcess is probably more descriptive.

Yeah, that probably makes sense.

> Can we validate that the message actually came from extension.parentProcessManager? That would be kind of nice for security.

At the moment, this only handles messages from content scripts, which can come from anywhere. At some point we're going to have to give more thought to content sandboxing, though.

> This doesn't seem right to me. If two extensions run in separate processes (which would be nice to support eventually), you want to use the destination extension's PPMM. You're using the source here. I know they're the same for now, but I think we should get it right. Can you just use GlobalManager.extensionMap.get(recipient.extensionId)?

We're handling messages mainly from content scripts, being proxied to code in the extension process, so this is the right message manager.

Comment 26

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810144 [details]
Bug 1317101 - Part 7b: Run remote extension background pages in a visible window for testing.

https://reviewboard.mozilla.org/r/92546/#review92878

Comment 27

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810146 [details]
Bug 1317101 - Part 7d: Load moz-extension: URLs remotely based on a pref.

https://reviewboard.mozilla.org/r/92550/#review92880

Comment 28

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810149 [details]
Bug 1317101 - Part 8b: Run browser mochitests in both parent-process and remote configurations.

https://reviewboard.mozilla.org/r/92556/#review92882

Comment 29

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810150 [details]
Bug 1317101 - Part 8c: Run plain mochitests in both parent-process and remote configurations.

https://reviewboard.mozilla.org/r/92558/#review92884

Comment 30

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810141 [details]
Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling.

https://reviewboard.mozilla.org/r/92540/#review92886

I reviewed the original code. This looks good to me.

Comment 31

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Olli, do you know of any reason why the docshell type check is there for remote browsers (see the part 1 patch)? The only reason I can think of is that we want window.parent/top to mean the same thing for remote=true and remote=false. In this case, window.parent/top are apparently broken in the non-e10s case and remote=true gives the correct behavior.

We could use an actual mozbrowser here, but that would probably be a lot of work and it doesn't seem worth it to me.

Comment 32

[Kris Maglione [:kmag]]

Comment on attachment 8810148 [details]
Bug 1317101 - Part 8a: Read defaults from ancestor manifests when processing test metadata.

I confirmed that with this change, things work as expected both with local `mach mochitest` and on infrastructure. Without it, local `mach mochitest` does not work as expected. I'm not sure whether infra does.

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Bill McCloskey (:billm) from comment #31)
> Olli, do you know of any reason why the docshell type check is there for
> remote browsers (see the part 1 patch)? 
We don't want to try to create remote html:iframes inside web pages.

In practice if (!mOwnerContent->IsXULElement()) { saves us there too, since XUL is disabled in content.
(though, if there is some bug and web page manages to inject xul:browser to some xbl binding...)

> The only reason I can think of is
> that we want window.parent/top to mean the same thing for remote=true and
> remote=false. In this case, window.parent/top are apparently broken in the
> non-e10s case and remote=true gives the correct behavior.
I don't see what .parent/top has to do with this. What am I missing here.

Comment 34

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8810137 [details]
Bug 1317101 - Part 1: Allow about:addons to load remote <browser>s into its content docshell.

https://reviewboard.mozilla.org/r/92532/#review93232

I talked to Olli about this on IRC (#developers around 11:13am PST). It seems like it should be okay.

::: dom/base/nsFrameLoader.cpp:2675
(Diff revision 1)
>  
>    // <iframe mozbrowser> gets to skip these checks.
>    if (!OwnerIsMozBrowserOrAppFrame()) {
>      if (parentDocShell->ItemType() != nsIDocShellTreeItem::typeChrome) {
> +      // Allow about:addon an exception to this rule so it can load remote
> +      // extension options pages.

Please add a comment like this:

Note that the new frame's message manager will not be a child of the chrome window message manager. Also, the values of window.top and window.parent will be different than they would be for a non-remote frame.

Comment 35

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8810148 [details]
Bug 1317101 - Part 8a: Read defaults from ancestor manifests when processing test metadata.

https://reviewboard.mozilla.org/r/92554/#review93278

Comment 36

[Andrew Swan [:aswan]]

Comment on attachment 8810142 [details]
Bug 1317101 - Part 6: Remove or refactor code that prevents extensions from running in the child process.

https://reviewboard.mozilla.org/r/92542/#review93584

Comment 37

[Andrew Swan [:aswan]]

Comment on attachment 8810142 [details]
Bug 1317101 - Part 6: Remove or refactor code that prevents extensions from running in the child process.

https://reviewboard.mozilla.org/r/92542/#review93586

::: toolkit/components/extensions/ExtensionManagement.jsm:267
(Diff revision 1)
>    // has no access to APIs.
>    if (!addonId || getAddonIdForWindow(window) != addonId) {
>      return NO_PRIVILEGES;
>    }
>  
> -  // Extension pages running in the content process always defaults to
> +  if (!ExtensionManagement.isExtensionProcess) {

Sorry reading the patches in order, I'm now coming back to this.
With your changes in the following patch, I think isExtensionProcess is always true if oop extensions are enabled which means this branch never gets taken.  But since background scripts and content scripts are running in the same process, just checking the process type can't realy work.  In any case, doesn't this grant content scripts more privileges than they should have when oop is enabled?  Seems like that should break a test...

Comment 38

[Andrew Swan [:aswan]]

Comment on attachment 8810145 [details]
Bug 1317101 - Part 7c: Run extension popups in a remote browser.

https://reviewboard.mozilla.org/r/92548/#review93588

::: browser/components/extensions/ext-utils.js:260
(Diff revision 1)
> +
>      viewNode.appendChild(this.browser);
>  
>      extensions.emit("extension-browser-inserted", this.browser);
>  
>      let initBrowser = browser => {

Since this is now always chained after readyPromise, just do that once here rather than in two different places below?

Comment 39

[Kris Maglione [:kmag]]

Comment on attachment 8810142 [details]
Bug 1317101 - Part 6: Remove or refactor code that prevents extensions from running in the child process.

https://reviewboard.mozilla.org/r/92542/#review93586

> Sorry reading the patches in order, I'm now coming back to this.
> With your changes in the following patch, I think isExtensionProcess is always true if oop extensions are enabled which means this branch never gets taken.  But since background scripts and content scripts are running in the same process, just checking the process type can't realy work.  In any case, doesn't this grant content scripts more privileges than they should have when oop is enabled?  Seems like that should break a test...

This check only existed to make sure that we fell back to the content script API if an extension page somehow wound up running as a top-level page in the content process, where we couldn't actually provide the fully-privileged APIs. In practice, that shouldn't actually happen. Extension pages running at the top-level should always get redirected to the correct process. But either way, that's not an issue anymore.

Content scripts always get only content script APIs. This check doesn't apply to them, it just determines whether extension pages get full API privileges or content script privileges.

At some point, when we start implementing process sandboxing for privileged APIs, we'll need a stricter check for exactly which process we're running in (mainly on the parent side), but for now it doesn't really make a difference.

Comment 40

[Kris Maglione [:kmag]]

Comment on attachment 8810145 [details]
Bug 1317101 - Part 7c: Run extension popups in a remote browser.

https://reviewboard.mozilla.org/r/92548/#review93588

> Since this is now always chained after readyPromise, just do that once here rather than in two different places below?

Yeah, probably.

Comment 41

[Andrew Swan [:aswan]]

Comment on attachment 8810147 [details]
Bug 1317101 - Part 7e: Load extension options pages in a remote browser.

https://reviewboard.mozilla.org/r/92552/#review93592

::: toolkit/mozapps/extensions/content/extensions.js:170
(Diff revision 1)
> +  return new Promise(resolve => {
> +    function listener(event) {
> +      target.removeEventListener(event, listener, useCapture);
> +      resolve(event);
> +    }
> +    target.addEventListener(event, listener, useCapture);

You could simplify this with the `once` option to `addEventListener()`: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Parameters

Comment 42

[Kris Maglione [:kmag]]

Comment on attachment 8810147 [details]
Bug 1317101 - Part 7e: Load extension options pages in a remote browser.

https://reviewboard.mozilla.org/r/92552/#review93592

> You could simplify this with the `once` option to `addEventListener()`: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Parameters

Huh. Nifty.

Comment 43

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ffc63be3557c3a9531591b28adff6feb0e88e3e3
Bug 1317101 - Part 1: Allow about:addons to load remote <browser>s into its content docshell. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/47d283897283f33d3590ba5b946cc1e5eb5e44d3
Bug 1317101 - Part 2: Listen for tab messages only on tabbrowser browsers. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/07321664430ab3416a0aa3a29055012b5eb149c5
Bug 1317101 - Part 3: Apply remote-browser binding to all remote="true" <browser>s. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/b0521588011d866ee6b2019ee71654768b9e7677
Bug 1317101 - Part 4: Deduplicate the handling of context tab and window IDs, and handle <browser> nesting in tabs. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/225ad2535585ff6ce38e1e9f8fe5371194a70658
Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/af7b81d7a5cc8bef552e440da1b3dada47507fd5
Bug 1317101 - Part 6: Remove or refactor code that prevents extensions from running in the child process. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef7e061b37bf1337c7832df01664adc1fd504adf
Bug 1317101 - Part 7a: Add a `remote` flag to run an extension out-of-process based on a preference. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/039d63d5fef77d7b77c0a7f9724ec9e6347be09a
Bug 1317101 - Part 7b: Run remote extension background pages in a visible window for testing. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7892d3fb0ca5268a252377ecbb44dfb1d289500
Bug 1317101 - Part 7c: Run extension popups in a remote browser. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f8144296a9d8e67a4f307491559a0051f95a9a7
Bug 1317101 - Part 7d: Load moz-extension: URLs remotely based on a pref. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e1bfb578dcd24433d43fb67cc1f75e87ba48d21
Bug 1317101 - Part 7e: Load extension options pages in a remote browser. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/e06d269a5d4f35b608b5bc7f1c61e875f2322b45
Bug 1317101 - Part 8a: Read defaults from ancestor manifests when processing test metadata. r=chmanchester

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8cdc81cdcce558592d01d3cc6d0ee66269f1077
Bug 1317101 - Part 8b: Run browser mochitests in both parent-process and remote configurations. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/61f8a4084bbd9bdfda1d2d460f659c880a7deb13
Bug 1317101 - Part 8c: Run plain mochitests in both parent-process and remote configurations. r=billm

Comment 44

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/17757ba4c0e82c7300a0b832b1726230cbbe5f29
Bug 1317101: Follow-up: Fix race in popup tests.

Comment 45

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back all of this out to fix some jsreftest problems that appear to have started with this push: https://treeherder.mozilla.org/logviewer.html#?job_id=39321194&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed48815cbe18f8c787bb9441e35e6c9fc4c4be42

Comment 46

[Kris Maglione [:kmag]]

Hm. I guess this is because we're now applying the remote-browser bindings to remote reftest browsers... Not entirely sure why that causes crashes, though.

Comment 47

[Phil Ringnalda (:philor)]

If you don't like jsreftests, it also killed crashtests (e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=39321797&repo=mozilla-inbound) and reftests (e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=39328063&repo=mozilla-inbound). And if you don't like crashes with crappy stacks, on Windows and Mac reftests it just hangs instead of crashing (with equally useless stacks like https://treeherder.mozilla.org/logviewer.html#?job_id=39325837&repo=mozilla-inbound).

Comment 48

[Kris Maglione [:kmag]]

Yeah, crashtests are just reftests underneath. They're both failing for the same reason.

Comment 49

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/33fcde82df5da13d1863d5e8942cecc513a53544
Bug 1317101 - Part 1: Allow about:addons to load remote <browser>s into its content docshell. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/723511df403369d6715d8bee11bcfe919826e141
Bug 1317101 - Part 2: Listen for tab messages only on tabbrowser browsers. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d40ef9db8cef4da9899dcac38e5f0e3b4cbba6d
Bug 1317101 - Part 3: Apply remote-browser binding to all remote="true" <browser>s. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/67e3dc5de89c4b135ff39916f3369028682fe822
Bug 1317101 - Part 4: Deduplicate the handling of context tab and window IDs, and handle <browser> nesting in tabs. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/11df9a175c437b31ef1227940d8d8b8aed9511b5
Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/0bd9c50ea2238c1ea815fd9eb8e6500265305ad3
Bug 1317101 - Part 6: Remove or refactor code that prevents extensions from running in the child process. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a20d43e7cac9b558d85d7882aa6b7d07cfd66a2
Bug 1317101 - Part 7a: Add a `remote` flag to run an extension out-of-process based on a preference. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/eab933e775a9b9b6c4fd3a10517727af3ecc6f4d
Bug 1317101 - Part 7b: Run remote extension background pages in a visible window for testing. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/a9a2a53688b69f0c418e319b3cbe0517d5f4afd7
Bug 1317101 - Part 7c: Run extension popups in a remote browser. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/29f9433a43acf579cb3314c484d8a0eff2e88d72
Bug 1317101 - Part 7d: Load moz-extension: URLs remotely based on a pref. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/3330301202ab60286f826d61a473f1e1a8057301
Bug 1317101 - Part 7e: Load extension options pages in a remote browser. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/2fa61db0ad05e0d55f58ed2046c8a41503a3ce13
Bug 1317101 - Part 8a: Read defaults from ancestor manifests when processing test metadata. r=chmanchester

https://hg.mozilla.org/integration/mozilla-inbound/rev/7651bcf89bf1669c7cd3c1d954ccd440f91599bf
Bug 1317101 - Part 8b: Run browser mochitests in both parent-process and remote configurations. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/93ef05cf81ef89a9810e97f820a80db8ef784885
Bug 1317101 - Part 8c: Run plain mochitests in both parent-process and remote configurations. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/478d8f5e7ebb18092190b68eb49921b076e32d18
Bug 1317101: Follow-up: Don't use remote-browser bindings for reftests.

https://hg.mozilla.org/integration/mozilla-inbound/rev/06ef26669994ec4018e5072defb55b1af360c9f2
Bug 1317101: Follow-up: Split browserAction preloading tests into a separate unit.

Comment 50

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/33fcde82df5d
https://hg.mozilla.org/mozilla-central/rev/723511df4033
https://hg.mozilla.org/mozilla-central/rev/6d40ef9db8ce
https://hg.mozilla.org/mozilla-central/rev/67e3dc5de89c
https://hg.mozilla.org/mozilla-central/rev/11df9a175c43
https://hg.mozilla.org/mozilla-central/rev/0bd9c50ea223
https://hg.mozilla.org/mozilla-central/rev/7a20d43e7cac
https://hg.mozilla.org/mozilla-central/rev/eab933e775a9
https://hg.mozilla.org/mozilla-central/rev/a9a2a53688b6
https://hg.mozilla.org/mozilla-central/rev/29f9433a43ac
https://hg.mozilla.org/mozilla-central/rev/3330301202ab
https://hg.mozilla.org/mozilla-central/rev/2fa61db0ad05
https://hg.mozilla.org/mozilla-central/rev/7651bcf89bf1
https://hg.mozilla.org/mozilla-central/rev/93ef05cf81ef
https://hg.mozilla.org/mozilla-central/rev/478d8f5e7ebb
https://hg.mozilla.org/mozilla-central/rev/06ef26669994

Comment 51

[areya]

https://hg.mozilla.org/mozilla-central/rev/33fcde82df5d
https://hg.mozilla.org/mozilla-central/rev/723511df4033
https://hg.mozilla.org/mozilla-central/rev/6d40ef9db8ce
https://hg.mozilla.org/mozilla-central/rev/67e3dc5de89c
https://hg.mozilla.org/mozilla-central/rev/11df9a175c43
https://hg.mozilla.org/mozilla-central/rev/0bd9c50ea223
https://hg.mozilla.org/mozilla-central/rev/7a20d43e7cac
https://hg.mozilla.org/mozilla-central/rev/eab933e775a9
https://hg.mozilla.org/mozilla-central/rev/a9a2a53688b6
https://hg.mozilla.org/mozilla-central/rev/29f9433a43ac
https://hg.mozilla.org/mozilla-central/rev/3330301202ab
https://hg.mozilla.org/mozilla-central/rev/2fa61db0ad05
https://hg.mozilla.org/mozilla-central/rev/7651bcf89bf1
https://hg.mozilla.org/mozilla-central/rev/93ef05cf81ef
https://hg.mozilla.org/mozilla-central/rev/478d8f5e7ebb
https://hg.mozilla.org/mozilla-central/rev/06ef26669994