Bug 679279 - Part 3: Object.keys and test improvements. r=philikon
authorRichard Newman <rnewman@mozilla.com>
Fri, 19 Aug 2011 17:17:58 -0700
changeset 77039 becba4169e3738740e5130f848b3a6be0dc83843
parent 77038 7e4d90c683519e6877acc7c482baf00f4a06e50f
child 77040 dfb016c497aeefbd7d810eb648586c67ce6eb0b7
push id78
push userclegnitto@mozilla.com
push dateFri, 16 Dec 2011 17:32:24 +0000
treeherdermozilla-release@79d24e644fdd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersphilikon
bugs679279
milestone9.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 679279 - Part 3: Object.keys and test improvements. r=philikon
services/sync/modules/engines.js
services/sync/tests/unit/head_helpers.js
services/sync/tests/unit/test_bookmark_tracker.js
services/sync/tests/unit/test_forms_tracker.js
services/sync/tests/unit/test_history_store.js
services/sync/tests/unit/test_history_tracker.js
services/sync/tests/unit/test_password_tracker.js
services/sync/tests/unit/test_prefs_store.js
services/sync/tests/unit/test_prefs_tracker.js
services/sync/tests/unit/test_syncengine_sync.js
services/sync/tests/unit/test_tab_tracker.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -627,17 +627,17 @@ SyncEngine.prototype = {
       }
     }
     // Clear the tracker now. If the sync fails we'll add the ones we failed
     // to upload back.
     this._tracker.clearChangedIDs();
  
     // Array of just the IDs from this._modified. This is what we iterate over
     // so we can modify this._modified during the iteration.
-    this._modifiedIDs = [id for (id in this._modified)];
+    this._modifiedIDs = Object.keys(this._modified);
     this._log.info(this._modifiedIDs.length +
                    " outgoing items pre-reconciliation");
 
     // Keep track of what to delete at the end of sync
     this._delete = {};
   },
 
   // Process incoming records
@@ -998,17 +998,17 @@ SyncEngine.prototype = {
           throw resp;
         }
 
         // Update server timestamp from the upload.
         let modified = resp.headers["x-weave-timestamp"];
         if (modified > this.lastSync)
           this.lastSync = modified;
 
-        let failed_ids = [id for (id in resp.obj.failed)];
+        let failed_ids = Object.keys(resp.obj.failed);
         if (failed_ids.length)
           this._log.debug("Records that will be uploaded again because "
                           + "the server couldn't store them: "
                           + failed_ids.join(", "));
 
         // Clear successfully uploaded objects.
         for each (let id in resp.obj.success) {
           delete this._modified[id];
--- a/services/sync/tests/unit/head_helpers.js
+++ b/services/sync/tests/unit/head_helpers.js
@@ -199,16 +199,24 @@ function encryptPayload(cleartext) {
 }
 
 function generateNewKeys(collections) {
   let wbo = CollectionKeys.generateNewKeysWBO(collections);
   let modified = new_timestamp();
   CollectionKeys.setContents(wbo.cleartext, modified);
 }
 
+function do_check_empty(obj) {
+  do_check_attribute_count(obj, 0);
+}
+
+function do_check_attribute_count(obj, c) {
+  do_check_eq(c, Object.keys(obj).length);
+}
+
 function do_check_throws(aFunc, aResult, aStack)
 {
   if (!aStack) {
     try {
       // We might not have a 'Components' object.
       aStack = Components.stack.caller;
     } catch (e) {}
   }
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -7,59 +7,59 @@ Cu.import("resource://gre/modules/Places
 Engines.register(BookmarksEngine);
 let engine = Engines.get("bookmarks");
 let store  = engine._store;
 store.wipe();
 
 function test_tracking() {
   _("Verify we've got an empty tracker to work with.");
   let tracker = engine._tracker;
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_empty(tracker.changedIDs);
 
   let folder = PlacesUtils.bookmarks.createFolder(
     PlacesUtils.bookmarks.bookmarksMenuFolder,
     "Test Folder", PlacesUtils.bookmarks.DEFAULT_INDEX);
   function createBmk() {
     return PlacesUtils.bookmarks.insertBookmark(
       folder, Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
   }
 
   try {
     _("Create bookmark. Won't show because we haven't started tracking yet");
     createBmk();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     do_check_eq(tracker.score, 0);
 
     _("Tell the tracker to start tracking changes.");
     Svc.Obs.notify("weave:engine:start-tracking");
     createBmk();
     // We expect two changed items because the containing folder
     // changed as well (new child).
-    do_check_eq([id for (id in tracker.changedIDs)].length, 2);
+    do_check_attribute_count(tracker.changedIDs, 2);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 2);
 
     _("Notifying twice won't do any harm.");
     Svc.Obs.notify("weave:engine:start-tracking");
     createBmk();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 3);
+    do_check_attribute_count(tracker.changedIDs, 3);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 4);
 
     _("Let's stop tracking again.");
     tracker.clearChangedIDs();
     tracker.resetScore();
     Svc.Obs.notify("weave:engine:stop-tracking");
     createBmk();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     do_check_eq(tracker.score, 0);
 
     _("Notifying twice won't do any harm.");
     Svc.Obs.notify("weave:engine:stop-tracking");
     createBmk();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     do_check_eq(tracker.score, 0);
 
   } finally {
     _("Clean up.");
     store.wipe();
     tracker.clearChangedIDs();
     tracker.resetScore();
     Svc.Obs.notify("weave:engine:stop-tracking");
@@ -67,17 +67,17 @@ function test_tracking() {
 }
 
 function test_onItemChanged() {
   // Anno that's in ANNOS_TO_TRACK.
   const DESCRIPTION_ANNO = "bookmarkProperties/description";
 
   _("Verify we've got an empty tracker to work with.");
   let tracker = engine._tracker;
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_empty(tracker.changedIDs);
   do_check_eq(tracker.score, 0);
 
   try {
     Svc.Obs.notify("weave:engine:stop-tracking");
     let folder = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
     _("Track changes to annos.");
@@ -102,17 +102,17 @@ function test_onItemChanged() {
     tracker.resetScore();
     Svc.Obs.notify("weave:engine:stop-tracking");
   }
 }
 
 function test_onItemMoved() {
   _("Verify we've got an empty tracker to work with.");
   let tracker = engine._tracker;
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_empty(tracker.changedIDs);
   do_check_eq(tracker.score, 0);
 
   try {
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
--- a/services/sync/tests/unit/test_forms_tracker.js
+++ b/services/sync/tests/unit/test_forms_tracker.js
@@ -1,44 +1,44 @@
 Cu.import("resource://services-sync/engines/forms.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-sync/log4moz.js");
 
 function run_test() {
   _("Verify we've got an empty tracker to work with.");
   let tracker = new FormEngine()._tracker;
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_empty(tracker.changedIDs);
   Log4Moz.repository.rootLogger.addAppender(new Log4Moz.DumpAppender());
 
   try {
     _("Create an entry. Won't show because we haven't started tracking yet");
     Svc.Form.addEntry("name", "John Doe");
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
 
     _("Tell the tracker to start tracking changes.");
     Svc.Obs.notify("weave:engine:start-tracking");
     Svc.Form.removeEntry("name", "John Doe");
     Svc.Form.addEntry("email", "john@doe.com");
-    do_check_eq([id for (id in tracker.changedIDs)].length, 2);
+    do_check_attribute_count(tracker.changedIDs, 2);
 
     _("Notifying twice won't do any harm.");
     Svc.Obs.notify("weave:engine:start-tracking");
     Svc.Form.addEntry("address", "Memory Lane");
-    do_check_eq([id for (id in tracker.changedIDs)].length, 3);
+    do_check_attribute_count(tracker.changedIDs, 3);
 
     _("Let's stop tracking again.");
     tracker.clearChangedIDs();
     Svc.Obs.notify("weave:engine:stop-tracking");
     Svc.Form.removeEntry("address", "Memory Lane");
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
 
     _("Notifying twice won't do any harm.");
     Svc.Obs.notify("weave:engine:stop-tracking");
     Svc.Form.removeEntry("email", "john@doe.com");
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
   
     _("Test error detection.");
     // This throws an exception without the fix for Bug 597400.
     tracker.trackEntry("foo", "bar");
     
   } finally {
     _("Clean up.");
     Svc.Form.removeAllEntries();
--- a/services/sync/tests/unit/test_history_store.js
+++ b/services/sync/tests/unit/test_history_store.js
@@ -73,24 +73,24 @@ let fxuri, fxguid, tburi, tbguid;
 
 function run_test() {
   initTestLogging("Trace");
   run_next_test();
 }
 
 add_test(function test_store() {
   _("Verify that we've got an empty store to work with.");
-  do_check_eq([id for (id in store.getAllIDs())].length, 0);
+  do_check_empty(store.getAllIDs());
 
   _("Let's create an entry in the database.");
   fxuri = Utils.makeURI("http://getfirefox.com/");
    PlacesUtils.history.addPageWithDetails(fxuri, "Get Firefox!", TIMESTAMP1);
 
   _("Verify that the entry exists.");
-  let ids = [id for (id in store.getAllIDs())];
+  let ids = Object.keys(store.getAllIDs());
   do_check_eq(ids.length, 1);
   fxguid = ids[0];
   do_check_true(store.itemExists(fxguid));
 
   _("If we query a non-existent record, it's marked as deleted.");
   let record = store.createRecord("non-existent");
   do_check_true(record.deleted);
 
@@ -122,17 +122,17 @@ add_test(function test_store() {
   ]);
 });
 
 add_test(function test_store_create() {
   _("Create a brand new record through the store.");
   tbguid = Utils.makeGUID();
   tburi = Utils.makeURI("http://getthunderbird.com");
   onNextTitleChanged(ensureThrows(function() {
-    do_check_eq([id for (id in store.getAllIDs())].length, 2);
+    do_check_attribute_count(store.getAllIDs(), 2);
     let queryres = queryHistoryVisits(tburi);
     do_check_eq(queryres.length, 1);
     do_check_eq(queryres[0].time, TIMESTAMP3);
     do_check_eq(queryres[0].title, "The bird is the word!");
     run_next_test();
   }));
   applyEnsureNoFailures([
     {id: tbguid,
@@ -149,31 +149,31 @@ add_test(function test_null_title() {
   let resuri = Utils.makeURI("unknown://title");
   applyEnsureNoFailures([
     {id: resguid,
      histUri: resuri.spec,
      title: null,
      visits: [{date: TIMESTAMP3,
                type: Ci.nsINavHistoryService.TRANSITION_TYPED}]}
   ]);
-  do_check_eq([id for (id in store.getAllIDs())].length, 3);
+  do_check_attribute_count(store.getAllIDs(), 3);
   let queryres = queryHistoryVisits(resuri);
   do_check_eq(queryres.length, 1);
   do_check_eq(queryres[0].time, TIMESTAMP3);
   run_next_test();
 });
 
 add_test(function test_invalid_records() {
   _("Make sure we handle invalid URLs in places databases gracefully.");
   let query = "INSERT INTO moz_places "
     + "(url, title, rev_host, visit_count, last_visit_date) "
     + "VALUES ('invalid-uri', 'Invalid URI', '.', 1, " + TIMESTAMP3 + ")";
   let stmt = PlacesUtils.history.DBConnection.createAsyncStatement(query);
   let result = Async.querySpinningly(stmt);
-  do_check_eq([id for (id in store.getAllIDs())].length, 4);
+  do_check_attribute_count(store.getAllIDs(), 4);
 
   _("Make sure we report records with invalid URIs.");
   let invalid_uri_guid = Utils.makeGUID();
   let failed = store.applyIncomingBatch([{
     id: invalid_uri_guid,
     histUri: ":::::::::::::::",
     title: "Doesn't have a valid URI",
     visits: [{date: TIMESTAMP3,
@@ -250,17 +250,17 @@ add_test(function test_remove() {
   applyEnsureNoFailures([{id: fxguid, deleted: true},
                          {id: Utils.makeGUID(), deleted: true}]);
   do_check_false(store.itemExists(fxguid));
   let queryres = queryHistoryVisits(fxuri);
   do_check_eq(queryres.length, 0);
 
   _("Make sure wipe works.");
   store.wipe();
-  do_check_eq([id for (id in store.getAllIDs())].length, 0);
+  do_check_empty(store.getAllIDs());
   queryres = queryHistoryVisits(fxuri);
   do_check_eq(queryres.length, 0);
   queryres = queryHistoryVisits(tburi);
   do_check_eq(queryres.length, 0);
   run_next_test();
 });
 
 add_test(function cleanup() {
--- a/services/sync/tests/unit/test_history_tracker.js
+++ b/services/sync/tests/unit/test_history_tracker.js
@@ -29,62 +29,62 @@ function addVisit() {
 
 
 function run_test() {
   run_next_test();
 }
 
 add_test(function test_empty() {
   _("Verify we've got an empty tracker to work with.");
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_empty(tracker.changedIDs);
   do_check_eq(tracker.score, 0);
   run_next_test();
 });
 
 add_test(function test_not_tracking(next) {
   _("Create history item. Won't show because we haven't started tracking yet");
   addVisit();
   Utils.nextTick(function() {
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     do_check_eq(tracker.score, 0);
     run_next_test();
   });
 });
 
 add_test(function test_start_tracking() {
   _("Tell the tracker to start tracking changes.");
   onScoreUpdated(function() {
-    do_check_eq([id for (id in tracker.changedIDs)].length, 1);
+    do_check_attribute_count(tracker.changedIDs, 1);
     do_check_eq(tracker.score, SCORE_INCREMENT_SMALL);
     run_next_test();
   });
   Svc.Obs.notify("weave:engine:start-tracking");
   addVisit();
 });
 
 add_test(function test_start_tracking_twice() {
   _("Notifying twice won't do any harm.");
   onScoreUpdated(function() {
-    do_check_eq([id for (id in tracker.changedIDs)].length, 2);
+    do_check_attribute_count(tracker.changedIDs, 2);
     do_check_eq(tracker.score, 2 * SCORE_INCREMENT_SMALL);
     run_next_test();
   });
   Svc.Obs.notify("weave:engine:start-tracking");
   addVisit();
 });
 
 add_test(function test_track_delete() {
   _("Deletions are tracked.");
   let uri = Utils.makeURI("http://getfirefox.com/0");
   let guid = engine._store.GUIDForUri(uri);
   do_check_false(guid in tracker.changedIDs);
 
   onScoreUpdated(function() {
     do_check_true(guid in tracker.changedIDs);
-    do_check_eq([id for (id in tracker.changedIDs)].length, 3);
+    do_check_attribute_count(tracker.changedIDs, 3);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE + 2 * SCORE_INCREMENT_SMALL);
     run_next_test();
   });
   do_check_eq(tracker.score, 2 * SCORE_INCREMENT_SMALL);
   PlacesUtils.history.removePage(uri);
 });
 
 add_test(function test_dont_track_expiration() {
@@ -96,17 +96,17 @@ add_test(function test_dont_track_expira
 
   tracker.clearChangedIDs();
   do_check_false(guidToExpire in tracker.changedIDs);
   do_check_false(guidToRemove in tracker.changedIDs);
 
   onScoreUpdated(function() {
     do_check_false(guidToExpire in tracker.changedIDs);
     do_check_true(guidToRemove in tracker.changedIDs);
-    do_check_eq([id for (id in tracker.changedIDs)].length, 1);
+    do_check_attribute_count(tracker.changedIDs, 1);
     run_next_test();
   });
 
   // Observe expiration.
   Services.obs.addObserver(function onExpiration(aSubject, aTopic, aData) {
     Services.obs.removeObserver(onExpiration, aTopic);
     // Remove the remaining page to update its score.
     PlacesUtils.history.removePage(uriToRemove);
@@ -120,27 +120,27 @@ add_test(function test_dont_track_expira
 });
 
 add_test(function test_stop_tracking() {
   _("Let's stop tracking again.");
   tracker.clearChangedIDs();
   Svc.Obs.notify("weave:engine:stop-tracking");
   addVisit();
   Utils.nextTick(function() {
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     run_next_test();
   });
 });
 
 add_test(function test_stop_tracking_twice() {
   _("Notifying twice won't do any harm.");
   Svc.Obs.notify("weave:engine:stop-tracking");
   addVisit();
   Utils.nextTick(function() {
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     run_next_test();
   });
 });
 
 add_test(function cleanup() {
    _("Clean up.");
   PlacesUtils.history.removeAllPages();
   run_next_test();
--- a/services/sync/tests/unit/test_password_tracker.js
+++ b/services/sync/tests/unit/test_password_tracker.js
@@ -6,17 +6,17 @@ Engines.register(PasswordEngine);
 let engine = Engines.get("passwords");
 let store  = engine._store;
 
 function test_tracking() {
   let recordNum = 0;
 
   _("Verify we've got an empty tracker to work with.");
   let tracker = engine._tracker;
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_empty(tracker.changedIDs);
 
   function createPassword() {
     _("RECORD NUM: " + recordNum);
     let record = {id: "GUID" + recordNum,
                   hostname: "http://foo.bar.com",
                   formSubmitURL: "http://foo.bar.com/baz",
                   username: "john" + recordNum,
                   password: "smith",
@@ -25,58 +25,58 @@ function test_tracking() {
     recordNum++;
     let login = store._nsLoginInfoFromRecord(record);
     Services.logins.addLogin(login);
   }
 
   try {
     _("Create a password record. Won't show because we haven't started tracking yet");
     createPassword();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     do_check_eq(tracker.score, 0);
 
     _("Tell the tracker to start tracking changes.");
     Svc.Obs.notify("weave:engine:start-tracking");
     createPassword();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 1);
+    do_check_attribute_count(tracker.changedIDs, 1);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
 
     _("Notifying twice won't do any harm.");
     Svc.Obs.notify("weave:engine:start-tracking");
     createPassword();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 2);
+    do_check_attribute_count(tracker.changedIDs, 2);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 2);
 
     _("Let's stop tracking again.");
     tracker.clearChangedIDs();
     tracker.resetScore();
     Svc.Obs.notify("weave:engine:stop-tracking");
     createPassword();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     do_check_eq(tracker.score, 0);
 
     _("Notifying twice won't do any harm.");
     Svc.Obs.notify("weave:engine:stop-tracking");
     createPassword();
-    do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+    do_check_empty(tracker.changedIDs);
     do_check_eq(tracker.score, 0);
 
   } finally {
     _("Clean up.");
     store.wipe();
     tracker.clearChangedIDs();
     tracker.resetScore();
     Svc.Obs.notify("weave:engine:stop-tracking");
   }
 }
 
 function test_onWipe() {
   _("Verify we've got an empty tracker to work with.");
   let tracker = engine._tracker;
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_empty(tracker.changedIDs);
   do_check_eq(tracker.score, 0);
 
   try {
     _("A store wipe should increment the score");
     Svc.Obs.notify("weave:engine:start-tracking");
     store.wipe();
     
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
--- a/services/sync/tests/unit/test_prefs_store.js
+++ b/services/sync/tests/unit/test_prefs_store.js
@@ -31,17 +31,17 @@ function run_test() {
     prefs.set("testing.string", "ohai");
     prefs.set("testing.bool", true);
     prefs.set("testing.dont.change", "Please don't change me.");
     prefs.set("testing.turned.off", "I won't get synced.");
     prefs.set("testing.not.turned.on", "I won't get synced either!");
 
     _("The GUID corresponds to XUL App ID.");
     let allIDs = store.getAllIDs();
-    let ids = [id for (id in allIDs)];
+    let ids = Object.keys(allIDs);
     do_check_eq(ids.length, 1);
     do_check_eq(ids[0], PREFS_GUID);
     do_check_true(allIDs[PREFS_GUID], true);
 
     do_check_true(store.itemExists(PREFS_GUID));
     do_check_false(store.itemExists("random-gibberish"));
 
     _("Unknown prefs record is created as deleted.");
--- a/services/sync/tests/unit/test_prefs_tracker.js
+++ b/services/sync/tests/unit/test_prefs_tracker.js
@@ -15,25 +15,25 @@ function run_test() {
     do_check_false(tracker.modified);
 
     tracker.modified = true;
     do_check_eq(Svc.Prefs.get("engine.prefs.modified"), true);
     do_check_true(tracker.modified);
 
     _("Engine's getChangedID() just returns the one GUID we have.");
     let changedIDs = engine.getChangedIDs();
-    let ids = [id for (id in changedIDs)];
+    let ids = Object.keys(changedIDs);
     do_check_eq(ids.length, 1);
     do_check_eq(ids[0], Utils.encodeBase64url(Services.appinfo.ID));
 
     Svc.Prefs.set("engine.prefs.modified", false);
     do_check_false(tracker.modified);
 
     _("No modified state, so no changed IDs.");
-    do_check_eq([id for (id in engine.getChangedIDs())].length, 0);
+    do_check_empty(engine.getChangedIDs());
 
     _("Initial score is 0");
     do_check_eq(tracker.score, 0);
 
     _("Test fixtures.");
     Svc.Prefs.set("prefs.sync.testing.int", true);
 
     _("Test fixtures haven't upped the tracker score yet because it hasn't started tracking yet.");
--- a/services/sync/tests/unit/test_syncengine_sync.js
+++ b/services/sync/tests/unit/test_syncengine_sync.js
@@ -397,17 +397,17 @@ add_test(function test_processIncoming_m
   meta_global.payload.engines = {rotary: {version: engine.version,
                                          syncID: engine.syncID}};
 
   try {
 
     _("On a mobile client, we get new records from the server in batches of 50.");
     engine._syncStartup();
     engine._processIncoming();
-    do_check_eq([id for (id in engine._store.items)].length, 234);
+    do_check_attribute_count(engine._store.items, 234);
     do_check_true('record-no-0' in engine._store.items);
     do_check_true('record-no-49' in engine._store.items);
     do_check_true('record-no-50' in engine._store.items);
     do_check_true('record-no-233' in engine._store.items);
 
     // Verify that the right number of GET requests with the right
     // kind of parameters were made.
     do_check_eq(collection.get_log.length,
@@ -468,28 +468,28 @@ add_test(function test_processIncoming_s
   let server = sync_httpd_setup({
       "/1.1/foo/storage/rotary": collection.handler()
   });
 
   try {
 
     // Confirm initial environment
     do_check_eq(engine.lastSync, 0);
-    do_check_eq([id for (id in engine._store.items)].length, 0);
+    do_check_empty(engine._store.items);
 
     let error;
     try {
       engine.sync();
     } catch (ex) {
       error = ex;
     }
     do_check_true(!!error);
 
     // Only the first two batches have been applied.
-    do_check_eq([id for (id in engine._store.items)].length,
+    do_check_eq(Object.keys(engine._store.items).length,
                 MOBILE_BATCH_SIZE * 2);
 
     // The third batch is stuck in toFetch. lastSync has been moved forward to
     // the last successful item's timestamp.
     do_check_eq(engine.toFetch.length, MOBILE_BATCH_SIZE);
     do_check_eq(engine.lastSync, collection.wbos["record-no-99"].modified);
 
   } finally {
@@ -597,24 +597,23 @@ add_test(function test_processIncoming_a
                                          syncID: engine.syncID}};
   let server = sync_httpd_setup({
       "/1.1/foo/storage/rotary": collection.handler()
   });
 
   try {
 
     // Confirm initial environment
-    do_check_eq([id for (id in engine._store.items)].length, 0);
+    do_check_empty(engine._store.items);
 
     engine._syncStartup();
     engine._processIncoming();
 
     // Records have been applied and the expected failures have failed.
-    do_check_eq([id for (id in engine._store.items)].length,
-                APPLY_BATCH_SIZE - 1 - 2);
+    do_check_attribute_count(engine._store.items, APPLY_BATCH_SIZE - 1 - 2);
     do_check_eq(engine.toFetch.length, 0);
     do_check_eq(engine.previousFailed.length, 2);
     do_check_eq(engine.previousFailed[0], "record-no-0");
     do_check_eq(engine.previousFailed[1], "record-no-8");
 
   } finally {
     cleanAndGo(server);
   }
@@ -653,25 +652,24 @@ add_test(function test_processIncoming_a
                                          syncID: engine.syncID}};
   let server = sync_httpd_setup({
       "/1.1/foo/storage/rotary": collection.handler()
   });
 
   try {
 
     // Confirm initial environment
-    do_check_eq([id for (id in engine._store.items)].length, 0);
+    do_check_empty(engine._store.items);
 
     engine._syncStartup();
     engine._processIncoming();
 
     // Records have been applied in 3 batches.
     do_check_eq(batchCalls, 3);
-    do_check_eq([id for (id in engine._store.items)].length,
-                APPLY_BATCH_SIZE * 3);
+    do_check_attribute_count(engine._store.items, APPLY_BATCH_SIZE * 3);
 
   } finally {
     cleanAndGo(server);
   }
 });
 
 
 add_test(function test_processIncoming_notify_count() {
@@ -707,49 +705,49 @@ add_test(function test_processIncoming_n
       "/1.1/foo/storage/rotary": collection.handler()
   });
 
   try {
     // Confirm initial environment.
     do_check_eq(engine.lastSync, 0);
     do_check_eq(engine.toFetch.length, 0);
     do_check_eq(engine.previousFailed.length, 0);
-    do_check_eq([id for (id in engine._store.items)].length, 0);
+    do_check_empty(engine._store.items);
 
     let called = 0;
     let counts;
     function onApplied(count) {
       _("Called with " + JSON.stringify(counts));
       counts = count;
       called++;
     }
     Svc.Obs.add("weave:engine:sync:applied", onApplied);
 
     // Do sync.
     engine._syncStartup();
     engine._processIncoming();
     
     // Confirm failures.
-    do_check_eq([id for (id in engine._store.items)].length, 12);
+    do_check_attribute_count(engine._store.items, 12);
     do_check_eq(engine.previousFailed.length, 3);
     do_check_eq(engine.previousFailed[0], "record-no-0");
     do_check_eq(engine.previousFailed[1], "record-no-5");
     do_check_eq(engine.previousFailed[2], "record-no-10");
 
     // There are newly failed records and they are reported.
     do_check_eq(called, 1);
     do_check_eq(counts.failed, 3);
     do_check_eq(counts.applied, 15);
     do_check_eq(counts.newFailed, 3);
 
     // Sync again, 1 of the failed items are the same, the rest didn't fail.
     engine._processIncoming();
     
     // Confirming removed failures.
-    do_check_eq([id for (id in engine._store.items)].length, 14);
+    do_check_attribute_count(engine._store.items, 14);
     do_check_eq(engine.previousFailed.length, 1);
     do_check_eq(engine.previousFailed[0], "record-no-0");
 
     do_check_eq(called, 2);
     do_check_eq(counts.failed, 1);
     do_check_eq(counts.applied, 3);
     do_check_eq(counts.newFailed, 0);
 
@@ -794,45 +792,45 @@ add_test(function test_processIncoming_p
       "/1.1/foo/storage/rotary": collection.handler()
   });
 
   try {
     // Confirm initial environment.
     do_check_eq(engine.lastSync, 0);
     do_check_eq(engine.toFetch.length, 0);
     do_check_eq(engine.previousFailed.length, 0);
-    do_check_eq([id for (id in engine._store.items)].length, 0);
+    do_check_empty(engine._store.items);
 
     // Initial failed items in previousFailed to be reset.
     let previousFailed = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
     engine.previousFailed = previousFailed;
     do_check_eq(engine.previousFailed, previousFailed);
 
     // Do sync.
     engine._syncStartup();
     engine._processIncoming();
 
     // Expected result: 4 sync batches with 2 failures each => 8 failures
-    do_check_eq([id for (id in engine._store.items)].length, 6);
+    do_check_attribute_count(engine._store.items, 6);
     do_check_eq(engine.previousFailed.length, 8);
     do_check_eq(engine.previousFailed[0], "record-no-0");
     do_check_eq(engine.previousFailed[1], "record-no-1");
     do_check_eq(engine.previousFailed[2], "record-no-4");
     do_check_eq(engine.previousFailed[3], "record-no-5");
     do_check_eq(engine.previousFailed[4], "record-no-8");
     do_check_eq(engine.previousFailed[5], "record-no-9");
     do_check_eq(engine.previousFailed[6], "record-no-12");
     do_check_eq(engine.previousFailed[7], "record-no-13");
 
     // Sync again with the same failed items (records 0, 1, 8, 9).
     engine._processIncoming();
 
     // A second sync with the same failed items should not add the same items again.
     // Items that did not fail a second time should no longer be in previousFailed.
-    do_check_eq([id for (id in engine._store.items)].length, 10);
+    do_check_attribute_count(engine._store.items, 10);
     do_check_eq(engine.previousFailed.length, 4);
     do_check_eq(engine.previousFailed[0], "record-no-0");
     do_check_eq(engine.previousFailed[1], "record-no-1");
     do_check_eq(engine.previousFailed[2], "record-no-8");
     do_check_eq(engine.previousFailed[3], "record-no-9");
 
     // Refetched items that didn't fail the second time are in engine._store.items.
     do_check_eq(engine._store.items['record-no-4'], "Record No. 4");
@@ -849,17 +847,17 @@ add_test(function test_processIncoming_f
   _("Ensure that failed records from _reconcile and applyIncomingBatch are refetched.");
   let syncTesting = new SyncTestingInfrastructure();
   Svc.Prefs.set("clusterURL", "http://localhost:8080/");
   Svc.Prefs.set("username", "foo");
 
   // Let's create three and a bit batches worth of server side records.
   let collection = new ServerCollection();
   const NUMBER_OF_RECORDS = MOBILE_BATCH_SIZE * 3 + 5;
-  for (var i = 0; i < NUMBER_OF_RECORDS; i++) {
+  for (let i = 0; i < NUMBER_OF_RECORDS; i++) {
     let id = 'record-no-' + i;
     let payload = encryptPayload({id: id, denomination: "Record No. " + id});
     let wbo = new ServerWBO(id, payload);
     wbo.modified = Date.now()/1000 + 60 * (i - MOBILE_BATCH_SIZE * 3);
     collection.wbos[id] = wbo;
   }
 
   // Engine that batches but likes to throw on a couple of records,
@@ -911,32 +909,32 @@ add_test(function test_processIncoming_f
   });
 
   try {
 
     // Confirm initial environment
     do_check_eq(engine.lastSync, 0);
     do_check_eq(engine.toFetch.length, 0);
     do_check_eq(engine.previousFailed.length, 0);
-    do_check_eq([id for (id in engine._store.items)].length, 0);
+    do_check_empty(engine._store.items);
 
     let observerSubject;
     let observerData;
     Svc.Obs.add("weave:engine:sync:applied", function onApplied(subject, data) {
       Svc.Obs.remove("weave:engine:sync:applied", onApplied);
       observerSubject = subject;
       observerData = data;
     });
 
     engine._syncStartup();
     engine._processIncoming();
 
     // Ensure that all records but the bogus 4 have been applied.
-    do_check_eq([id for (id in engine._store.items)].length,
-                NUMBER_OF_RECORDS - BOGUS_RECORDS.length);
+    do_check_attribute_count(engine._store.items,
+                             NUMBER_OF_RECORDS - BOGUS_RECORDS.length);
 
     // Ensure that the bogus records will be fetched again on the next sync.
     do_check_eq(engine.previousFailed.length, BOGUS_RECORDS.length);
     engine.previousFailed.sort();
     BOGUS_RECORDS.sort();
     for (let i = 0; i < engine.previousFailed.length; i++) {
       do_check_eq(engine.previousFailed[i], BOGUS_RECORDS[i]);
     }
@@ -1525,17 +1523,17 @@ add_test(function test_syncapplied_obser
 
   try {
     SyncScheduler.hasIncomingItems = false;
 
     // Do sync.
     engine._syncStartup();
     engine._processIncoming();
 
-    do_check_eq([id for (id in engine._store.items)].length, 10);
+    do_check_attribute_count(engine._store.items, 10);
 
     do_check_eq(numApplyCalls, 1);
     do_check_eq(engine_name, "rotary");
     do_check_eq(count.applied, 10);
 
     do_check_true(SyncScheduler.hasIncomingItems);
   } finally {
     cleanAndGo(server);
--- a/services/sync/tests/unit/test_tab_tracker.js
+++ b/services/sync/tests/unit/test_tab_tracker.js
@@ -42,17 +42,17 @@ function fakeSvcSession() {
 }
 
 function run_test() {
   let engine = new TabEngine();
 
   _("We assume that tabs have changed at startup.");
   let tracker = engine._tracker;
   do_check_true(tracker.modified);
-  do_check_true(Utils.deepEquals([id for (id in engine.getChangedIDs())],
+  do_check_true(Utils.deepEquals(Object.keys(engine.getChangedIDs()),
                                  [Clients.localID]));
 
   let logs;
 
   _("Test listeners are registered on windows");
   logs = fakeSvcWinMediator();
   Svc.Obs.notify("weave:engine:start-tracking");
   do_check_eq(logs.length, 2);
@@ -86,26 +86,26 @@ function run_test() {
   for each (let evttype in ["TabOpen", "TabClose", "TabSelect"]) {
     // Pretend we just synced.
     tracker.clearChangedIDs();
     do_check_false(tracker.modified);
 
     // Send a fake tab event
     tracker.onTab({type: evttype , originalTarget: evttype});
     do_check_true(tracker.modified);
-    do_check_true(Utils.deepEquals([id for (id in engine.getChangedIDs())],
+    do_check_true(Utils.deepEquals(Object.keys(engine.getChangedIDs()),
                                    [Clients.localID]));
     do_check_eq(logs.length, idx+1);
     do_check_eq(logs[idx].target, evttype);
     do_check_eq(logs[idx].prop, "weaveLastUsed");
     do_check_true(typeof logs[idx].value == "number");
     idx++;
   }
 
   // Pretend we just synced.
   tracker.clearChangedIDs();
   do_check_false(tracker.modified);
 
   tracker.onTab({type: "pageshow", originalTarget: "pageshow"});
-  do_check_true(Utils.deepEquals([id for (id in engine.getChangedIDs())],
+  do_check_true(Utils.deepEquals(Object.keys(engine.getChangedIDs()),
                                  [Clients.localID]));
   do_check_eq(logs.length, idx); // test that setTabValue isn't called
 }