Code review

mozilla · Jun 12, 2017 · 623ebed · 623ebed
1 parent bc167ff
commit 623ebed
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 22 deletions.
diff --git a/recipe-client-addon/bootstrap.js b/recipe-client-addon/bootstrap.js
@@ -10,24 +10,21 @@ XPCOMUtils.defineLazyModuleGetter(this, "LogManager",
   "resource://shield-recipe-client/lib/LogManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ShieldRecipeClient",
   "resource://shield-recipe-client/lib/ShieldRecipeClient.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "AboutStudiesProtocol",
-  "resource://shield-recipe-client/lib/AboutStudiesProtocol.jsm");
 
 this.install = function() {};
 
 this.startup = async function() {
   await ShieldRecipeClient.startup();
-  AboutStudiesProtocol.register();
 };
 
 this.shutdown = function(data, reason) {
-  AboutStudiesProtocol.unregister();
   ShieldRecipeClient.shutdown(reason);
 
   // Unload add-on modules. We don't do this in ShieldRecipeClient so that
   // modules are not unloaded accidentally during tests.
   const log = LogManager.getLogger("bootstrap");
   const modules = [
+    "lib/AboutStudiesProtocol.jsm",
     "lib/ActionSandboxManager.jsm",
     "lib/CleanupManager.jsm",
     "lib/ClientEnvironment.jsm",

diff --git a/recipe-client-addon/lib/AboutStudies.xml → recipe-client-addon/lib/AboutStudies.html b/recipe-client-addon/lib/AboutStudies.xml → recipe-client-addon/lib/AboutStudies.html
diff --git a/recipe-client-addon/lib/AboutStudiesProtocol.jsm b/recipe-client-addon/lib/AboutStudiesProtocol.jsm
@@ -11,42 +11,49 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 this.EXPORTED_SYMBOLS = ["AboutStudiesProtocol"];
 
+
 /**
- * Component definition for the about:studies protocol handler.
- * Registers a component with the browser that establishes an `about:studies`
- * protocol handler. Navigating to `about:studies` displays the
- * `AboutStudies.xml` file.
+ * Required data for registering a protocol handler. This data is referred to
+ * both when creating the new channel, as well as actually registering the
+ * component factory.
  */
-function StudiesProtocolHandler() {}
-StudiesProtocolHandler.prototype = {
-  uri: Services.io.newURI("resource://shield-recipe-client/lib/AboutStudies.xml"),
+const protocolInfo = {
+  // The file/destination for the protocol.
+  uri: Services.io.newURI("resource://shield-recipe-client/lib/AboutStudies.html"),
+  // Other properties are used internally by the protocol handler.
   classDescription: "about:studies page module",
   classID: Components.ID("c7c3dd48-c1cf-4bbf-a5df-69eaf6cb27d9"),
   contractID: "@mozilla.org/network/protocol/about;1?what=studies",
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIStudiesProtocolHandler]),
+};
 
-  newChannel: (aURI) => {
+/**
+ * Component definition for the about:studies protocol handler.
+ * Registers a component with the browser that establishes an `about:studies`
+ * protocol handler. Navigating to `about:studies` displays `AboutStudies.html`.
+ */
+class StudiesProtocolHandler {
+  newChannel(uri) {
     let chan;
     try {
       chan = Services.io.newChannelFromURI2(
-        StudiesProtocolHandler.prototype.uri,
+        protocolInfo.uri,
         null,
         Services.scriptSecurityManager.getSystemPrincipal(),
         null,
         Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL,
-        Ci.nsIContentPolicy.TYPE_OTHER
+        Ci.nsIContentPolicy.TYPE_DOCUMENT
       );
-
-      chan.originalURI = aURI;
     } catch (ex) {
       throw new Error(`Error creating about:studies protocol - ${ex}`);
     }
 
     return chan;
-  },
+  }
 
-  getURIFlags: (aURI) => {}
-};
+  // Required by the protocol handler, despite not doing anything.
+  getURIFlags() {}
+}
 
 
 /**
@@ -81,7 +88,7 @@ const AboutStudiesProtocol = {
       classID,
       classDescription,
       contractID
-    } = StudiesProtocolHandler.prototype;
+    } = protocolInfo;
 
     // Actually register the component (and therefor protocol) with the browser.
     Cm.registerFactory(classID, classDescription, contractID, protocolFactory);
@@ -95,8 +102,7 @@ const AboutStudiesProtocol = {
    */
   unregister() {
     if (this.instance) {
-      const {classID} = StudiesProtocolHandler.prototype;
-
+      const {classID} = protocolInfo;
       Cm.unregisterFactory(classID, this.instance);
     }
 

diff --git a/recipe-client-addon/lib/ShieldRecipeClient.jsm b/recipe-client-addon/lib/ShieldRecipeClient.jsm
@@ -17,6 +17,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "CleanupManager",
   "resource://shield-recipe-client/lib/CleanupManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PreferenceExperiments",
   "resource://shield-recipe-client/lib/PreferenceExperiments.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "AboutStudiesProtocol",
+  "resource://shield-recipe-client/lib/AboutStudiesProtocol.jsm");
 
 this.EXPORTED_SYMBOLS = ["ShieldRecipeClient"];
 
@@ -82,6 +84,9 @@ this.ShieldRecipeClient = {
     }
 
     await RecipeRunner.init();
+
+    // Enable the about:studies page
+    AboutStudiesProtocol.register();
   },
 
   shutdown(reason) {
@@ -91,6 +96,8 @@ this.ShieldRecipeClient = {
     if (reason === REASONS.ADDON_DISABLE || reason === REASONS.ADDON_UNINSTALL) {
       Services.prefs.setBoolPref(PREF_SELF_SUPPORT_ENABLED, true);
     }
+
+    AboutStudiesProtocol.unregister();
   },
 
   setDefaultPrefs() {