Bug 1395427 p2 - Include guid in formhistory-remove notifications. r=markh
☠☠ backed out by 583edc8f8c18 ☠ ☠
authorEdouard Oger <eoger@fastmail.com>
Fri, 22 Sep 2017 15:19:56 -0400
changeset 383224 553d26ab8ee8641dd91972b43286d0ce9fc598dc
parent 383223 cf5ecd16d87ed1afb96e4956d06fa2b50f260bea
child 383225 d7876bfd8bfce35e6ef351f395038d6298860bd0
push id32587
push userarchaeopteryx@coole-files.de
push dateWed, 27 Sep 2017 21:55:32 +0000
treeherdermozilla-central@69e3f8981645 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1395427
milestone58.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 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 guid (string)
+   *
+   *        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,39 @@
  * 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);
+        subjectIsGuid ? verifySubjectIsGuid() : do_check_eq(null, subject);
         break;
       default:
         do_throw("Unhandled notification: " + data + " / " + topic);
     }
 
     expectedNotification = null;
     expectedData = null;
   },
@@ -97,17 +104,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 ========== */