Merge places with mozilla-central. a=merge

browser/base/content/browser-syncui.js

@@ -41,8 +41,13 @@
 // gSyncUI handles updating the tools menu
 let gSyncUI = {
   init: function SUI_init() {
-    // this will be the first notification fired during init
-    // we can set up everything else later
+    // 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) {
+      this.initUI();
+      return;
+    }
+
     Services.obs.addObserver(this, "weave:service:ready", true);
 
     // Remove the observer if the window is closed before the observer
@@ -52,6 +57,7 @@ let gSyncUI = {
       Services.obs.removeObserver(gSyncUI, "weave:service:ready");
     }, false);
   },
+
   initUI: function SUI_initUI() {
     let obs = ["weave:service:sync:start",
                "weave:service:sync:finish",
@@ -70,19 +76,16 @@ let gSyncUI = {
       obs.push("weave:notification:added");
     }
 
-    let self = this;
     obs.forEach(function(topic) {
-      Services.obs.addObserver(self, topic, true);
-    });
+      Services.obs.addObserver(this, topic, true);
+    }, this);
 
     // Find the alltabs-popup, only if there is a gBrowser
     if (gBrowser) {
       let popup = document.getElementById("alltabs-popup");
       if (popup) {
-        let self = this;
-        popup.addEventListener("popupshowing", function() {
-          self.alltabsPopupShowing();
-        }, true);
+        popup.addEventListener(
+          "popupshowing", this.alltabsPopupShowing.bind(this), true);
       }
 
       if (Weave.Notifications.notifications.length)
@@ -118,12 +121,6 @@ let gSyncUI = {
            firstSync == "notReady";
   },
 
-  _isLoggedIn: function() {
-    if (this._needsSetup())
-      return false;
-    return Weave.Service.isLoggedIn;
-  },
-
   updateUI: function SUI_updateUI() {
     let needsSetup = this._needsSetup();
     document.getElementById("sync-setup-state").hidden = !needsSetup;
@@ -144,6 +141,8 @@ let gSyncUI = {
 
   alltabsPopupShowing: function(event) {
     // Should we show the menu item?
+    //XXXphilikon We should remove the check for isLoggedIn here and have
+    //            about:sync-tabs auto-login (bug 583344)
     if (!Weave.Service.isLoggedIn || !Weave.Engines.get("tabs").enabled)
       return;
 

services/sync/modules/engines/bookmarks.js

@@ -825,6 +825,7 @@ BookmarksStore.prototype = {
       // Reorder children according to the GUID list. Gracefully deal
       // with missing items, e.g. locally deleted.
       let delta = 0;
+      let parent = null;
       for (let idx = 0; idx < children.length; idx++) {
         let itemid = this.idForGUID(children[idx]);
         if (itemid == -1) {
@@ -833,7 +834,12 @@ BookmarksStore.prototype = {
           continue;
         }
         try {
-          Svc.Bookmark.setItemIndex(itemid, idx - delta);
+          // This code path could be optimized by caching the parent earlier.
+          // Doing so should take in count any edge case due to reparenting
+          // or parent invalidations though.
+          if (!parent)
+            parent = Svc.Bookmark.getFolderIdForItem(itemid);
+          Svc.Bookmark.moveItem(itemid, parent, idx - delta);
         } catch (ex) {
           this._log.debug("Could not move item " + children[idx] + ": " + ex);
         }

services/sync/modules/service.js

@@ -433,9 +433,12 @@ WeaveSvc.prototype = {
     }
 
     // Send an event now that Weave service is ready.  We don't do this
-    // synchronously so that observers will definitely have access to the
-    // 'Weave' namespace.
-    Utils.delay(function() Svc.Obs.notify("weave:service:ready"), 0);
+    // synchronously so that observers can import this module before
+    // registering an observer.
+    Utils.delay(function() {
+      Status.ready = true;
+      Svc.Obs.notify("weave:service:ready");
+    }, 0);
   },
 
   _checkSetup: function WeaveSvc__checkSetup() {

services/sync/modules/status.js

@@ -42,6 +42,8 @@ const Cu = Components.utils;
 Cu.import("resource://services-sync/constants.js");
 
 let Status = {
+  ready: false,
+
   get login() this._login,
   set login(code) {
     this._login = code;

services/sync/tests/unit/test_service_startup.js

@@ -1,5 +1,5 @@
 Cu.import("resource://services-sync/ext/Observers.js");
-Cu.import("resource://services-sync/ext/Sync.js");
+Cu.import("resource://services-sync/status.js");
 Cu.import("resource://services-sync/identity.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-sync/engines.js");
@@ -8,34 +8,31 @@ function run_test() {
   _("When imported, Service.onStartup is called");
 
   // Test fixtures
-  let observerCalled = false;
-  Observers.add("weave:service:ready", function (subject, data) {
-    observerCalled = true;
-  });
   Svc.Prefs.set("registerEngines", "Tab,Bookmarks,Form,History");
   Svc.Prefs.set("username", "johndoe");
 
-  try {
-    Cu.import("resource://services-sync/service.js");
+  Cu.import("resource://services-sync/service.js");
 
-    _("Service is enabled.");
-    do_check_eq(Service.enabled, true);
+  _("Service is enabled.");
+  do_check_eq(Service.enabled, true);
 
-    _("Engines are registered.");
-    let engines = Engines.getAll();
-    do_check_true(Utils.deepEquals([engine.name for each (engine in engines)],
-                                   ['tabs', 'bookmarks', 'forms', 'history']));
+  _("Engines are registered.");
+  let engines = Engines.getAll();
+  do_check_true(Utils.deepEquals([engine.name for each (engine in engines)],
+                                 ['tabs', 'bookmarks', 'forms', 'history']));
 
-    _("Identities are registered.");
-    do_check_eq(ID.get('WeaveID').username, "johndoe");
-    do_check_eq(ID.get('WeaveCryptoID').username, "johndoe");
+  _("Identities are registered.");
+  do_check_eq(ID.get('WeaveID').username, "johndoe");
+  do_check_eq(ID.get('WeaveCryptoID').username, "johndoe");
 
-    _("Observers are notified of startup");
-    // Synchronize with Service.onStartup's async notification
-    Sync.sleep(0);
-    do_check_true(observerCalled);
+  _("Observers are notified of startup");
+  do_test_pending();
+  do_check_false(Status.ready);
+  Observers.add("weave:service:ready", function (subject, data) {
+    do_check_true(Status.ready);
 
-  } finally {
+    // Clean up.
     Svc.Prefs.resetBranch("");
-  }
+    do_test_finished();
+  });
 }