Bug 586094 - Enabling Sync regresses Ts on all platforms, r=dolske, a=beltzner
authorMike Connor <mconnor@mozilla.com>
Wed, 11 Aug 2010 21:28:27 -0400
changeset 50353 6b5c4e509001e92aa394737ff6573abbb66f191d
parent 50352 8b618223ad693d124463c9ee15d6cb05b13e1cf7
child 50354 dd73091a6315087715e49b635db2a8d927f28d7c
push id15043
push usermconnor@mozilla.com
push dateFri, 13 Aug 2010 00:05:39 +0000
treeherdermozilla-central@dd73091a6315 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdolske, beltzner
bugs586094
milestone2.0b4pre
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 586094 - Enabling Sync regresses Ts on all platforms, r=dolske, a=beltzner
browser/base/content/browser-menubar.inc
browser/base/content/browser-syncui.js
browser/base/content/browser.xul
browser/components/nsBrowserGlue.js
--- a/browser/base/content/browser-menubar.inc
+++ b/browser/base/content/browser-menubar.inc
@@ -589,17 +589,21 @@
       </menu>
       <menuseparator/>
     </menupopup>
   </menu>
 
             <menu id="tools-menu"
                   label="&toolsMenu.label;"
                   accesskey="&toolsMenu.accesskey;">
-              <menupopup id="menu_ToolsPopup">
+              <menupopup id="menu_ToolsPopup"
+#ifdef MOZ_SERVICES_SYNC
+                         onpopupshowing="gSyncUI.updateUI();"
+#endif
+                         >
               <menuitem id="menu_search"
                         label="&search.label;"
                         accesskey="&search.accesskey;"
                         key="key_search"
                         command="Tools:Search"/>
               <menuseparator id="browserToolsSeparator"/>
               <menuitem id="menu_openDownloads"
                         label="&downloads.label;"
--- a/browser/base/content/browser-syncui.js
+++ b/browser/base/content/browser-syncui.js
@@ -35,57 +35,50 @@
 # the provisions above, a recipient may use your version of this file under
 # the terms of any one of the MPL, the GPL or the LGPL.
 #
 # ***** END LICENSE BLOCK *****
 
 // gSyncUI handles updating the tools menu
 let gSyncUI = {
   init: function SUI_init() {
-    let obs = [["weave:service:sync:start", "onActivityStart"],
-               ["weave:service:sync:finish", "onSyncFinish"],
-               ["weave:service:sync:error", "onSyncError"],
-               ["weave:service:sync:delayed", "onSyncDelay"],
-               ["weave:service:setup-complete", "onLoginFinish"],
-               ["weave:service:login:start", "onActivityStart"],
-               ["weave:service:login:finish", "onLoginFinish"],
-               ["weave:service:login:error", "onLoginError"],
-               ["weave:service:logout:finish", "onLogout"],
-               ["weave:service:start-over", "onStartOver"]];
+    // this will be the first notification fired during init
+    // we can set up everything else later
+    Services.obs.addObserver(this, "weave:service:ready", true);
+  },
+  initUI: function SUI_initUI() {
+    let obs = ["weave:service:sync:start",
+               "weave:service:sync:finish",
+               "weave:service:sync:error",
+               "weave:service:sync:delayed",
+               "weave:service:setup-complete",
+               "weave:service:login:start",
+               "weave:service:login:finish",
+               "weave:service:login:error",
+               "weave:service:logout:finish",
+               "weave:service:start-over"];
 
     // If this is a browser window?
     if (gBrowser) {
-      obs.push(["weave:notification:added", "onNotificationAdded"],
-               ["weave:notification:removed", "onNotificationRemoved"]);
+      obs.push("weave:notification:added", "weave:notification:removed");
     }
 
-    // Add the observers now and remove them on unload
     let self = this;
-    let addRem = function(add) {
-      obs.forEach(function([topic, func]) {
-        //XXXzpao This should use Services.obs.* but Weave's Obs does nice handling
-        //        of `this`. Fix in a followup. (bug 583347)
-        if (add)
-          Weave.Svc.Obs.add(topic, self[func], self);
-        else
-          Weave.Svc.Obs.remove(topic, self[func], self);
-      });
-    };
-    addRem(true);
-    window.addEventListener("unload", function() addRem(false), false);
+    obs.forEach(function(topic) {
+      Services.obs.addObserver(self, topic, true);
+    });
 
     // Find the alltabs-popup, only if there is a gBrowser
     if (gBrowser) {
       let popup = document.getElementById("alltabs-popup");
       let self = this;
       popup.addEventListener("popupshowing", function() {
         self.alltabsPopupShowing();
       }, true);
     }
-
     this.updateUI();
   },
 
   _wasDelayed: false,
 
   _needsSetup: function SUI__needsSetup() {
     let firstSync = "";
     try {
@@ -237,17 +230,16 @@ let gSyncUI = {
       document.getElementById("sync-notifications-button").hidden = true;
     }
     else {
       // Display remaining notifications
       this.onNotificationAdded();
     }
   },
 
-
   // Commands
   doUpdateMenu: function SUI_doUpdateMenu(event) {
     this._updateLastSyncItem();
 
     let loginItem = document.getElementById("sync-loginitem");
     let logoutItem = document.getElementById("sync-logoutitem");
     let syncItem = document.getElementById("sync-syncnowitem");
 
@@ -364,17 +356,60 @@ let gSyncUI = {
     if (this._wasDelayed && Weave.Status.sync != Weave.NO_SYNC_NODE_FOUND) {
       title = this._stringBundle.GetStringFromName("error.sync.no_node_found.title");
       Weave.Notifications.removeAll(title);
       this._wasDelayed = false;
     }
 
     this.updateUI();
     this._updateLastSyncItem();
-  }
+  },
+  
+  observe: function SUI_observe(subject, topic, data) {
+    switch (topic) {
+      case "weave:service:sync:start":
+        this.onActivityStart();
+        break;
+      case "weave:service:sync:finish":
+        this.onSyncFinish();
+        break;
+      case "weave:service:sync:error":
+        this.onSyncError();
+        break;
+      case "weave:service:sync:delayed":
+        this.onSyncDelay();
+        break;
+      case "weave:service:setup-complete":
+        this.onLoginFinish();
+        break;
+      case "weave:service:login:start":
+        this.onActivityStart();
+        break;
+      case "weave:service:login:finish":
+        this.onLoginFinish();
+        break;
+      case "weave:service:login:error":
+        this.onLoginError();
+        break;
+      case "weave:service:logout:finish":
+        this.onLogout();
+        break;
+      case "weave:service:start-over":
+        this.onStartOver();
+        break;
+      case "weave:service:ready":
+        this.initUI();
+        break;
+    }
+  },
+
+  QueryInterface: XPCOMUtils.generateQI([
+    Ci.nsIObserver,
+    Ci.nsISupportsWeakReference
+  ])
 };
 
 XPCOMUtils.defineLazyGetter(gSyncUI, "_stringBundle", function() {
   //XXXzpao these strings should probably be moved from /services to /browser... (bug 583381)
   //        but for now just make it work
   return Cc["@mozilla.org/intl/stringbundle;1"].
          getService(Ci.nsIStringBundleService).
          createBundle("chrome://weave/locale/services/sync.properties");
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -1056,17 +1056,17 @@
       <statusbarpanel id="download-monitor" class="statusbarpanel-iconic-text"
                       tooltiptext="&downloadMonitor2.tooltip;" hidden="true"
                       command="Tools:Downloads"/>
       <statusbarpanel id="security-button" class="statusbarpanel-iconic"
                       hidden="true"
                       onclick="if (event.button == 0 &amp;&amp; event.detail == 1) displaySecurityInfo();"/>
 #ifdef MOZ_SERVICES_SYNC
       <statusbarpanel id="sync-status-button"
-                      class="statusbarpanel-iconic-text"
+                      class="statusbarpanel-iconic"
                       image="chrome://browser/skin/sync-16.png"
                       label="&syncLogInItem.label;"
                       oncommand="gSyncUI.handleStatusbarButton();"
                       onmousedown="event.preventDefault();">
       </statusbarpanel>
       <separator class="thin"/>
       <statusbarpanel id="sync-notifications-button"
                       class="statusbarpanel-iconic-text"
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -123,16 +123,41 @@ BrowserGlue.prototype = {
     Services.prefs.setBoolPref("browser.sessionstore.resume_session_once", true);
 
     // This method can be called via [NSApplication terminate:] on Mac, which
     // ends up causing prefs not to be flushed to disk, so we need to do that
     // explicitly here. See bug 497652.
     Services.prefs.savePrefFile(null);
   },
 
+#ifdef MOZ_SERVICES_SYNC
+  _setSyncAutoconnectDelay: function BG__setSyncAutoconnectDelay() {
+    // Assume that a non-zero value for services.sync.autoconnectDelay should override
+    if (Services.prefs.prefHasUserValue("services.sync.autoconnectDelay")) {
+      let prefDelay = Services.prefs.getIntPref("services.sync.autoconnectDelay");
+
+      if (prefDelay > 0)
+        return;
+    }
+
+    // delays are in seconds
+    const MAX_DELAY = 300;
+    let delay = 3;
+    let enum = Services.wm.getEnumerator("navigator:browser");
+    while (enum.hasMoreElements()) {
+      delay += enum.getNext().gBrowser.tabs.length;
+    }
+    delay = delay <= MAX_DELAY ? delay : MAX_DELAY;
+
+    let syncTemp = {};
+    Cu.import("resource://services-sync/service.js", syncTemp);
+    syncTemp.Weave.Service.delayedAutoConnect(delay);
+  },
+#endif
+
   // nsIObserver implementation 
   observe: function BG_observe(subject, topic, data) {
     switch (topic) {
       case "xpcom-shutdown":
         this._dispose();
         break;
       case "prefservice:after-app-defaults":
         this._onAppDefaults();
@@ -161,16 +186,21 @@ BrowserGlue.prototype = {
         // The application is not actually quitting, but the last full browser
         // window is about to be closed.
         this._onQuitRequest(subject, "lastwindow");
         break;
       case "browser-lastwindow-close-granted":
         this._setPrefToSaveSession();
         break;
 #endif
+#ifdef MOZ_SERVICES_SYNC
+      case "weave:service:ready":
+        this._setSyncAutoconnectDelay();
+        break;
+#endif
       case "session-save":
         this._setPrefToSaveSession(true);
         subject.QueryInterface(Ci.nsISupportsPRBool);
         subject.data = true;
         break;
       case "places-init-complete":
         this._initPlaces();
         Services.obs.removeObserver(this, "places-init-complete");
@@ -234,16 +264,19 @@ BrowserGlue.prototype = {
     os.addObserver(this, "sessionstore-windows-restored", false);
     os.addObserver(this, "browser:purge-session-history", false);
     os.addObserver(this, "quit-application-requested", false);
     os.addObserver(this, "quit-application-granted", false);
 #ifdef OBSERVE_LASTWINDOW_CLOSE_TOPICS
     os.addObserver(this, "browser-lastwindow-close-requested", false);
     os.addObserver(this, "browser-lastwindow-close-granted", false);
 #endif
+#ifdef MOZ_SERVICES_SYNC
+    os.addObserver(this, "weave:service:ready", false);
+#endif
     os.addObserver(this, "session-save", false);
     os.addObserver(this, "places-init-complete", false);
     this._isPlacesInitObserver = true;
     os.addObserver(this, "places-database-locked", false);
     this._isPlacesLockedObserver = true;
     os.addObserver(this, "distribution-customization-complete", false);
     os.addObserver(this, "places-shutdown", false);
     this._isPlacesShutdownObserver = true;
@@ -258,16 +291,19 @@ BrowserGlue.prototype = {
     os.removeObserver(this, "sessionstore-windows-restored");
     os.removeObserver(this, "browser:purge-session-history");
     os.removeObserver(this, "quit-application-requested");
     os.removeObserver(this, "quit-application-granted");
 #ifdef OBSERVE_LASTWINDOW_CLOSE_TOPICS
     os.removeObserver(this, "browser-lastwindow-close-requested");
     os.removeObserver(this, "browser-lastwindow-close-granted");
 #endif
+#ifdef MOZ_SERVICES_SYNC
+    os.removeObserver(this, "weave:service:ready", false);
+#endif
     os.removeObserver(this, "session-save");
     if (this._isIdleObserver)
       this._idleService.removeIdleObserver(this, BOOKMARKS_BACKUP_IDLE_TIME);
     if (this._isPlacesInitObserver)
       os.removeObserver(this, "places-init-complete");
     if (this._isPlacesLockedObserver)
       os.removeObserver(this, "places-database-locked");
     if (this._isPlacesShutdownObserver)
@@ -381,39 +417,16 @@ BrowserGlue.prototype = {
     if (WINTASKBAR_CONTRACTID in Cc &&
         Cc[WINTASKBAR_CONTRACTID].getService(Ci.nsIWinTaskbar).available) {
       let temp = {};
       Cu.import("resource://gre/modules/WindowsJumpLists.jsm", temp);
       temp.WinTaskbarJumpList.startup();
     }
 #endif
 #endif
-
-#ifdef MOZ_SERVICES_SYNC
-    // Assume that a non-zero value for services.sync.autoconnectDelay should override
-    if (Services.prefs.prefHasUserValue("services.sync.autoconnectDelay")) {
-      let prefDelay = Services.prefs.getIntPref("services.sync.autoconnectDelay");
-
-      if (prefDelay > 0)
-        return;
-    }
-
-    // delays are in seconds
-    const MAX_DELAY = 300;
-    let delay = 3;
-    let enum = Services.wm.getEnumerator("navigator:browser");
-    while (enum.hasMoreElements()) {
-      delay += enum.getNext().gBrowser.tabs.length;
-    }
-    delay = delay <= MAX_DELAY ? delay : MAX_DELAY;
-
-    let syncTemp = {};
-    Cu.import("resource://services-sync/service.js", syncTemp);
-    syncTemp.Weave.Service.delayedAutoConnect(delay);
-#endif
   },
 
   _onQuitRequest: function BG__onQuitRequest(aCancelQuit, aQuitType) {
     // If user has already dismissed quit request, then do nothing
     if ((aCancelQuit instanceof Ci.nsISupportsPRBool) && aCancelQuit.data)
       return;
 
     var windowcount = 0;
@@ -1382,13 +1395,13 @@ GeolocationPrompt.prototype = {
     }
 
     var requestingWindow = request.requestingWindow.top;
     var chromeWin = getChromeWindow(requestingWindow).wrappedJSObject;
     var browser = chromeWin.gBrowser.getBrowserForDocument(requestingWindow.document);
 
     chromeWin.PopupNotifications.show(browser, "geolocation", message, "geo-notification-icon",
                                       mainAction, secondaryActions);
-  },
+  }
 };
 
 var components = [BrowserGlue, GeolocationPrompt];
 var NSGetFactory = XPCOMUtils.generateNSGetFactory(components);