Bug 1405833 - Ensure SyncEngine uses CommonUtils.namedTimer properly. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Wed, 04 Oct 2017 17:26:20 -0400
changeset 675134 b73dac439c686a0a1609a96cbc2dca952b774832
parent 675107 69ce595533cda259186ac7469c54256839ca3762
child 734524 1e0131de29a4391a72ef150d0029c89034f9986a
push id83047
push userbmo:tchiovoloni@mozilla.com
push dateWed, 04 Oct 2017 21:26:41 +0000
reviewersmarkh
bugs1405833
milestone58.0a1
Bug 1405833 - Ensure SyncEngine uses CommonUtils.namedTimer properly. r?markh MozReview-Commit-ID: 6YnhcSjKW9U
services/sync/modules/engines.js
services/sync/tests/unit/test_syncengine.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -885,20 +885,22 @@ SyncEngine.prototype = {
   set toFetch(val) {
     // Coerce the array to a string for more efficient comparison.
     if (val + "" == this._toFetch) {
       return;
     }
     this._toFetch = val;
     CommonUtils.namedTimer(function() {
       try {
-        Async.promiseSpinningly(Utils.jsonSave("toFetch/" + this.name, this, val));
+        Async.promiseSpinningly(Utils.jsonSave("toFetch/" + this.name, this, this._toFetch));
       } catch (error) {
         this._log.error("Failed to read JSON records to fetch", error);
       }
+      // Notify our tests that we finished writing the file.
+      Observers.notify("sync-testing:file-saved:toFetch", null, this.name);
     }, 0, this, "_toFetchDelay");
   },
 
   async loadToFetch() {
     // Initialize to empty if there's no file.
     this._toFetch = [];
     let toFetch = await Utils.jsonLoad("toFetch/" + this.name, this);
     if (toFetch) {
@@ -911,21 +913,25 @@ SyncEngine.prototype = {
   },
   set previousFailed(val) {
     // Coerce the array to a string for more efficient comparison.
     if (val + "" == this._previousFailed) {
       return;
     }
     this._previousFailed = val;
     CommonUtils.namedTimer(function() {
-      Utils.jsonSave("failed/" + this.name, this, val).then(() => {
+      Utils.jsonSave("failed/" + this.name, this, this._previousFailed).then(() => {
         this._log.debug("Successfully wrote previousFailed.");
       })
       .catch((error) => {
         this._log.error("Failed to set previousFailed", error);
+      })
+      .then(() => {
+        // Notify our tests that we finished writing the file.
+        Observers.notify("sync-testing:file-saved:previousFailed", null, this.name);
       });
     }, 0, this, "_previousFailedDelay");
   },
 
   async loadPreviousFailed() {
     // Initialize to empty if there's no file
     this._previousFailed = [];
     let previousFailed = await Utils.jsonLoad("failed/" + this.name, this);
--- a/services/sync/tests/unit/test_syncengine.js
+++ b/services/sync/tests/unit/test_syncengine.js
@@ -88,23 +88,39 @@ add_task(async function test_toFetch() {
   const filename = "weave/toFetch/steam.json";
   let engine = await makeSteamEngine();
   try {
     // Ensure pristine environment
     do_check_eq(engine.toFetch.length, 0);
 
     // Write file to disk
     let toFetch = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
+    let wrotePromise = promiseOneObserver("sync-testing:file-saved:toFetch");
     engine.toFetch = toFetch;
     do_check_eq(engine.toFetch, toFetch);
     // toFetch is written asynchronously
-    await Async.promiseYield();
+    await wrotePromise;
     let fakefile = syncTesting.fakeFilesystem.fakeContents[filename];
     do_check_eq(fakefile, JSON.stringify(toFetch));
 
+    // Make sure it work for consecutive writes before the callback is executed.
+    toFetch = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
+    let toFetch2 = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
+    wrotePromise = promiseOneObserver("sync-testing:file-saved:toFetch");
+
+    engine.toFetch = toFetch;
+    do_check_eq(engine.toFetch, toFetch);
+
+    engine.toFetch = toFetch2;
+    do_check_eq(engine.toFetch, toFetch2);
+    // Note that do to the way CommonUtils.namedTimer works, we won't get a 2nd callback.
+    await wrotePromise;
+    fakefile = syncTesting.fakeFilesystem.fakeContents[filename];
+    do_check_eq(fakefile, JSON.stringify(toFetch2));
+
     // Read file from disk
     toFetch = [Utils.makeGUID(), Utils.makeGUID()];
     syncTesting.fakeFilesystem.fakeContents[filename] = JSON.stringify(toFetch);
     await engine.loadToFetch();
     do_check_eq(engine.toFetch.length, 2);
     do_check_eq(engine.toFetch[0], toFetch[0]);
     do_check_eq(engine.toFetch[1], toFetch[1]);
   } finally {
@@ -118,23 +134,39 @@ add_task(async function test_previousFai
   const filename = "weave/failed/steam.json";
   let engine = await makeSteamEngine();
   try {
     // Ensure pristine environment
     do_check_eq(engine.previousFailed.length, 0);
 
     // Write file to disk
     let previousFailed = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
+    let wrotePromise = promiseOneObserver("sync-testing:file-saved:previousFailed");
     engine.previousFailed = previousFailed;
     do_check_eq(engine.previousFailed, previousFailed);
     // previousFailed is written asynchronously
-    await Async.promiseYield();
+    await wrotePromise;
     let fakefile = syncTesting.fakeFilesystem.fakeContents[filename];
     do_check_eq(fakefile, JSON.stringify(previousFailed));
 
+    // Make sure it work for consecutive writes before the callback is executed.
+    previousFailed = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
+    let previousFailed2 = [Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID(), Utils.makeGUID()];
+    wrotePromise = promiseOneObserver("sync-testing:file-saved:previousFailed");
+
+    engine.previousFailed = previousFailed;
+    do_check_eq(engine.previousFailed, previousFailed);
+
+    engine.previousFailed = previousFailed2;
+    do_check_eq(engine.previousFailed, previousFailed2);
+    // Note that do to the way CommonUtils.namedTimer works, we're only notified once.
+    await wrotePromise;
+    fakefile = syncTesting.fakeFilesystem.fakeContents[filename];
+    do_check_eq(fakefile, JSON.stringify(previousFailed2));
+
     // Read file from disk
     previousFailed = [Utils.makeGUID(), Utils.makeGUID()];
     syncTesting.fakeFilesystem.fakeContents[filename] = JSON.stringify(previousFailed);
     await engine.loadPreviousFailed();
     do_check_eq(engine.previousFailed.length, 2);
     do_check_eq(engine.previousFailed[0], previousFailed[0]);
     do_check_eq(engine.previousFailed[1], previousFailed[1]);
   } finally {