Bug 696974 - Bookmarks engine: "invalid 'in' operand this._modified" in reconcile. r=gps
authorRichard Newman <rnewman@mozilla.com>
Sat, 22 Dec 2012 12:44:05 -0800
changeset 117368 b29575b9f9dd0c8859fe68d88ebabd88c6da2e13
parent 117367 bd2ac0175972d697750398f4b0600ae93789f3ae
child 117369 c80bd4e618c88b2966b97f9c89fd8e4c858cced7
push id24098
push userrnewman@mozilla.com
push dateThu, 03 Jan 2013 03:39:06 +0000
treeherdermozilla-central@6955309291ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs696974
milestone20.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 696974 - Bookmarks engine: "invalid 'in' operand this._modified" in reconcile. r=gps
services/sync/modules/engines.js
services/sync/tests/unit/test_engine.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -88,20 +88,26 @@ Tracker.prototype = {
       this._log.debug("Not saving changedIDs.");
       return;
     }
     Utils.namedTimer(function() {
       Utils.jsonSave("changes/" + this.file, this, this.changedIDs, cb);
     }, 1000, this, "_lazySave");
   },
 
-  loadChangedIDs: function T_loadChangedIDs() {
+  loadChangedIDs: function (cb) {
     Utils.jsonLoad("changes/" + this.file, this, function(json) {
-      if (json) {
+      if (json && (typeof(json) == "object")) {
         this.changedIDs = json;
+      } else {
+        this._log.warn("Changed IDs file " + this.file + " contains non-object value.");
+        json = null;
+      }
+      if (cb) {
+        cb.call(this, json);
       }
     });
   },
 
   // ignore/unignore specific IDs.  Useful for ignoring items that are
   // being processed, or that shouldn't be synced.
   // But note: not persisted to disk
 
--- a/services/sync/tests/unit/test_engine.js
+++ b/services/sync/tests/unit/test_engine.js
@@ -1,17 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-common/observers.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 
-
 function SteamStore(engine) {
   Store.call(this, "Steam", engine);
   this.wasWiped = false;
 }
 SteamStore.prototype = {
   __proto__: Store.prototype,
 
   wipe: function() {
@@ -59,58 +58,79 @@ let engineObserver = {
 };
 Observers.add("weave:engine:reset-client:start", engineObserver);
 Observers.add("weave:engine:reset-client:finish", engineObserver);
 Observers.add("weave:engine:wipe-client:start", engineObserver);
 Observers.add("weave:engine:wipe-client:finish", engineObserver);
 Observers.add("weave:engine:sync:start", engineObserver);
 Observers.add("weave:engine:sync:finish", engineObserver);
 
+function run_test() {
+  run_next_test();
+}
 
-function test_members() {
+add_test(function test_members() {
   _("Engine object members");
   let engine = new SteamEngine(Service);
   do_check_eq(engine.Name, "Steam");
   do_check_eq(engine.prefName, "steam");
   do_check_true(engine._store instanceof SteamStore);
   do_check_true(engine._tracker instanceof SteamTracker);
-}
+  run_next_test();
+});
 
-function test_score() {
+add_test(function test_score() {
   _("Engine.score corresponds to tracker.score and is readonly");
   let engine = new SteamEngine(Service);
   do_check_eq(engine.score, 0);
   engine._tracker.score += 5;
   do_check_eq(engine.score, 5);
 
   try {
     engine.score = 10;
   } catch(ex) {
     // Setting an attribute that has a getter produces an error in
     // Firefox <= 3.6 and is ignored in later versions.  Either way,
     // the attribute's value won't change.
   }
   do_check_eq(engine.score, 5);
-}
+  run_next_test();
+});
 
-function test_resetClient() {
+add_test(function test_resetClient() {
   _("Engine.resetClient calls _resetClient");
   let engine = new SteamEngine(Service);
   do_check_false(engine.wasReset);
 
   engine.resetClient();
   do_check_true(engine.wasReset);
   do_check_eq(engineObserver.topics[0], "weave:engine:reset-client:start");
   do_check_eq(engineObserver.topics[1], "weave:engine:reset-client:finish");
 
   engine.wasReset = false;
   engineObserver.reset();
-}
+  run_next_test();
+});
 
-function test_wipeClient() {
+add_test(function test_invalidChangedIDs() {
+  _("Test that invalid changed IDs on disk don't end up live.");
+  let engine = new SteamEngine(Service);
+  let tracker = engine._tracker;
+  tracker.changedIDs = 5;
+  tracker.saveChangedIDs(function onSaved() {
+      tracker.changedIDs = {placeholder: true};
+      tracker.loadChangedIDs(function onLoaded(json) {
+        do_check_null(json);
+        do_check_true(tracker.changedIDs.placeholder);
+        run_next_test();
+      });
+    });
+});
+
+add_test(function test_wipeClient() {
   _("Engine.wipeClient calls resetClient, wipes store, clears changed IDs");
   let engine = new SteamEngine(Service);
   do_check_false(engine.wasReset);
   do_check_false(engine._store.wasWiped);
   do_check_true(engine._tracker.addChangedID("a-changed-id"));
   do_check_true("a-changed-id" in engine._tracker.changedIDs);
 
   engine.wipeClient();
@@ -120,55 +140,49 @@ function test_wipeClient() {
   do_check_eq(engineObserver.topics[0], "weave:engine:wipe-client:start");
   do_check_eq(engineObserver.topics[1], "weave:engine:reset-client:start");
   do_check_eq(engineObserver.topics[2], "weave:engine:reset-client:finish");
   do_check_eq(engineObserver.topics[3], "weave:engine:wipe-client:finish");
 
   engine.wasReset = false;
   engine._store.wasWiped = false;
   engineObserver.reset();
-}
+  run_next_test();
+});
 
-function test_enabled() {
+add_test(function test_enabled() {
   _("Engine.enabled corresponds to preference");
   let engine = new SteamEngine(Service);
   try {
     do_check_false(engine.enabled);
     Svc.Prefs.set("engine.steam", true);
     do_check_true(engine.enabled);
 
     engine.enabled = false;
     do_check_false(Svc.Prefs.get("engine.steam"));
+    run_next_test();
   } finally {
     Svc.Prefs.resetBranch("");
   }
-}
+});
 
-function test_sync() {
+add_test(function test_sync() {
   let engine = new SteamEngine(Service);
   try {
     _("Engine.sync doesn't call _sync if it's not enabled");
     do_check_false(engine.enabled);
     do_check_false(engine.wasSynced);
     engine.sync();
     do_check_false(engine.wasSynced);
 
     _("Engine.sync calls _sync if it's enabled");
     engine.enabled = true;
     engine.sync();
     do_check_true(engine.wasSynced);
     do_check_eq(engineObserver.topics[0], "weave:engine:sync:start");
     do_check_eq(engineObserver.topics[1], "weave:engine:sync:finish");
+    run_next_test();
   } finally {
     Svc.Prefs.resetBranch("");
     engine.wasSynced = false;
     engineObserver.reset();
   }
-}
-
-function run_test() {
-  test_members();
-  test_score();
-  test_resetClient();
-  test_wipeClient();
-  test_enabled();
-  test_sync();
-}
+});