Bug 1474410 - Move ActivityStream instantiation and uninit into AboutNewTab r=Mardak
authorUrsula Sarracini <usarracini@mozilla.com>
Mon, 16 Jul 2018 11:58:35 -0400
changeset 426798 f98112e495824b4d710ccedee63c8fc1f1aa4047
parent 426797 4aa2f74bad5b77dd8f8c7e6e0e9eacafd64ef95d
child 426799 0287f9144ec2f38ae2f8d9dd8f0dfc5745133514
push id34285
push userbtara@mozilla.com
push dateMon, 16 Jul 2018 21:58:41 +0000
treeherdermozilla-central@f6df6a982ee9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMardak
bugs1474410
milestone63.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 1474410 - Move ActivityStream instantiation and uninit into AboutNewTab r=Mardak MozReview-Commit-ID: 5PupSQra62m
browser/extensions/activity-stream/bootstrap.js
browser/modules/AboutNewTab.jsm
--- a/browser/extensions/activity-stream/bootstrap.js
+++ b/browser/extensions/activity-stream/bootstrap.js
@@ -1,91 +1,24 @@
 /* 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";
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-XPCOMUtils.defineLazyGlobalGetters(this, ["fetch"]);
 
 ChromeUtils.defineModuleGetter(this, "Services",
   "resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "resProto",
                                    "@mozilla.org/network/protocol;1?name=resource",
                                    "nsISubstitutingProtocolHandler");
 
 const RESOURCE_HOST = "activity-stream";
 
-const BROWSER_READY_NOTIFICATION = "sessionstore-windows-restored";
-const RESOURCE_BASE = "resource://activity-stream";
-
-let activityStream;
-let modulesToUnload = new Set();
-let waitingForBrowserReady = true;
-
-// Lazily load ActivityStream then find related modules to unload
-XPCOMUtils.defineLazyModuleGetter(this, "ActivityStream",
-  "resource://activity-stream/lib/ActivityStream.jsm", null, null, () => {
-    // Helper to fetch a resource directory listing and call back with each item
-    const processListing = async (uri, cb) => {
-      try {
-        (await (await fetch(uri)).text())
-          .split("\n").slice(2).forEach(line => cb(line.split(" ").slice(1)));
-      } catch (e) {
-        // Silently ignore listings that fail to load
-        // probably because the resource: has been unloaded
-      }
-    };
-
-    // Look for modules one level deeper than the top resource URI
-    processListing(RESOURCE_BASE, ([directory, , , type]) => {
-      if (type === "DIRECTORY") {
-        // Look into this directory for .jsm files
-        const subDir = `${RESOURCE_BASE}/${directory}`;
-        processListing(subDir, ([name]) => {
-          if (name && name.search(/\.jsm$/) !== -1) {
-            modulesToUnload.add(`${subDir}/${name}`);
-          }
-        });
-      }
-    });
-  });
-
-/**
- * init - Initializes an instance of ActivityStream. This could be called by
- *        the startup() function exposed by bootstrap.js.
- */
-function init() {
-  // Don't re-initialize
-  if (activityStream && activityStream.initialized) {
-    return;
-  }
-  activityStream = new ActivityStream();
-  try {
-    activityStream.init();
-  } catch (e) {
-    Cu.reportError(e);
-  }
-}
-
-/**
- * uninit - Uninitializes the activityStream instance, if it exsits.This could be
- *          called by the shutdown() function exposed by bootstrap.js.
- *
- * @param  {type} reason Reason for uninitialization. Could be uninstall, upgrade, or PREF_OFF
- */
-function uninit(reason) {
-  // Make sure to only uninit once in case both pref change and shutdown happen
-  if (activityStream) {
-    activityStream.uninit(reason);
-    activityStream = null;
-  }
-}
-
 /**
  * Check if an old pref has a custom value to migrate. Clears the pref so that
  * it's the default after migrating (to avoid future need to migrate).
  *
  * @param oldPrefName {string} Pref to check and migrate
  * @param cbIfNotDefault {function} Callback that gets the current pref value
  */
 function migratePref(oldPrefName, cbIfNotDefault) {
@@ -108,22 +41,24 @@ function migratePref(oldPrefName, cbIfNo
       break;
   }
 
   // Give the callback the current value then clear the pref
   cbIfNotDefault(Services.prefs[prefGetter](oldPrefName));
   Services.prefs.clearUserPref(oldPrefName);
 }
 
-/**
- * onBrowserReady - Continues startup of the add-on after browser is ready.
- */
-function onBrowserReady() {
-  waitingForBrowserReady = false;
-  init();
+// The functions below are required by bootstrap.js
+
+this.install = function install(data, reason) {};
+
+this.startup = function startup(data, reason) {
+  resProto.setSubstitutionWithFlags(RESOURCE_HOST,
+                                    Services.io.newURI("chrome/content/", null, data.resourceURI),
+                                    resProto.ALLOW_CONTENT_ACCESS);
 
   // Do a one time migration of Tiles about:newtab prefs that have been modified
   migratePref("browser.newtabpage.rows", rows => {
     // Just disable top sites if rows are not desired
     if (rows <= 0) {
       Services.prefs.setBoolPref("browser.newtabpage.activity-stream.feeds.topsites", false);
     } else {
       Services.prefs.setIntPref("browser.newtabpage.activity-stream.topSitesRows", rows);
@@ -135,62 +70,15 @@ function onBrowserReady() {
       Services.prefs.setBoolPref("browser.newtabpage.activity-stream.feeds.topsites", false);
     }
   });
 
   // Old activity stream topSitesCount pref showed 6 per row
   migratePref("browser.newtabpage.activity-stream.topSitesCount", count => {
     Services.prefs.setIntPref("browser.newtabpage.activity-stream.topSitesRows", Math.ceil(count / 6));
   });
-}
-
-/**
- * observe - nsIObserver callback to handle various browser notifications.
- */
-function observe(subject, topic, data) {
-  switch (topic) {
-    case BROWSER_READY_NOTIFICATION:
-      Services.obs.removeObserver(observe, BROWSER_READY_NOTIFICATION);
-      // Avoid running synchronously during this event that's used for timing
-      Services.tm.dispatchToMainThread(() => onBrowserReady());
-      break;
-  }
-}
-
-// The functions below are required by bootstrap.js
-
-this.install = function install(data, reason) {};
-
-this.startup = function startup(data, reason) {
-  resProto.setSubstitutionWithFlags(RESOURCE_HOST,
-                                    Services.io.newURI("chrome/content/", null, data.resourceURI),
-                                    resProto.ALLOW_CONTENT_ACCESS);
-
-  // Only start Activity Stream up when the browser UI is ready
-  if (Services.startup.startingUp) {
-    Services.obs.addObserver(observe, BROWSER_READY_NOTIFICATION);
-  } else {
-    // Handle manual install or automatic install after manual uninstall
-    onBrowserReady();
-  }
 };
 
 this.shutdown = function shutdown(data, reason) {
   resProto.setSubstitution(RESOURCE_HOST, null);
-
-  // Uninitialize Activity Stream
-  uninit(reason);
-
-  // Stop waiting for browser to be ready
-  if (waitingForBrowserReady) {
-    Services.obs.removeObserver(observe, BROWSER_READY_NOTIFICATION);
-  }
-
-  // Unload any add-on modules that might might have been imported
-  modulesToUnload.forEach(Cu.unload);
 };
 
-this.uninstall = function uninstall(data, reason) {
-  if (activityStream) {
-    activityStream.uninstall(reason);
-    activityStream = null;
-  }
-};
+this.uninstall = function uninstall(data, reason) {};
--- a/browser/modules/AboutNewTab.jsm
+++ b/browser/modules/AboutNewTab.jsm
@@ -4,72 +4,112 @@
 
 "use strict";
 
 var EXPORTED_SYMBOLS = [ "AboutNewTab" ];
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 
-ChromeUtils.defineModuleGetter(this, "AutoMigrate",
-  "resource:///modules/AutoMigrate.jsm");
-ChromeUtils.defineModuleGetter(this, "NewTabUtils",
-  "resource://gre/modules/NewTabUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "RemotePages",
-  "resource://gre/modules/RemotePageManager.jsm");
+XPCOMUtils.defineLazyModuleGetters(this, {
+  ActivityStream: "resource://activity-stream/lib/ActivityStream.jsm",
+  RemotePages: "resource://gre/modules/RemotePageManager.jsm"
+});
+
+const BROWSER_READY_NOTIFICATION = "sessionstore-windows-restored";
 
 var AboutNewTab = {
+  QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
+
+  // AboutNewTab
 
   pageListener: null,
 
   isOverridden: false,
 
+  activityStream: null,
+
+  /**
+   * init - Initializes an instance of Activity Stream if one doesn't exist already
+   *        and creates the instance of Remote Page Manager which Activity Stream
+   *        uses for message passing.
+   *
+   * @param {obj} pageListener - Optional argument. An existing instance of RemotePages
+   *                             which Activity Stream has previously made, and we
+   *                             would like to re-use.
+   */
   init(pageListener) {
     if (this.isOverridden) {
       return;
     }
+
+    // Since `init` can be called via `reset` at a later time with an existing
+    // pageListener, we want to only add the observer if we are initializing
+    // without this pageListener argument. This means it was the first call to `init`
+    if (!pageListener) {
+      Services.obs.addObserver(this, BROWSER_READY_NOTIFICATION);
+    }
+
     this.pageListener = pageListener || new RemotePages(["about:home", "about:newtab", "about:welcome"]);
-    this.pageListener.addMessageListener("NewTab:Customize", this.customize);
-    this.pageListener.addMessageListener("NewTab:MaybeShowMigrateMessage",
-      this.maybeShowMigrateMessage);
   },
 
-  maybeShowMigrateMessage({ target }) {
-    AutoMigrate.shouldShowMigratePrompt(target.browser).then((prompt) => {
-      if (prompt) {
-        AutoMigrate.showUndoNotificationBar(target.browser);
-      }
-    });
+  /**
+   * onBrowserReady - Continues the initialization of Activity Stream after browser is ready.
+   */
+  onBrowserReady() {
+    if (this.activityStream && this.activityStream.initialized) {
+       return;
+    }
+
+    this.activityStream = new ActivityStream();
+    try {
+      this.activityStream.init();
+    } catch (e) {
+      Cu.reportError(e);
+    }
   },
 
-  customize(message) {
-    NewTabUtils.allPages.enabled = message.data.enabled;
-    NewTabUtils.allPages.enhanced = message.data.enhanced;
-  },
+  /**
+   * uninit - Uninitializes Activity Stream if it exists, and destroys the pageListener
+   *        if it exists.
+   */
+  uninit() {
+    if (this.activityStream) {
+      this.activityStream.uninit();
+      this.activityStream = null;
+    }
 
-  uninit() {
     if (this.pageListener) {
       this.pageListener.destroy();
       this.pageListener = null;
     }
   },
 
   override(shouldPassPageListener) {
     this.isOverridden = true;
     const pageListener = this.pageListener;
     if (!pageListener)
       return null;
     if (shouldPassPageListener) {
       this.pageListener = null;
-      pageListener.removeMessageListener("NewTab:Customize", this.customize);
-      pageListener.removeMessageListener("NewTab:MaybeShowMigrateMessage",
-        this.maybeShowMigrateMessage);
       return pageListener;
     }
     this.uninit();
     return null;
   },
 
   reset(pageListener) {
     this.isOverridden = false;
     this.init(pageListener);
+  },
+
+  // nsIObserver implementation
+
+  observe(subject, topic, data) {
+    switch (topic) {
+      case BROWSER_READY_NOTIFICATION:
+        Services.obs.removeObserver(this, BROWSER_READY_NOTIFICATION);
+        // Avoid running synchronously during this event that's used for timing
+        Services.tm.dispatchToMainThread(() => this.onBrowserReady());
+        break;
+    }
   }
 };