Bug 1426742 - Fix TPS test_history and cleanup failures. r=kitcambridge
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Thu, 21 Dec 2017 15:15:24 -0500
changeset 451526 a71ff50707d45d37a2f7fac51f2c07f7df14f732
parent 451525 76b305fd466731e8c22e1a5933737d0548027555
child 451527 7696a00a06cd6eee03c1f9607a4fbabb431418c6
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskitcambridge
bugs1426742
milestone59.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 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));