Bug 1479241: Don't eagerly load AboutPages.jsm in content processes. r?mythmon draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 31 Jul 2018 14:13:03 -0700
changeset 825041 d60fd4349506fb8c8c6c1a77a92ef093a1af7ed9
parent 823895 f5dbf87e95021529194bf8860e033924bdef0a4a
child 825074 1addbc47ea352ad1182c3f5d836b38aa249e0adc
child 825168 b3cbbf17486e096355ae1c692d0a97136022156a
push id118028
push usermaglione.k@gmail.com
push dateTue, 31 Jul 2018 23:11:55 +0000
reviewersmythmon
bugs1479241
milestone63.0a1
Bug 1479241: Don't eagerly load AboutPages.jsm in content processes. r?mythmon MozReview-Commit-ID: 1ewRIxTVzJR
browser/base/content/test/performance/browser_startup_content.js
browser/installer/package-manifest.in
toolkit/components/normandy/content/AboutPages.jsm
toolkit/components/normandy/content/shield-content-process.js
toolkit/components/normandy/moz.build
toolkit/components/normandy/shield-content-process.js
toolkit/components/normandy/shield.manifest
--- a/browser/base/content/test/performance/browser_startup_content.js
+++ b/browser/base/content/test/performance/browser_startup_content.js
@@ -64,19 +64,16 @@ const whitelist = {
 
     // PDF.js
     "resource://pdf.js/PdfJsRegistration.jsm",
     "resource://pdf.js/PdfjsContentUtils.jsm",
 
     // Extensions
     "resource://gre/modules/ExtensionUtils.jsm",
     "resource://gre/modules/MessageChannel.jsm",
-
-    // Shield
-    "resource://normandy-content/AboutPages.jsm",
   ]),
 };
 
 // Items on this list are allowed to be loaded but not
 // required, as opposed to items in the main whitelist,
 // which are all required.
 const intermittently_loaded_whitelist = {
   components: new Set([
--- a/browser/installer/package-manifest.in
+++ b/browser/installer/package-manifest.in
@@ -364,16 +364,20 @@
 @RESPATH@/browser/components/startupRecorder.js
 #endif
 
 ; [Extensions]
 @RESPATH@/components/extensions-toolkit.manifest
 @RESPATH@/components/extension-process-script.js
 @RESPATH@/browser/components/extensions-browser.manifest
 
+; [Normandy]
+@RESPATH@/components/shield.manifest
+@RESPATH@/components/shield-content-process.js
+
 ; Modules
 @RESPATH@/browser/modules/*
 @RESPATH@/modules/*
 
 ; Safe Browsing
 @RESPATH@/components/nsURLClassifier.manifest
 @RESPATH@/components/nsUrlClassifierHashCompleter.js
 @RESPATH@/components/nsUrlClassifierListManager.js
--- a/toolkit/components/normandy/content/AboutPages.jsm
+++ b/toolkit/components/normandy/content/AboutPages.jsm
@@ -1,14 +1,13 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
-const Cm = Components.manager;
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 ChromeUtils.defineModuleGetter(
   this, "CleanupManager", "resource://normandy/lib/CleanupManager.jsm",
 );
 ChromeUtils.defineModuleGetter(
   this, "AddonStudies", "resource://normandy/lib/AddonStudies.jsm",
@@ -16,37 +15,30 @@ ChromeUtils.defineModuleGetter(
 ChromeUtils.defineModuleGetter(
   this, "RecipeRunner", "resource://normandy/lib/RecipeRunner.jsm",
 );
 
 var EXPORTED_SYMBOLS = ["AboutPages"];
 
 const SHIELD_LEARN_MORE_URL_PREF = "app.normandy.shieldLearnMoreUrl";
 
-// Due to bug 1051238 frame scripts are cached forever, so we can't update them
-// as a restartless add-on. The Math.random() is the work around for this.
-const PROCESS_SCRIPT = (
-  `resource://normandy-content/shield-content-process.js?${Math.random()}`
-);
-const FRAME_SCRIPT = (
-  `resource://normandy-content/shield-content-frame.js?${Math.random()}`
-);
+const FRAME_SCRIPT = "resource://normandy-content/shield-content-frame.js";
 
 /**
  * Class for managing an about: page that Normandy provides. Adapted from
  * browser/extensions/pocket/content/AboutPocket.jsm.
  *
  * @implements nsIFactory
  * @implements nsIAboutModule
  */
 class AboutPage {
-  constructor({chromeUrl, aboutHost, classId, description, uriFlags}) {
+  constructor({chromeUrl, aboutHost, classID, description, uriFlags}) {
     this.chromeUrl = chromeUrl;
     this.aboutHost = aboutHost;
-    this.classId = Components.ID(classId);
+    this.classID = Components.ID(classID);
     this.description = description;
     this.uriFlags = uriFlags;
   }
 
   getURIFlags() {
     return this.uriFlags;
   }
 
@@ -56,84 +48,52 @@ class AboutPage {
     channel.originalURI = uri;
 
     if (this.uriFlags & Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT) {
       const principal = Services.scriptSecurityManager.createCodebasePrincipal(uri, {});
       channel.owner = principal;
     }
     return channel;
   }
-
-  createInstance(outer, iid) {
-    if (outer !== null) {
-      throw Cr.NS_ERROR_NO_AGGREGATION;
-    }
-    return this.QueryInterface(iid);
-  }
-
-  /**
-   * Register this about: page with XPCOM. This must be called once in each
-   * process (parent and content) to correctly initialize the page.
-   */
-  register() {
-    Cm.QueryInterface(Ci.nsIComponentRegistrar).registerFactory(
-      this.classId,
-      this.description,
-      `@mozilla.org/network/protocol/about;1?what=${this.aboutHost}`,
-      this,
-    );
-  }
-
-  /**
-   * Unregister this about: page with XPCOM. This must be called before the
-   * add-on is cleaned up if the page has been registered.
-   */
-  unregister() {
-    Cm.QueryInterface(Ci.nsIComponentRegistrar).unregisterFactory(this.classId, this);
-  }
 }
 AboutPage.prototype.QueryInterface = ChromeUtils.generateQI([Ci.nsIAboutModule]);
 
 /**
  * The module exported by this file.
  */
 var AboutPages = {
   async init() {
     // Load scripts in content processes and tabs
-    Services.ppmm.loadProcessScript(PROCESS_SCRIPT, true);
     Services.mm.loadFrameScript(FRAME_SCRIPT, true);
 
     // Register about: pages and their listeners
-    this.aboutStudies.register();
     this.aboutStudies.registerParentListeners();
 
     CleanupManager.addCleanupHandler(() => {
       // Stop loading processs scripts and notify existing scripts to clean up.
-      Services.ppmm.removeDelayedProcessScript(PROCESS_SCRIPT);
       Services.ppmm.broadcastAsyncMessage("Shield:ShuttingDown");
       Services.mm.removeDelayedFrameScript(FRAME_SCRIPT);
       Services.mm.broadcastAsyncMessage("Shield:ShuttingDown");
 
       // Clean up about pages
       this.aboutStudies.unregisterParentListeners();
-      this.aboutStudies.unregister();
     });
   },
 };
 
 /**
  * about:studies page for displaying in-progress and past Shield studies.
  * @type {AboutPage}
  * @implements {nsIMessageListener}
  */
 XPCOMUtils.defineLazyGetter(this.AboutPages, "aboutStudies", () => {
   const aboutStudies = new AboutPage({
     chromeUrl: "resource://normandy-content/about-studies/about-studies.html",
     aboutHost: "studies",
-    classId: "{6ab96943-a163-482c-9622-4faedc0e827f}",
+    classID: "{6ab96943-a163-482c-9622-4faedc0e827f}",
     description: "Shield Study Listing",
     uriFlags: (
       Ci.nsIAboutModule.ALLOW_SCRIPT
       | Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT
       | Ci.nsIAboutModule.URI_MUST_LOAD_IN_CHILD
     ),
   });
 
--- a/toolkit/components/normandy/moz.build
+++ b/toolkit/components/normandy/moz.build
@@ -4,12 +4,17 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 with Files('**'):
     BUG_COMPONENT = ('Firefox', 'Normandy Client')
 
 JAR_MANIFESTS += ['jar.mn']
 
+EXTRA_COMPONENTS += [
+    'shield-content-process.js',
+    'shield.manifest',
+]
+
 SPHINX_TREES['normandy'] = 'docs'
 
 BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini']
 XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']
rename from toolkit/components/normandy/content/shield-content-process.js
rename to toolkit/components/normandy/shield-content-process.js
--- a/toolkit/components/normandy/content/shield-content-process.js
+++ b/toolkit/components/normandy/shield-content-process.js
@@ -7,41 +7,20 @@
  * Registers about: pages provided by Shield, and listens for a shutdown event
  * from the add-on before un-registering them.
  *
  * This file is loaded as a process script. It is executed once for each
  * process, including the parent one.
  */
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://normandy-content/AboutPages.jsm");
 
-class ShieldChildListener {
-  onStartup() {
-    Services.cpmm.addMessageListener("Shield:ShuttingDown", this, true);
-    AboutPages.aboutStudies.register();
-  }
-
-  onShutdown() {
-    AboutPages.aboutStudies.unregister();
-    Services.cpmm.removeMessageListener("Shield:ShuttingDown", this);
-
-    // Unload AboutPages.jsm in case the add-on is reinstalled and we need to
-    // load a new version of it.
-    Cu.unload("resource://normandy-content/AboutPages.jsm");
-  }
+// generateNSGetFactory only knows how to register factory classes, with
+// classID properties on their prototype, and a cnstructor that returns
+// an instance. It can't handle singletons directly. So wrap the
+// aboutStudies singleton in a trivial constructor function.
+function AboutStudies() {
+  return AboutStudies.prototype;
+}
+AboutStudies.prototype = AboutPages.aboutStudies;
 
-  receiveMessage(message) {
-    switch (message.name) {
-      case "Shield:ShuttingDown":
-        this.onShutdown();
-        break;
-    }
-  }
-}
-
-// Only register in content processes; the parent process handles registration
-// separately.
-if (Services.appinfo.processType === Services.appinfo.PROCESS_TYPE_CONTENT) {
-  const listener = new ShieldChildListener();
-  listener.onStartup();
-}
+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutStudies]);
new file mode 100644
--- /dev/null
+++ b/toolkit/components/normandy/shield.manifest
@@ -0,0 +1,3 @@
+
+component {6ab96943-a163-482c-9622-4faedc0e827f} shield-content-process.js
+contract @mozilla.org/network/protocol/about;1?what=studies {6ab96943-a163-482c-9622-4faedc0e827f}