Bug 1395427 p2 - Include guid in formhistory-remove notifications. r?markh
MozReview-Commit-ID: Je0rV277d7
--- 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 ========== */