Merge fx-sync

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();
+  });
 }