Bug 1553371 - Load chrome frameScripts when switching processes. r=snorp a=jcristau
authorAgi Sferro <agi@sferro.dev>
Wed, 22 May 2019 19:20:54 +0000
changeset 533420 fb23d87ad492d731511d436db7e446afea90b4ef
parent 533419 3455989277dc4d6c8d5fb6303d6076f333af624c
child 533421 c6df1fc8a2a17b2bbbf1c2dca6a391d0df67f449
push id11313
push usernbeleuzu@mozilla.com
push dateMon, 27 May 2019 10:32:57 +0000
treeherdermozilla-beta@7f83ca2147b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp, jcristau
bugs1553371
milestone68.0
Bug 1553371 - Load chrome frameScripts when switching processes. r=snorp a=jcristau 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")
     }