1628307 - ExtensionTestUtils.loadContentPage(url, {extension}) uses incorrect remote setting when extensions run in-process

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[Rob Wu [:robwu]]

In https://bugzilla.mozilla.org/show_bug.cgi?id=1613796#c14 Shane found that the following test fails:

./mach xpcshell-test --log-mach-verbose --tag in-process-webextensions toolkit/components/extensions/test/xpcshell/test_ext_permissions.js

I ran it locally, and the failure is in a specific part of test_alreadyGranted in test_ext_permissions.js that uses an extension API from an extension page in a ContentPage. The error message is as follows:

JavaScript error: moz-extension://daa1899d-c62c-4875-9651-c92609526ed3/page.js, line 4: TypeError: can't access property "request", browser.permissions is undefined

Upon further investigation, I found that the relevant line is:

let page = await ExtensionTestUtils.loadContentPage(url, { extension });

When extensions run in-process, the ContentPage should be loaded in the main process. However, the ContentPage constructor defaults to using a content process when e10s is enabled. As a result, the extension page is sent to the wrong process, the extension API bindings are not exposed, and the test fails.

Comment 1

[Rob Wu [:robwu]]

This specific bug can be fixed by defaulting to extension.remote if remote are set, like this:

--- a/toolkit/components/extensions/ExtensionXPCShellUtils.jsm
+++ b/toolkit/components/extensions/ExtensionXPCShellUtils.jsm
@@ -177,11 +177,18 @@ function promiseBrowserLoaded(browser, url, redirectUrl) {
 
 class ContentPage {
   constructor(
-    remote = REMOTE_CONTENT_SCRIPTS,
+    remote = null,
     extension = null,
     privateBrowsing = false,
     userContextId = undefined
   ) {
+    if (remote === null) {
+      if (extension) {
+        remote = extension.remote;
+      } else {
+        remote = REMOTE_CONTENT_SCRIPTS;
+      }
+    }
     this.remote = remote;
     this.extension = extension;
     this.privateBrowsing = privateBrowsing;

Not sure if that is worth the effort though, especially since we're probably going to drop support for in-process extensions soonish (bug 1613141). GeckoView has switched to out-of-process extensions (bug 1535365), which means that the only situation with in-process extensions is when e10s is disabled altogether. That condition matches with the current default value for remote in the ContentPage constructor.

I think that long-term, we should remove the extension parameter from the constructor, and have a common implementation for process switching (bug 1580811) (not just for the initial load, but also for any redirects/navigations).

Comment 2

[Shane Caraveo (:mixedpuppy)]

The problem is that the default for "browser.tabs.remote.autostart" has changed from false to true (or something runtime is doing that). There is a comment in head_e10s about the default being false for xpcshell, so when running in-process we just assume it is false. Explicitly setting it to false fixes the problem without the change in comment 1.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1628307 fix running webextension in-process tests

Comment 4

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea29ee4c2e1df6ac68852616dbf407d1b67ea0b8

Comment 5

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=687eea6d1fb61cf41145d4fdc422c8b800ed050e

Comment 6

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a65afdc0b2b60c61bcdc2150a321a93b8164014

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f1d8fcb50da0
fix running webextension in-process tests r=robwu

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/f1d8fcb50da0