Bug 1395427 p2 - Include guid in formhistory-remove notifications. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Fri, 22 Sep 2017 15:19:56 -0400
changeset 671232 8fe0c98bc00080b2c268359cc8960e2c24966649
parent 671137 02f2b7a34f3ef8130309b1bfdf01e9c742c75f05
child 671233 67b8a285910207821f403ddb2be312843aefd03a
push id81882
push userbmo:eoger@fastmail.com
push dateWed, 27 Sep 2017 17:08:01 +0000
reviewersmarkh
bugs1395427
milestone58.0a1
Bug 1395427 p2 - Include guid in formhistory-remove notifications. r?markh MozReview-Commit-ID: Je0rV277d7
toolkit/components/satchel/FormHistory.jsm
toolkit/components/satchel/FormHistoryStartup.js
toolkit/components/satchel/nsFormAutoComplete.js
toolkit/components/satchel/test/unit/test_notify.js
--- a/toolkit/components/satchel/FormHistory.jsm
+++ b/toolkit/components/satchel/FormHistory.jsm
@@ -1032,17 +1032,17 @@ this.FormHistory = {
      * 3) additional weight for aged entries surviving expiry - these entries are relevant
      *    since they have been used multiple times over a large time span so rank them higher
      * The score is then divided by the bucket size and we round the result so that entries
      * with a very similar frecency are bucketed together with an alphabetical sort. This is
      * to reduce the amount of moving around by entries while typing.
      */
 
     let query = "/* do not warn (bug 496471): can't use an index */ " +
-                "SELECT value, " +
+                "SELECT value, guid, " +
                 "ROUND( " +
                     "timesUsed / MAX(1.0, (lastUsed - firstUsed) / :timeGroupingSize) * " +
                     "MAX(1.0, :maxTimeGroupings - (:now - lastUsed) / :timeGroupingSize) * " +
                     "MAX(1.0, :agedWeight * (firstUsed < :expiryDate)) / " +
                     ":bucketSize " +
                 ", 3) AS frecency, " +
                 boundaryCalc + " AS boundaryBonuses " +
                 "FROM moz_formhistory " +
@@ -1068,19 +1068,21 @@ this.FormHistory = {
       // no additional params need to be substituted into the query when the
       // length is zero or one
     }
 
     let pending = stmt.executeAsync({
       handleResult(aResultSet) {
         for (let row = aResultSet.getNextRow(); row; row = aResultSet.getNextRow()) {
           let value = row.getResultByName("value");
+          let guid = row.getResultByName("guid");
           let frecency = row.getResultByName("frecency");
           let entry = {
             text:          value,
+            guid,
             textLowerCase: value.toLowerCase(),
             frecency,
             totalScore:    Math.round(frecency * row.getResultByName("boundaryBonuses")),
           };
           if (aCallbacks && aCallbacks.handleResult) {
             aCallbacks.handleResult(entry);
           }
         }
--- a/toolkit/components/satchel/FormHistoryStartup.js
+++ b/toolkit/components/satchel/FormHistoryStartup.js
@@ -121,21 +121,22 @@ FormHistoryStartup.prototype = {
         };
 
         query = FormHistory.getAutoCompleteResults(searchString, params, processResults);
         this.pendingQuery = query;
         break;
       }
 
       case "FormHistory:RemoveEntry": {
-        let { inputName, value } = message.data;
+        let { inputName, value, guid } = message.data;
         FormHistory.update({
           op: "remove",
           fieldname: inputName,
           value,
+          guid,
         });
         break;
       }
     }
   },
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([FormHistoryStartup]);
--- a/toolkit/components/satchel/nsFormAutoComplete.js
+++ b/toolkit/components/satchel/nsFormAutoComplete.js
@@ -112,21 +112,26 @@ FormHistoryClient.prototype = {
 
   /**
    * Remove an item from FormHistory.
    *
    * @param {string} value
    *
    *        The value to remove for this particular
    *        field.
+   *
+   * @param {string} guid
+   *
+   *        The guid for the item being removed.
    */
-  remove(value) {
+  remove(value, guid) {
     this.mm.sendAsyncMessage("FormHistory:RemoveEntry", {
       inputName: this.inputName,
       value,
+      guid,
     });
   },
 
   // Private methods
 
   receiveMessage(msg) {
     let { id, results } = msg.data;
     if (id != this.id) {
@@ -616,14 +621,14 @@ FormAutoCompleteResult.prototype = {
   },
 
   removeValueAt(index, removeFromDB) {
     this._checkIndexBounds(index);
 
     let [removedEntry] = this.entries.splice(index, 1);
 
     if (removeFromDB) {
-      this.client.remove(removedEntry.text);
+      this.client.remove(removedEntry.text, removedEntry.guid);
     }
   },
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([FormAutoComplete]);
--- a/toolkit/components/satchel/test/unit/test_notify.js
+++ b/toolkit/components/satchel/test/unit/test_notify.js
@@ -2,32 +2,43 @@
  * Test suite for satchel notifications
  *
  * Tests notifications dispatched when modifying form history.
  *
  */
 
 let expectedNotification;
 let expectedData;
+let subjectIsGuid = false;
+let lastGUID;
 
 let TestObserver = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
 
   observe(subject, topic, data) {
     do_check_eq(topic, "satchel-storage-changed");
     do_check_eq(data, expectedNotification);
 
+    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":
-        do_check_true(subject instanceof Ci.nsISupportsString);
-        do_check_true(isGUID.test(subject.toString()));
+        verifySubjectIsGuid();
         break;
       case "formhistory-remove":
-        do_check_eq(null, subject);
+        if (subjectIsGuid) {
+          verifySubjectIsGuid();
+        } else {
+          do_check_eq(null, subject);
+        }
         break;
       default:
         do_throw("Unhandled notification: " + data + " / " + topic);
     }
 
     expectedNotification = null;
     expectedData = null;
   },
@@ -97,17 +108,34 @@ function* run_test_steps() {
     do_check_eq(expectedNotification, null);
 
     /* ========== 4 ========== */
     testnum++;
     testdesc = "removeEntry";
 
     expectedNotification = "formhistory-remove";
     expectedData = entry1;
-    yield updateEntry("remove", entry1[0], entry1[1], next_test);
+
+    subjectIsGuid = true;
+    yield FormHistory.update({
+      op: "remove",
+      fieldname: entry1[0],
+      value: entry1[1],
+      guid: lastGUID,
+    }, {
+      handleError(error) {
+        do_throw("Error occurred updating form history: " + error);
+      },
+      handleCompletion(reason) {
+        if (!reason) {
+          next_test();
+        }
+      },
+    });
+    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();
     });
 
     /* ========== 5 ========== */