Bug 888784 - Prevent duplicate fieldname/value pairs from being inserted in FormHistory. r?mak draft
authorMike Conley <mconley@mozilla.com>
Tue, 09 Jan 2018 18:54:39 -0500
changeset 723118 248a1bd926505df4712d228662e8913969f9ed38
parent 723117 402a7c0cbd467b52841f7ae8a2e08a104572d0bb
child 723119 68ac9fc8a26b9adfc00abb09adbefaa2e957dd3b
push id96341
push usermconley@mozilla.com
push dateMon, 22 Jan 2018 16:46:17 +0000
reviewersmak
bugs888784
milestone60.0a1
Bug 888784 - Prevent duplicate fieldname/value pairs from being inserted in FormHistory. r?mak MozReview-Commit-ID: 9icdBPW2Px9
toolkit/components/satchel/FormHistory.jsm
--- a/toolkit/components/satchel/FormHistory.jsm
+++ b/toolkit/components/satchel/FormHistory.jsm
@@ -323,32 +323,68 @@ function prepareInsertQuery(change, now)
   updatedChange.lastUsed = updatedChange.lastUsed || now;
 
   return {
     updatedChange,
     query,
   };
 }
 
+// There is a fieldname / value uniqueness constraint that's at this time
+// only enforced at this level. This Map maps fieldnames => values that
+// are in the process of being inserted into the database so that we know
+// not to try to insert the same ones on top. Attempts to do so will be
+// ignored.
+this.InProgressInserts = {
+  _inProgress: new Map(),
+
+  add(fieldname, value) {
+    let fieldnameSet = this._inProgress.get(fieldname);
+    if (!fieldnameSet) {
+      this._inProgress.set(fieldname, new Set([value]));
+      return true;
+    }
+
+    if (!fieldnameSet.has(value)) {
+      fieldnameSet.add(value);
+      return true;
+    }
+
+    return false;
+  },
+
+  clear(fieldnamesAndValues) {
+    for (let [fieldname, value] of fieldnamesAndValues) {
+      let fieldnameSet = this._inProgress.get(fieldname);
+      if (fieldnameSet &&
+          fieldnameSet.delete(value) &&
+          fieldnameSet.size == 0) {
+        this._inProgress.delete(fieldname);
+      }
+    }
+  },
+};
+
 /**
  * Constructs and executes database statements from a pre-processed list of
  * inputted changes.
  *
  * @param {Array.<Object>} aChanges changes to form history
  * @param {Object} aPreparedHandlers
  */
 // XXX This should be split up and the complexity reduced.
 // eslint-disable-next-line complexity
 async function updateFormHistoryWrite(aChanges, aPreparedHandlers) {
   log("updateFormHistoryWrite  " + aChanges.length);
 
   // pass 'now' down so that every entry in the batch has the same timestamp
   let now = Date.now() * 1000;
   let queries = [];
   let notifications = [];
+  let adds = [];
   let conn = await FormHistory.db;
 
   for (let change of aChanges) {
     let operation = change.op;
     delete change.op;
     switch (operation) {
       case "remove": {
         log("Remove from form history  " + change);
@@ -431,24 +467,37 @@ async function updateFormHistoryWrite(aC
           let queryParams = {
             lastUsed: now,
             guid: change.guid,
           };
 
           queries.push({ query, params: queryParams });
           notifications.push(["formhistory-update", change.guid]);
         } else {
+          if (!InProgressInserts.add(change.fieldname, change.value)) {
+            // This updateFormHistoryWrite call, or a previous one, is already
+            // going to add this fieldname / value pair, so we can ignore this.
+            continue;
+          }
+          adds.push([change.fieldname, change.value]);
           change.guid = generateGUID();
           let { query, updatedChange } = prepareInsertQuery(change, now);
           queries.push({ query, params: updatedChange });
           notifications.push(["formhistory-add", updatedChange.guid]);
         }
         break;
       }
       case "add": {
+        if (!InProgressInserts.add(change.fieldname, change.value)) {
+          // This updateFormHistoryWrite call, or a previous one, is already
+          // going to add this fieldname / value pair, so we can ignore this.
+          continue;
+        }
+        adds.push([change.fieldname, change.value]);
+
         log("Add to form history " + change);
         if (!change.guid) {
           change.guid = generateGUID();
         }
 
         let { query, updatedChange } = prepareInsertQuery(change, now);
         queries.push({ query, params: updatedChange });
         notifications.push(["formhistory-add", updatedChange.guid]);
@@ -459,28 +508,28 @@ async function updateFormHistoryWrite(aC
         throw Components.Exception("Invalid operation " + operation,
                                    Cr.NS_ERROR_ILLEGAL_VALUE);
       }
     }
   }
 
   try {
     await runUpdateQueries(conn, queries);
+    for (let [notification, param] of notifications) {
+      // We're either sending a GUID or nothing at all.
+      sendNotification(notification, param);
+    }
+
+    aPreparedHandlers.handleCompletion(0);
   } catch (e) {
     aPreparedHandlers.handleError(e);
     aPreparedHandlers.handleCompletion(1);
-    return;
+  } finally {
+    InProgressInserts.clear(adds);
   }
-
-  for (let [notification, param] of notifications) {
-    // We're either sending a GUID or nothing at all.
-    sendNotification(notification, param);
-  }
-
-  aPreparedHandlers.handleCompletion(0);
 }
 
 /**
  * Runs queries for an update operation to the database. This
  * is separated out from updateFormHistoryWrite to avoid shutdown
  * leaks where the handlers passed to updateFormHistoryWrite would
  * leak from the closure around the executeTransaction function.
  *