Bug 836120 - Reduce memory overhead of Sync when it's not configured; r=rnewman
authorGregory Szorc <gps@mozilla.com>
Wed, 30 Jan 2013 07:05:12 -0800
changeset 120358 bb8895c7f091e0cf64b200da9779d890badbcd0f
parent 120357 6a2236d7b359ef486974101024f10aafcf8c29ed
child 120359 34a80b4e0acdaeb9c4d5c6e7741df0eadde874c6
push id24249
push usergszorc@mozilla.com
push dateThu, 31 Jan 2013 00:12:11 +0000
treeherdermozilla-central@20bbf73921f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs836120, 825728
milestone21.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 836120 - Reduce memory overhead of Sync when it's not configured; r=rnewman If Sync is (likely) not configured, the only loaded JS is for the XPCOM service itself. The UI code is now smart enough to initialize the Sync service if it isn't yet loaded. This addresses bug 825728.
browser/base/content/browser-syncui.js
browser/components/preferences/in-content/sync.js
browser/components/preferences/sync.js
mobile/xul/chrome/content/sync.js
services/sync/Weave.js
services/sync/modules/service.js
services/sync/tests/unit/test_service_startup.js
--- a/browser/base/content/browser-syncui.js
+++ b/browser/base/content/browser-syncui.js
@@ -18,17 +18,20 @@ let gSyncUI = {
          "weave:ui:clear-error",
   ],
 
   _unloaded: false,
 
   init: function SUI_init() {
     // Proceed to set up the UI if Sync has already started up.
     // Otherwise we'll do it when Sync is firing up.
-    if (Weave.Status.ready) {
+    let xps = Components.classes["@mozilla.org/weave/service;1"]
+                                .getService(Components.interfaces.nsISupports)
+                                .wrappedJSObject;
+    if (xps.ready) {
       this.initUI();
       return;
     }
 
     Services.obs.addObserver(this, "weave:service:ready", true);
 
     // Remove the observer if the window is closed before the observer
     // was triggered.
--- a/browser/components/preferences/in-content/sync.js
+++ b/browser/components/preferences/in-content/sync.js
@@ -28,16 +28,47 @@ let gSyncPane = {
   needsUpdate: function () {
     this.page = PAGE_NEEDS_UPDATE;
     let label = document.getElementById("loginError");
     label.value = Weave.Utils.getErrorString(Weave.Status.login);
     label.className = "error";
   },
 
   init: function () {
+    // If the Service hasn't finished initializing, wait for it.
+    let xps = Components.classes["@mozilla.org/weave/service;1"]
+                                .getService(Components.interfaces.nsISupports)
+                                .wrappedJSObject;
+
+    if (xps.ready) {
+      this._init();
+      return;
+    }
+
+    let onUnload = function () {
+      window.removeEventListener("unload", onUnload, false);
+      try {
+        Services.obs.removeObserver(onReady, "weave:service:ready");
+      } catch (e) {}
+    };
+
+    let onReady = function () {
+      Services.obs.removeObserver(onReady, "weave:service:ready");
+      window.removeEventListener("unload", onUnload, false);
+      this._init();
+    }.bind(this);
+
+
+    Services.obs.addObserver(onReady, "weave:service:ready", false);
+    window.addEventListener("unload", onUnload, false);
+
+    xps.ensureLoaded();
+  },
+
+  _init: function () {
     let topics = ["weave:service:login:error",
                   "weave:service:login:finish",
                   "weave:service:start-over",
                   "weave:service:setup-complete",
                   "weave:service:logout:finish"];
 
     // Add the observers now and remove them on unload
     //XXXzpao This should use Services.obs.* but Weave's Obs does nice handling
--- a/browser/components/preferences/sync.js
+++ b/browser/components/preferences/sync.js
@@ -29,16 +29,46 @@ let gSyncPane = {
   needsUpdate: function () {
     this.page = PAGE_NEEDS_UPDATE;
     let label = document.getElementById("loginError");
     label.value = Weave.Utils.getErrorString(Weave.Status.login);
     label.className = "error";
   },
 
   init: function () {
+    // If the Service hasn't finished initializing, wait for it.
+    let xps = Components.classes["@mozilla.org/weave/service;1"]
+                                .getService(Components.interfaces.nsISupports)
+                                .wrappedJSObject;
+
+    if (xps.ready) {
+      this._init();
+      return;
+    }
+
+    let onUnload = function () {
+      window.removeEventListener("unload", onUnload, false);
+      try {
+        Services.obs.removeObserver(onReady, "weave:service:ready");
+      } catch (e) {}
+    };
+
+    let onReady = function () {
+      Services.obs.removeObserver(onReady, "weave:service:ready");
+      window.removeEventListener("unload", onUnload, false);
+      this._init();
+    }.bind(this);
+
+    Services.obs.addObserver(onReady, "weave:service:ready", false);
+    window.addEventListener("unload", onUnload, false);
+
+    xps.ensureLoaded();
+  },
+
+  _init: function () {
     let topics = ["weave:service:login:error",
                   "weave:service:login:finish",
                   "weave:service:start-over",
                   "weave:service:setup-complete",
                   "weave:service:logout:finish"];
 
     // Add the observers now and remove them on unload
     //XXXzpao This should use Services.obs.* but Weave's Obs does nice handling
--- a/mobile/xul/chrome/content/sync.js
+++ b/mobile/xul/chrome/content/sync.js
@@ -9,19 +9,38 @@ let WeaveGlue = {
   jpake: null,
   _bundle: null,
   _loginError: false,
   _progressBar: null,
   _progressValue: 0,
   _progressMax: null,
 
   init: function init() {
-    if (this._bundle)
+    if (this._bundle) {
+      return;
+    }
+
+    let service = Components.classes["@mozilla.org/weave/service;1"]
+                                    .getService(Components.interfaces.nsISupports)
+                                    .wrappedJSObject;
+
+    if (service.ready) {
+      this._init();
       return;
+    }
 
+    Services.obs.addObserver(function onReady() {
+      Services.obs.removeObserver(onReady, "weave:service:ready");
+      this._init();
+    }.bind(this), "weave:service:ready", false);
+
+    service.ensureLoaded();
+  },
+
+  _init: function () {
     this._bundle = Services.strings.createBundle("chrome://browser/locale/sync.properties");
     this._msg = document.getElementById("prefs-messages");
 
     this._addListeners();
 
     this.setupData = { account: "", password: "" , synckey: "", serverURL: "" };
 
     if (Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED) {
--- a/services/sync/Weave.js
+++ b/services/sync/Weave.js
@@ -5,41 +5,98 @@
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/FileUtils.jsm");
 
+const SYNC_PREFS_BRANCH = "services.sync.";
+
+
+/**
+ * Sync's XPCOM service.
+ *
+ * It is named "Weave" for historical reasons.
+ *
+ * It's worth noting how Sync is lazily loaded. We register a timer that
+ * loads Sync a few seconds after app startup. This is so Sync does not
+ * adversely affect application start time.
+ *
+ * If Sync is not configured, no extra Sync code is loaded. If an
+ * external component (say the UI) needs to interact with Sync, it
+ * should do something like the following:
+ *
+ * // 1. Grab a handle to the Sync XPCOM service.
+ * let service = Cc["@mozilla.org/weave/service;1"]
+ *                 .getService(Components.interfaces.nsISupports)
+ *                 .wrappedJSObject;
+ *
+ * // 2. Check if the service has been initialized.
+ * if (service.ready) {
+ *   // You are free to interact with "Weave." objects.
+ *   return;
+ * }
+ *
+ * // 3. Install "ready" listener.
+ * Services.obs.addObserver(function onReady() {
+ *   Services.obs.removeObserver(onReady, "weave:service:ready");
+ *
+ *   // You are free to interact with "Weave." objects.
+ * }, "weave:service:ready", false);
+ *
+ * // 4. Trigger loading of Sync.
+ * service.ensureLoaded();
+ */
 function WeaveService() {
   this.wrappedJSObject = this;
+  this.ready = false;
 }
 WeaveService.prototype = {
   classID: Components.ID("{74b89fb0-f200-4ae8-a3ec-dd164117f6de}"),
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
                                          Ci.nsISupportsWeakReference]),
 
-  observe: function BSS__observe(subject, topic, data) {
+  ensureLoaded: function () {
+    Components.utils.import("resource://services-sync/main.js");
+
+    // Side-effect of accessing the service is that it is instantiated.
+    Weave.Service;
+  },
+
+  observe: function (subject, topic, data) {
     switch (topic) {
     case "app-startup":
       let os = Cc["@mozilla.org/observer-service;1"].
                getService(Ci.nsIObserverService);
       os.addObserver(this, "final-ui-startup", true);
       break;
 
     case "final-ui-startup":
       // Force Weave service to load if it hasn't triggered from overlays
       this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
       this.timer.initWithCallback({
         notify: function() {
-          Cu.import("resource://services-sync/main.js");
-          if (Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED)
-            Weave.Service;
+          // We only load more if it looks like Sync is configured.
+          let prefs = Services.prefs.getBranch(SYNC_PREFS_BRANCH);
+
+          if (!prefs.prefHasUserValue("username")) {
+            return;
+          }
+
+          // We have a username. So, do a more thorough check. This will
+          // import a number of modules and thus increase memory
+          // accordingly. We could potentially copy code performed by
+          // this check into this file if our above code is yielding too
+          // many false positives.
+          if (Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED) {
+            this.ensureLoaded();
+          }
         }
       }, 10000, Ci.nsITimer.TYPE_ONE_SHOT);
       break;
     }
   }
 };
 
 function AboutWeaveLog() {}
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -357,16 +357,24 @@ Sync11Service.prototype = {
       Svc.Obs.notify("weave:engine:start-tracking");
     }
 
     // Send an event now that Weave service is ready.  We don't do this
     // synchronously so that observers can import this module before
     // registering an observer.
     Utils.nextTick(function onNextTick() {
       this.status.ready = true;
+
+      // UI code uses the flag on the XPCOM service so it doesn't have
+      // to load a bunch of modules.
+      let xps = Cc["@mozilla.org/weave/service;1"]
+                  .getService(Ci.nsISupports)
+                  .wrappedJSObject;
+      xps.ready = true;
+
       Svc.Obs.notify("weave:service:ready");
     }.bind(this));
   },
 
   _checkSetup: function _checkSetup() {
     if (!this.enabled) {
       return this.status.service = STATUS_DISABLED;
     }
--- a/services/sync/tests/unit/test_service_startup.js
+++ b/services/sync/tests/unit/test_service_startup.js
@@ -25,17 +25,24 @@ function run_test() {
 
   _("Engines are registered.");
   let engines = Service.engineManager.getAll();
   do_check_true(Utils.deepEquals([engine.name for each (engine in engines)],
                                  ['tabs', 'bookmarks', 'forms', 'history']));
 
   _("Observers are notified of startup");
   do_test_pending();
+
+  let xps = Cc["@mozilla.org/weave/service;1"]
+              .getService(Ci.nsISupports)
+              .wrappedJSObject;
+
   do_check_false(Service.status.ready);
+  do_check_false(xps.ready);
   Observers.add("weave:service:ready", function (subject, data) {
     do_check_true(Service.status.ready);
+    do_check_true(xps.ready);
 
     // Clean up.
     Svc.Prefs.resetBranch("");
     do_test_finished();
   });
 }