Bug 1404427 - Sync multiple form history deletions. r=kitcambridge

MozReview-Commit-ID: H7AmIBtFUOr

--HG--
extra : rebase_source : 596e4470b64e9406432c4d54f7ef90138a032e62

browser/base/content/test/general/browser_sanitize-timespans.js

@@ -1,7 +1,7 @@
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
-requestLongerTimeout(2);
+requestLongerTimeout(5);
 
 // Bug 453440 - Test the timespan-based logic of the sanitizer code
 var now_mSec = Date.now();

browser/components/search/test/browser_426329.js

@@ -249,7 +249,10 @@ add_task(async function testClearHistory() {
 
   let controller = searchBar.textbox.controllers.getControllerForCommand("cmd_clearhistory");
   ok(controller.isCommandEnabled("cmd_clearhistory"), "Clear history command enabled");
+
+  let historyCleared = promiseObserver("satchel-storage-changed");
   controller.doCommand("cmd_clearhistory");
+  await historyCleared;
   let count = await countEntries();
   ok(count == 0, "History cleared");
 });
@@ -262,3 +265,13 @@ add_task(async function asyncCleanup() {
   gBrowser.selectedBrowser.loadURI("about:blank");
   await promiseRemoveEngine();
 });
+
+function promiseObserver(topic) {
+  return new Promise(resolve => {
+    let obs = (aSubject, aTopic, aData) => {
+      Services.obs.removeObserver(obs, aTopic);
+      resolve(aSubject);
+    };
+    Services.obs.addObserver(obs, topic);
+  });
+}

toolkit/components/satchel/FormHistory.jsm

@@ -619,7 +619,7 @@ function dbClose(aShutdown) {
  * @param {Array.<Object>} aChanges changes to form history
  * @param {Object} aCallbacks
  */
-function updateFormHistoryWrite(aChanges, aCallbacks) {
+async function updateFormHistoryWrite(aChanges, aCallbacks) {
   log("updateFormHistoryWrite  " + aChanges.length);
 
   // pass 'now' down so that every entry in the batch has the same timestamp
@@ -648,7 +648,30 @@ function updateFormHistoryWrite(aChanges, aCallbacks) {
           delete change.timeDeleted;
         }
         stmt = makeRemoveStatement(change, bindingArrays);
-        notifications.push(["formhistory-remove", change.guid]);
+
+        // Fetch the GUIDs we are going to delete.
+        try {
+          await new Promise((res, rej) => {
+            let selectStmt = makeSearchStatement(change, ["guid"]);
+            let selectHandlers = {
+              handleCompletion() {
+                res();
+              },
+              handleError() {
+                log("remove select guids failure");
+              },
+              handleResult(aResultSet) {
+                for (let row = aResultSet.getNextRow(); row; row = aResultSet.getNextRow()) {
+                  notifications.push(["formhistory-remove", row.getResultByName("guid")]);
+                }
+              },
+            };
+            dbConnection.executeAsync([selectStmt], 1, selectHandlers);
+          });
+        } catch (e) {
+          log("Error in select statement: " + e);
+        }
+
         break;
       case "update":
         log("Update form history " + change);

toolkit/components/satchel/test/unit/head_satchel.js

@@ -102,6 +102,24 @@ function addEntry(name, value, then) {
   }, then);
 }
 
+function promiseCountEntries(name, value) {
+  return new Promise(res => {
+    countEntries(name, value, res);
+  });
+}
+
+function promiseUpdateEntry(op, name, value) {
+  return new Promise(res => {
+    updateEntry(op, name, value, res);
+  });
+}
+
+function promiseAddEntry(name, value) {
+  return new Promise(res => {
+    addEntry(name, value, res);
+  });
+}
+
 // Wrapper around FormHistory.update which handles errors. Calls then() when done.
 function updateFormHistory(changes, then) {
   FormHistory.update(changes, {

toolkit/components/satchel/test/unit/test_notify.js

@@ -5,196 +5,176 @@
  *
  */
 
-let expectedNotification;
+XPCOMUtils.defineLazyModuleGetter(this, "setTimeout", "resource://gre/modules/Timer.jsm");
-let expectedData;
-let subjectIsGuid = false;
-let lastGUID;
 
-let TestObserver = {
+const TestObserver = {
+  observed: [],
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
-
   observe(subject, topic, data) {
-    do_check_eq(topic, "satchel-storage-changed");
+    if (subject instanceof Ci.nsISupportsString) {
-    do_check_eq(data, expectedNotification);
+      subject = subject.toString();
-
-    let verifySubjectIsGuid = () => {
-      do_check_true(subject instanceof Ci.nsISupportsString);
-      do_check_true(isGUID.test(subject.toString()));
-      lastGUID = subject.toString();
-    };
-
-    switch (data) {
-      case "formhistory-add":
-      case "formhistory-update":
-        verifySubjectIsGuid();
-        break;
-      case "formhistory-remove":
-        if (subjectIsGuid) {
-          verifySubjectIsGuid();
-        } else {
-          do_check_eq(null, subject);
     }
-        break;
+    this.observed.push({subject, topic, data});
-      default:
+  },
-        do_throw("Unhandled notification: " + data + " / " + topic);
+  reset() {
-    }
+    this.observed = [];
-
-    expectedNotification = null;
-    expectedData = null;
   },
 };
 
-let testIterator = null;
+const entry1 = ["entry1", "value1"];
+const entry2 = ["entry2", "value2"];
+const entry3 = ["entry3", "value3"];
 
-function run_test() {
+add_task(async function setup() {
-  do_test_pending();
+  await promiseUpdateEntry("remove", null, null);
-  testIterator = run_test_steps();
+  const count = await promiseCountEntries(null, null);
-  testIterator.next();
+  do_check_false(count, "Checking initial DB is empty");
-}
-
-function next_test() {
-  testIterator.next();
-}
-
-function* run_test_steps() {
-  let testnum = 0;
-  let testdesc = "Setup of test form history entries";
-
-  try {
-    let entry1 = ["entry1", "value1"];
-
-    /* ========== 1 ========== */
-    testnum = 1;
-    testdesc = "Initial connection to storage module";
-
-    yield updateEntry("remove", null, null, next_test);
-    yield countEntries(null, null, function(num) {
-      do_check_false(num, "Checking initial DB is empty");
-      next_test();
-    });
 
   // Add the observer
   Services.obs.addObserver(TestObserver, "satchel-storage-changed");
+});
 
-    /* ========== 2 ========== */
+add_task(async function addAndUpdateEntry() {
-    testnum++;
+  // Add
-    testdesc = "addEntry";
+  await promiseUpdateEntry("add", entry1[0], entry1[1]);
+  do_check_eq(TestObserver.observed.length, 1);
+  let {subject, data} = TestObserver.observed[0];
+  do_check_eq(data, "formhistory-add");
+  do_check_true(isGUID.test(subject));
 
-    expectedNotification = "formhistory-add";
+  let count = await promiseCountEntries(entry1[0], entry1[1]);
-    expectedData = entry1;
+  do_check_eq(count, 1);
 
-    yield updateEntry("add", entry1[0], entry1[1], next_test);
+  // Update
-    do_check_eq(expectedNotification, null); // check that observer got a notification
+  TestObserver.reset();
 
-    yield countEntries(entry1[0], entry1[1], function(num) {
+  await promiseUpdateEntry("update", entry1[0], entry1[1]);
-      do_check_true(num > 0);
+  do_check_eq(TestObserver.observed.length, 1);
-      next_test();
+  ({subject, data} = TestObserver.observed[0]);
-    });
+  do_check_eq(data, "formhistory-update");
+  do_check_true(isGUID.test(subject));
 
-    /* ========== 3 ========== */
+  count = await promiseCountEntries(entry1[0], entry1[1]);
-    testnum++;
+  do_check_eq(count, 1);
-    testdesc = "modifyEntry";
 
-    expectedNotification = "formhistory-update";
+  // Clean-up
-    expectedData = entry1;
+  await promiseUpdateEntry("remove", null, null);
-    // will update previous entry
+});
-    yield updateEntry("update", entry1[0], entry1[1], next_test);
-    yield countEntries(entry1[0], entry1[1], function(num) {
-      do_check_true(num > 0);
-      next_test();
-    });
 
-    do_check_eq(expectedNotification, null);
+add_task(async function removeEntry() {
+  TestObserver.reset();
+  await promiseUpdateEntry("add", entry1[0], entry1[1]);
+  const guid = TestObserver.observed[0].subject;
+  TestObserver.reset();
 
-    /* ========== 4 ========== */
+  await new Promise(res => {
-    testnum++;
+    FormHistory.update({
-    testdesc = "removeEntry";
-
-    expectedNotification = "formhistory-remove";
-    expectedData = entry1;
-
-    subjectIsGuid = true;
-    yield FormHistory.update({
       op: "remove",
       fieldname: entry1[0],
       value: entry1[1],
-      guid: lastGUID,
+      guid,
     }, {
       handleError(error) {
         do_throw("Error occurred updating form history: " + error);
       },
       handleCompletion(reason) {
         if (!reason) {
-          next_test();
+          res();
         }
       },
     });
-    subjectIsGuid = false;
-
-    do_check_eq(expectedNotification, null);
-    yield countEntries(entry1[0], entry1[1], function(num) {
-      do_check_false(num, "doesn't exist after remove");
-      next_test();
   });
+  do_check_eq(TestObserver.observed.length, 1);
+  const {subject, data} = TestObserver.observed[0];
+  do_check_eq(data, "formhistory-remove");
+  do_check_true(isGUID.test(subject));
 
-    /* ========== 5 ========== */
+  const count = await promiseCountEntries(entry1[0], entry1[1]);
-    testnum++;
+  do_check_eq(count, 0, "doesn't exist after remove");
-    testdesc = "removeAllEntries";
+});
 
-    expectedNotification = "formhistory-remove";
+add_task(async function removeAllEntries() {
-    expectedData = null; // no data expected
+  await promiseAddEntry(entry1[0], entry1[1]);
-    yield updateEntry("remove", null, null, next_test);
+  await promiseAddEntry(entry2[0], entry2[1]);
+  await promiseAddEntry(entry3[0], entry3[1]);
+  TestObserver.reset();
 
-    do_check_eq(expectedNotification, null);
+  await promiseUpdateEntry("remove", null, null);
+  do_check_eq(TestObserver.observed.length, 3);
+  for (const notification of TestObserver.observed) {
+    const {subject, data} = notification;
+    do_check_eq(data, "formhistory-remove");
+    do_check_true(isGUID.test(subject));
+  }
 
-    /* ========== 6 ========== */
+  const count = await promiseCountEntries(null, null);
-    testnum++;
+  do_check_eq(count, 0);
-    testdesc = "removeAllEntries (again)";
+});
 
-    expectedNotification = "formhistory-remove";
+add_task(async function removeEntriesForName() {
-    expectedData = null;
+  await promiseAddEntry(entry1[0], entry1[1]);
-    yield updateEntry("remove", null, null, next_test);
+  await promiseAddEntry(entry2[0], entry2[1]);
+  await promiseAddEntry(entry3[0], entry3[1]);
+  TestObserver.reset();
 
-    do_check_eq(expectedNotification, null);
+  await promiseUpdateEntry("remove", entry2[0], null);
+  do_check_eq(TestObserver.observed.length, 1);
+  const {subject, data} = TestObserver.observed[0];
+  do_check_eq(data, "formhistory-remove");
+  do_check_true(isGUID.test(subject));
 
-    /* ========== 7 ========== */
+  let count = await promiseCountEntries(entry2[0], entry2[1]);
-    testnum++;
+  do_check_eq(count, 0);
-    testdesc = "removeEntriesForName";
 
-    expectedNotification = "formhistory-remove";
+  count = await promiseCountEntries(null, null);
-    expectedData = "field2";
+  do_check_eq(count, 2, "the other entries are still there");
-    yield updateEntry("remove", null, "field2", next_test);
 
-    do_check_eq(expectedNotification, null);
+  // Clean-up
+  await promiseUpdateEntry("remove", null, null);
+});
 
-    /* ========== 8 ========== */
+add_task(async function removeEntriesByTimeframe() {
-    testnum++;
+  await promiseAddEntry(entry1[0], entry1[1]);
-    testdesc = "removeEntriesByTimeframe";
+  await promiseAddEntry(entry2[0], entry2[1]);
 
-    expectedNotification = "formhistory-remove";
+  const cutoffDate = Date.now();
-    expectedData = [10, 99999999999];
+  // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+  await new Promise(res => setTimeout(res, 10));
 
-    yield FormHistory.update({
+  await promiseAddEntry(entry3[0], entry3[1]);
+  TestObserver.reset();
+
+  await new Promise(res => {
+    FormHistory.update({
       op: "remove",
-      firstUsedStart: expectedData[0],
+      firstUsedStart: 10,
-      firstUsedEnd: expectedData[1],
+      firstUsedEnd: cutoffDate * 1000,
     }, {
       handleCompletion(reason) {
         if (!reason) {
-          next_test();
+          res();
         }
       },
       handleErrors(error) {
         do_throw("Error occurred updating form history: " + error);
       },
     });
-
+  });
-    do_check_eq(expectedNotification, null);
+  do_check_eq(TestObserver.observed.length, 2);
-
+  for (const notification of TestObserver.observed) {
-    Services.obs.removeObserver(TestObserver, "satchel-storage-changed");
+    const {subject, data} = notification;
-
+    do_check_eq(data, "formhistory-remove");
-    do_test_finished();
+    do_check_true(isGUID.test(subject));
-  } catch (e) {
-    throw new Error(`FAILED in test #${testnum} -- ${testdesc}: ${e}`);
   }
-}
+
+  const count = await promiseCountEntries(null, null);
+  do_check_eq(count, 1, "entry2 should still be there");
+
+  // Clean-up
+  await promiseUpdateEntry("remove", null, null);
+});
+
+add_task(async function teardown() {
+  await promiseUpdateEntry("remove", null, null);
+  Services.obs.removeObserver(TestObserver, "satchel-storage-changed");
+});