Bug 1426742 - Fix TPS test_history and cleanup failures. r?kitcambridge draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Thu, 21 Dec 2017 15:15:24 -0500
changeset 714151 de3bfddbb82bb9af0eab517af5c60a138b44f192
parent 713998 5b1fdaa14d35ddf1a638c9422786ede707cacf1f
child 744543 dcba067a42238133bbf094ba321e3168201459ab
push id93871
push userbmo:tchiovoloni@mozilla.com
push dateThu, 21 Dec 2017 21:05:48 +0000
reviewerskitcambridge
bugs1426742
milestone59.0a1
Bug 1426742 - Fix TPS test_history and cleanup failures. r?kitcambridge MozReview-Commit-ID: 8jR4vtwUpOm
browser/extensions/formautofill/FormAutofillSync.jsm
services/sync/tps/extensions/tps/resource/modules/history.jsm
services/sync/tps/extensions/tps/resource/tps.jsm
--- a/browser/extensions/formautofill/FormAutofillSync.jsm
+++ b/browser/extensions/formautofill/FormAutofillSync.jsm
@@ -327,16 +327,17 @@ FormAutofillEngine.prototype = {
     this._store.storage.pushSyncChanges(this._modified.changes);
   },
 
   _deleteId(id) {
     this._noteDeletedId(id);
   },
 
   async _resetClient() {
+    await profileStorage.initialize();
     this._store.storage.resetSync();
   },
 };
 
 // The concrete engines
 
 function AddressesRecord(collection, id) {
   AutofillRecord.call(this, collection, id);
--- a/services/sync/tps/extensions/tps/resource/modules/history.jsm
+++ b/services/sync/tps/extensions/tps/resource/modules/history.jsm
@@ -46,17 +46,17 @@ var HistoryEntry = {
    *
    * Adds visits for a uri to the history database.  Throws on error.
    *
    * @param item An object representing one or more visits to a specific uri
    * @param usSinceEpoch The number of microseconds from Epoch to
    *        the time the current Crossweave run was started
    * @return nothing
    */
-  Add(item, usSinceEpoch) {
+  async Add(item, usSinceEpoch) {
     Logger.AssertTrue("visits" in item && "uri" in item,
       "History entry in test file must have both 'visits' " +
       "and 'uri' properties");
     let uri = Services.io.newURI(item.uri);
     let place = {
       uri,
       visits: []
     };
@@ -64,30 +64,27 @@ var HistoryEntry = {
       place.visits.push({
         visitDate: usSinceEpoch + (visit.date * 60 * 60 * 1000 * 1000),
         transitionType: visit.type
       });
     }
     if ("title" in item) {
       place.title = item.title;
     }
-    let cb = Async.makeSpinningCallback();
-    PlacesUtils.asyncHistory.updatePlaces(place, {
-        handleError: function Add_handleError() {
-          cb(new Error("Error adding history entry"));
-        },
-        handleResult: function Add_handleResult() {
-          cb();
-        },
-        handleCompletion: function Add_handleCompletion() {
-          // Nothing to do
-        }
+    return new Promise((resolve, reject) => {
+      PlacesUtils.asyncHistory.updatePlaces(place, {
+          handleError() {
+            reject(new Error("Error adding history entry"));
+          },
+          handleResult() {},
+          handleCompletion() {
+            resolve();
+          }
+      });
     });
-    // Spin the event loop to embed this async call in a sync API
-    cb.wait();
   },
 
   /**
    * Find
    *
    * Finds visits for a uri to the history database.  Throws on error.
    *
    * @param item An object representing one or more visits to a specific uri
@@ -108,46 +105,49 @@ var HistoryEntry = {
           itemvisit.found = true;
         }
       }
     }
 
     let all_items_found = true;
     for (let itemvisit of item.visits) {
       all_items_found = all_items_found && "found" in itemvisit;
-      Logger.logInfo("History entry for " + item.uri + ", type:" +
-              itemvisit.type + ", date:" + itemvisit.date +
-              ("found" in itemvisit ? " is present" : " is not present"));
+      Logger.logInfo(
+        `History entry for ${item.uri}, type: ${itemvisit.type}, date: ${itemvisit.date}` +
+        `(${itemvisit.date * 60 * 60 * 1000 * 1000}), found = ${!!itemvisit.found}`
+      );
     }
     return all_items_found;
   },
 
   /**
    * Delete
    *
    * Removes visits from the history database. Throws on error.
    *
    * @param item An object representing items to delete
    * @param usSinceEpoch The number of microseconds from Epoch to
    *        the time the current Crossweave run was started
    * @return nothing
    */
-  Delete(item, usSinceEpoch) {
+  async Delete(item, usSinceEpoch) {
     if ("uri" in item) {
-      Async.promiseSpinningly(PlacesUtils.history.remove(item.uri));
+      let removedAny = await PlacesUtils.history.remove(item.uri);
+      if (!removedAny) {
+        Logger.log("Warning: Removed 0 history visits for uri " + item.uri);
+      }
     } else if ("host" in item) {
-      PlacesUtils.history.removePagesFromHost(item.host, false);
+      await PlacesUtils.history.removePagesFromHost(item.host, false);
     } else if ("begin" in item && "end" in item) {
-      let cb = Async.makeSpinningCallback();
       let msSinceEpoch = parseInt(usSinceEpoch / 1000);
       let filter = {
         beginDate: new Date(msSinceEpoch + (item.begin * 60 * 60 * 1000)),
         endDate: new Date(msSinceEpoch + (item.end * 60 * 60 * 1000))
       };
-      PlacesUtils.history.removeVisitsByFilter(filter)
-      .catch(ex => Logger.AssertTrue(false, "An error occurred while deleting history: " + ex.message))
-      .then(result => { cb(null, result); }, err => { cb(err); });
-      Async.waitForSyncCallback(cb);
+      let removedAny = await PlacesUtils.history.removeVisitsByFilter(filter);
+      if (!removedAny) {
+        Logger.log("Warning: Removed 0 history visits with " + JSON.stringify({ item, filter }));
+      }
     } else {
-      Logger.AssertTrue(false, "invalid entry in delete history");
+      Logger.AssertTrue(false, "invalid entry in delete history " + JSON.stringify(item));
     }
   },
 };
--- a/services/sync/tps/extensions/tps/resource/tps.jsm
+++ b/services/sync/tps/extensions/tps/resource/tps.jsm
@@ -401,32 +401,33 @@ var TPS = {
     }
     Logger.logPass("executing action " + action.toUpperCase() +
                    " on formdata");
   },
 
   async HandleHistory(entries, action) {
     try {
       for (let entry of entries) {
+        const entryString = JSON.stringify(entry);
         Logger.logInfo("executing action " + action.toUpperCase() +
-                       " on history entry " + JSON.stringify(entry));
+                       " on history entry " + entryString);
         switch (action) {
           case ACTION_ADD:
-            HistoryEntry.Add(entry, this._usSinceEpoch);
+            await HistoryEntry.Add(entry, this._usSinceEpoch);
             break;
           case ACTION_DELETE:
-            HistoryEntry.Delete(entry, this._usSinceEpoch);
+            await HistoryEntry.Delete(entry, this._usSinceEpoch);
             break;
           case ACTION_VERIFY:
             Logger.AssertTrue((await HistoryEntry.Find(entry, this._usSinceEpoch)),
-              "Uri visits not found in history database");
+              "Uri visits not found in history database: " + entryString);
             break;
           case ACTION_VERIFY_NOT:
             Logger.AssertTrue(!(await HistoryEntry.Find(entry, this._usSinceEpoch)),
-              "Uri visits found in history database, but they shouldn't be");
+              "Uri visits found in history database, but they shouldn't be: " + entryString);
             break;
           default:
             Logger.AssertTrue(false, "invalid action: " + action);
         }
       }
       Logger.logPass("executing action " + action.toUpperCase() +
                      " on history");
     } catch (e) {
@@ -738,19 +739,22 @@ var TPS = {
         // we're all done
         Logger.logInfo("test phase " + this._currentPhase + ": " +
                        (this._errors ? "FAIL" : "PASS"));
         this._phaseFinished = true;
         this.quit();
         return;
       }
       this.seconds_since_epoch = Services.prefs.getIntPref("tps.seconds_since_epoch");
-      if (this.seconds_since_epoch)
-        this._usSinceEpoch = this.seconds_since_epoch * 1000 * 1000;
-      else {
+      if (this.seconds_since_epoch) {
+        // Places dislikes it if we add visits in the future. We pretend the
+        // real time is 1 minute ago to avoid issues caused by places using a
+        // different clock than the one that set the seconds_since_epoch pref.
+        this._usSinceEpoch = (this.seconds_since_epoch - 60) * 1000 * 1000;
+      } else {
         this.DumpError("seconds-since-epoch not set");
         return;
       }
 
       let phase = this._phaselist[this._currentPhase];
       let action = phase[this._currentAction];
       Logger.logInfo("starting action: " + action[0].name);
       await action[0].apply(this, action.slice(1));