Bug 1553371 - Load chrome frameScripts when switching processes. r=snorp
authorAgi Sferro <agi@sferro.dev>
Wed, 22 May 2019 19:20:54 +0000
changeset 475030 28d50d22a289aaf16d614fce4026aad2a81e4073
parent 475029 d4516071aefffa936541f886faefbfba386267ba
child 475031 418a61f5f87bff0dfceae36f3bbde0a242076cad
push id36054
push userdvarga@mozilla.com
push dateThu, 23 May 2019 15:52:15 +0000
treeherdermozilla-central@199eaff06ecd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1553371
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1553371 - Load chrome frameScripts when switching processes. r=snorp Whenever we switch processes in GeckoView we didn't inject frameScripts. This change adds a new method `loadInitFrameScript` that is called whenever a module's new browser is attached to a window so that the frame script is loaded correctly into the new browser. This fixes a bug in WebExtension pages where the WebExtension Process Script would never be notified of a new WebExtension page window, breaking privileged APIs. Differential Revision: https://phabricator.services.mozilla.com/D32148
mobile/android/chrome/geckoview/geckoview.js
mobile/android/geckoview/src/androidTest/assets/web_extensions/extension-page-update/tab-script.js
mobile/android/geckoview/src/androidTest/assets/web_extensions/extension-page-update/tab.html
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt
--- a/mobile/android/chrome/geckoview/geckoview.js
+++ b/mobile/android/chrome/geckoview/geckoview.js
@@ -65,16 +65,17 @@ var ModuleManager = {
       "GeckoView:UpdateSettings",
     ]);
 
     this.messageManager.addMessageListener("GeckoView:ContentModuleLoaded",
                                            this);
 
     this.forEach(module => {
       module.onInit();
+      module.loadInitFrameScript();
     });
 
     window.addEventListener("unload", () => {
       this.forEach(module => {
         module.enabled = false;
         module.onDestroy();
       });
 
@@ -157,16 +158,24 @@ var ModuleManager = {
     this.forEach(module => {
       if (module.impl) {
         module.impl.onInitBrowser();
       }
     });
 
     parent.appendChild(this.browser);
 
+    this.messageManager.addMessageListener("GeckoView:ContentModuleLoaded",
+                                           this);
+
+    this.forEach(module => {
+      // We're attaching a new browser so we have to reload the frame scripts
+      module.loadInitFrameScript();
+    });
+
     disabledModules.forEach(module => {
       module.enabled = true;
     });
 
     this.browser.focus();
     return true;
   },
 
@@ -259,72 +268,84 @@ class ModuleInfo {
     this._contentModuleLoaded = false;
     this._enabled = false;
     // Only enable once we performed initialization.
     this._enabledOnInit = enabled;
 
     // For init, load resource _before_ initializing browser to support the
     // onInitBrowser() override. However, load content module after initializing
     // browser, because we don't have a message manager before then.
-    this._loadPhase({
-      resource: onInit && onInit.resource,
-    });
-    this._onInitPhase = {
-      frameScript: onInit && onInit.frameScript,
-    };
+    this._loadResource(onInit);
+
+    this._onInitPhase = onInit;
     this._onEnablePhase = onEnable;
   }
 
   onInit() {
     if (this._impl) {
       this._impl.onInit();
       this._impl.onSettingsUpdate();
     }
-    this._loadPhase(this._onInitPhase);
 
     this.enabled = this._enabledOnInit;
   }
 
+  /**
+   * Loads the onInit frame script
+   */
+  loadInitFrameScript() {
+    this._loadFrameScript(this._onInitPhase);
+  }
+
   onDestroy() {
     if (this._impl) {
       this._impl.onDestroy();
     }
   }
 
-  // Called before the browser is removed
+  /**
+   * Called before the browser is removed
+   */
   onDestroyBrowser() {
-    if (this.impl) {
-      this.impl.onDestroyBrowser();
+    if (this._impl) {
+      this._impl.onDestroyBrowser();
     }
     this._contentModuleLoaded = false;
   }
 
   /**
-   * Load resources according to a phase object that contains possible keys,
+   * Load resource according to a phase object that contains possible keys,
    *
    * "resource": specify the JSM resource to load for this module.
    * "frameScript": specify a content JS frame script to load for this module.
    */
-  _loadPhase(aPhase) {
-    if (!aPhase) {
+  _loadResource(aPhase) {
+    if (!aPhase || !aPhase.resource || this._impl) {
       return;
     }
 
-    if (aPhase.resource && !this._impl) {
-      const exports = ChromeUtils.import(aPhase.resource);
-      this._impl = new exports[this._name](this);
+    const exports = ChromeUtils.import(aPhase.resource);
+    this._impl = new exports[this._name](this);
+  }
+
+  /**
+   * Load frameScript according to a phase object that contains possible keys,
+   *
+   * "frameScript": specify a content JS frame script to load for this module.
+   */
+  _loadFrameScript(aPhase) {
+    if (!aPhase || !aPhase.frameScript || this._contentModuleLoaded) {
+      return;
     }
 
-    if (aPhase.frameScript && !this._contentModuleLoaded) {
-      if (this._impl) {
-        this._impl.onLoadContentModule();
-      }
-      this._manager.messageManager.loadFrameScript(aPhase.frameScript, true);
-      this._contentModuleLoaded = true;
+    if (this._impl) {
+      this._impl.onLoadContentModule();
     }
+    this._manager.messageManager.loadFrameScript(aPhase.frameScript, true);
+    this._contentModuleLoaded = true;
   }
 
   get manager() {
     return this._manager;
   }
 
   get name() {
     return this._name;
@@ -345,17 +366,18 @@ class ModuleInfo {
 
     if (!aEnabled && this._impl) {
       this._impl.onDisable();
     }
 
     this._enabled = aEnabled;
 
     if (aEnabled) {
-      this._loadPhase(this._onEnablePhase);
+      this._loadResource(this._onEnablePhase);
+      this._loadFrameScript(this._onEnablePhase);
       if (this._impl) {
         this._impl.onEnable();
         this._impl.onSettingsUpdate();
       }
     }
 
     this._updateContentModuleState(/* includeSettings */ aEnabled);
   }
new file mode 100644
--- /dev/null
+++ b/mobile/android/geckoview/src/androidTest/assets/web_extensions/extension-page-update/tab-script.js
@@ -0,0 +1,2 @@
+// Let's test privileged APIs
+browser.runtime.sendNativeMessage("browser", "HELLO_FROM_PAGE");
--- a/mobile/android/geckoview/src/androidTest/assets/web_extensions/extension-page-update/tab.html
+++ b/mobile/android/geckoview/src/androidTest/assets/web_extensions/extension-page-update/tab.html
@@ -1,1 +1,2 @@
-<h1>Hello World!</h1>
\ No newline at end of file
+<h1>Hello World!</h1>
+<script src=tab-script.js></script>
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt
@@ -460,17 +460,34 @@ class WebExtensionTest : BaseSessionTest
             testIframeTopLevel()
         } finally {
             httpBin.stop()
         }
     }
 
     @Test
     fun loadWebExtensionPage() {
-        val extension = WebExtension("resource://android/assets/web_extensions/extension-page-update/")
+        val result = GeckoResult<String>()
+        var extension: WebExtension? = null;
+
+        val messageDelegate = object : WebExtension.MessageDelegate {
+            override fun onMessage(message: Any,
+                                   sender: WebExtension.MessageSender): GeckoResult<Any>? {
+                Assert.assertEquals(extension, sender.webExtension)
+                Assert.assertEquals(WebExtension.MessageSender.ENV_TYPE_EXTENSION,
+                        sender.environmentType)
+                result.complete(message as String)
+
+                return null
+            }
+        }
+
+        extension = WebExtension("resource://android/assets/web_extensions/extension-page-update/")
+        extension.setMessageDelegate(messageDelegate, "browser")
+
         sessionRule.waitForResult(sessionRule.runtime.registerWebExtension(extension))
 
         mainSession.loadUri("http://example.com");
 
         mainSession.waitUntilCalled(object : Callbacks.NavigationDelegate, Callbacks.ProgressDelegate {
             @GeckoSessionTestRule.AssertCalled(count = 1)
             override fun onLocationChange(session: GeckoSession, url: String?) {
                 assertThat("Url should load example.com first",
@@ -499,16 +516,20 @@ class WebExtensionTest : BaseSessionTest
                 }
             }
         })
 
         // Make sure the page loaded successfully
         sessionRule.waitForResult(pageStop)
 
         assertThat("Url should load WebExtension page", page, endsWith("/tab.html"))
+
+        assertThat("WebExtension page should have access to privileged APIs",
+            sessionRule.waitForResult(result), equalTo("HELLO_FROM_PAGE"))
+
         sessionRule.waitForResult(sessionRule.runtime.unregisterWebExtension(extension))
     }
 
     @Test
     fun badFileType() {
         testRegisterError("resource://android/bad/location/error",
                 "does not point to a folder or an .xpi")
     }