diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 51b54516cea8..07ed922d8c4b 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/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); } diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index 1574a833168a..c9bfecdbfd40 100644 --- a/services/sync/modules/service.js +++ b/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() { diff --git a/services/sync/modules/status.js b/services/sync/modules/status.js index 442352aa4fae..eb3f94a61ed9 100644 --- a/services/sync/modules/status.js +++ b/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; diff --git a/services/sync/tests/unit/test_service_startup.js b/services/sync/tests/unit/test_service_startup.js index 0abea079947f..4ae272830a8a 100644 --- a/services/sync/tests/unit/test_service_startup.js +++ b/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(); + }); }