Bug 976940 - FormHistory.update() should call handleError if form history is disabled and the operation is not a removal. r=markh
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 22 Feb 2016 18:17:43 +0100
changeset 322067 4b3e3358ff5cac1b8f6c7a6b4ab8e45f7e02833b
parent 322066 3eb3e6986e4b34193f54618f9db800e28c10e9d3
child 322068 6fe18594d84e5d68012d6d6ed5adc994fb6829f3
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs976940
milestone47.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 976940 - FormHistory.update() should call handleError if form history is disabled and the operation is not a removal. r=markh MozReview-Commit-ID: 94FRqGofGsP
browser/components/search/content/search.xml
browser/modules/ContentSearch.jsm
toolkit/components/satchel/FormHistory.jsm
toolkit/components/satchel/test/unit/test_history_api.js
toolkit/content/finddialog.js
--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@@ -399,17 +399,17 @@
       <method name="doSearch">
         <parameter name="aData"/>
         <parameter name="aWhere"/>
         <parameter name="aEngine"/>
         <body><![CDATA[
           var textBox = this._textbox;
 
           // Save the current value in the form history
-          if (aData && !PrivateBrowsingUtils.isWindowPrivate(window)) {
+          if (aData && !PrivateBrowsingUtils.isWindowPrivate(window) && this.FormHistory.enabled) {
             this.FormHistory.update(
               { op : "bump",
                 fieldname : textBox.getAttribute("autocompletesearchparam"),
                 value : aData },
               { handleError : function(aError) {
                   Components.utils.reportError("Saving search to form history failed: " + aError.message);
               }});
           }
--- a/browser/modules/ContentSearch.jsm
+++ b/browser/modules/ContentSearch.jsm
@@ -370,26 +370,28 @@ this.ContentSearch = {
       // properties, which won't be true if the browser has been destroyed.
       // That may be the case here due to the asynchronous nature of messaging.
       isPrivate = PrivateBrowsingUtils.isBrowserPrivate(msg.target);
     } catch (err) {}
     if (isPrivate || entry === "") {
       return Promise.resolve();
     }
     let browserData = this._suggestionDataForBrowser(msg.target, true);
-    FormHistory.update({
-      op: "bump",
-      fieldname: browserData.controller.formHistoryParam,
-      value: entry,
-    }, {
-      handleCompletion: () => {},
-      handleError: err => {
-        Cu.reportError("Error adding form history entry: " + err);
-      },
-    });
+    if (FormHistory.enabled) {
+      FormHistory.update({
+        op: "bump",
+        fieldname: browserData.controller.formHistoryParam,
+        value: entry,
+      }, {
+        handleCompletion: () => {},
+        handleError: err => {
+          Cu.reportError("Error adding form history entry: " + err);
+        },
+      });
+    }
     return Promise.resolve();
   },
 
   _onMessageRemoveFormHistoryEntry: function (msg, entry) {
     let browserData = this._suggestionDataForBrowser(msg.target);
     if (browserData && browserData.previousFormHistoryResult) {
       let { previousFormHistoryResult } = browserData;
       for (let i = 0; i < previousFormHistoryResult.matchCount; i++) {
--- a/toolkit/components/satchel/FormHistory.jsm
+++ b/toolkit/components/satchel/FormHistory.jsm
@@ -837,34 +837,44 @@ this.FormHistory = {
         }
       }
     };
 
     stmt.executeAsync(handlers);
   },
 
   update : function formHistoryUpdate(aChanges, aCallbacks) {
-    if (!Prefs.enabled) {
-      return;
-    }
-
     // Used to keep track of how many searches have been started. When that number
     // are finished, updateFormHistoryWrite can be called.
     let numSearches = 0;
     let completedSearches = 0;
     let searchFailed = false;
 
     function validIdentifier(change) {
       // The identifier is only valid if one of either the guid or the (fieldname/value) are set
       return Boolean(change.guid) != Boolean(change.fieldname && change.value);
     }
 
     if (!("length" in aChanges))
       aChanges = [aChanges];
 
+    let isRemoveOperation = aChanges.every(change => change && change.op && change.op == "remove");
+    if (!Prefs.enabled && !isRemoveOperation) {
+      if (aCallbacks && aCallbacks.handleError) {
+        aCallbacks.handleError({
+          message: "Form history is disabled, only remove operations are allowed",
+          result: Ci.mozIStorageError.MISUSE
+        });
+      }
+      if (aCallbacks && aCallbacks.handleCompletion) {
+        aCallbacks.handleCompletion(1);
+      }
+      return;
+    }
+
     for (let change of aChanges) {
       switch (change.op) {
         case "remove":
           validateSearchData(change, "Remove");
           continue;
         case "update":
           if (validIdentifier(change)) {
             validateOpData(change, "Update");
--- a/toolkit/components/satchel/test/unit/test_history_api.js
+++ b/toolkit/components/satchel/test/unit/test_history_api.js
@@ -53,27 +53,31 @@ function promiseUpdateEntry(op, name, va
   var change = { op: op };
   if (name !== null)
     change.fieldname = name;
   if (value !== null)
     change.value = value;
   return promiseUpdate(change);
 }
 
-function promiseUpdate(change)
-{
-  let deferred = Promise.defer();
-  FormHistory.update(change,
-                     { handleError: function (error) {
-                         do_throw("Error occurred updating form history: " + error);
-                         deferred.reject(error);
-                       },
-                       handleCompletion: function (reason) { if (!reason) deferred.resolve(); }
-                     });
-  return deferred.promise;
+function promiseUpdate(change) {
+  return new Promise((resolve, reject) => {
+    FormHistory.update(change, {
+      handleError(error) {
+        this._error = error;
+      },
+      handleCompletion(reason) {
+        if (reason) {
+          reject(this._error);
+        } else {
+          resolve();
+        }
+      }
+    });
+  });
 }
 
 function promiseSearchEntries(terms, params)
 {
   let deferred = Promise.defer();
   let results = [];
   FormHistory.search(terms, params,
                      { handleResult: result => results.push(result),
@@ -400,16 +404,49 @@ add_task(function* ()
   do_check_eq(13, results[3].timesUsed);
   do_check_eq(230, results[2].firstUsed);
   do_check_eq(430, results[3].firstUsed);
   do_check_true(results[2].lastUsed > 600);
   do_check_true(results[3].lastUsed > 700);
 
   yield promiseCountEntries(null, null, num => do_check_eq(num, 4));
 
+  // ===== 21 =====
+  // Check update fails if form history is disabled and the operation is not a
+  // pure removal.
+  testnum++;
+  Services.prefs.setBoolPref("browser.formfill.enable", false);
+
+  // Cannot use arrow functions, see bug 1237961.
+  Assert.rejects(promiseUpdate(
+                   { op : "bump", fieldname: "field5", value: "value5" }),
+                 function(err) { return err.result == Ci.mozIStorageError.MISUSE; },
+                 "bumping when form history is disabled should fail");
+  Assert.rejects(promiseUpdate(
+                   { op : "add", fieldname: "field5", value: "value5" }),
+                 function(err) { return err.result == Ci.mozIStorageError.MISUSE; },
+                 "Adding when form history is disabled should fail");
+  Assert.rejects(promiseUpdate([
+                     { op : "update", fieldname: "field5", value: "value5" },
+                     { op : "remove", fieldname: "field5", value: "value5" }
+                   ]),
+                 function(err) { return err.result == Ci.mozIStorageError.MISUSE; },
+                 "mixed operations when form history is disabled should fail");
+  Assert.rejects(promiseUpdate([
+                     null, undefined, "", 1, {},
+                     { op : "remove", fieldname: "field5", value: "value5" }
+                   ]),
+                 function(err) { return err.result == Ci.mozIStorageError.MISUSE; },
+                 "Invalid entries when form history is disabled should fail");
+
+  // Remove should work though.
+  yield promiseUpdate([{ op: "remove", fieldname: "field5", value: null },
+                       { op: "remove", fieldname: null, value: null }]);
+  Services.prefs.clearUserPref("browser.formfill.enable");
+
   } catch (e) {
     throw "FAILED in test #" + testnum + " -- " + e;
   }
   finally {
     FormHistory._supportsDeletedTable = oldSupportsDeletedTable;
     dbConnection.asyncClose(do_test_finished);
   }
 });
--- a/toolkit/content/finddialog.js
+++ b/toolkit/content/finddialog.js
@@ -131,19 +131,21 @@ function doEnabling()
 
 function updateFormHistory()
 {
   if (window.opener.PrivateBrowsingUtils &&
       window.opener.PrivateBrowsingUtils.isWindowPrivate(window.opener) ||
       !dialog.findKey.value)
     return;
 
-  FormHistory.update({
-    op: "bump",
-    fieldname: "find-dialog",
-    value: dialog.findKey.value
-  }, {
-    handleError: function(aError) {
-      Components.utils.reportError("Saving find to form history failed: " +
-                                   aError.message);
-    }
-  });
+  if (FormHistory.enabled) {
+    FormHistory.update({
+      op: "bump",
+      fieldname: "find-dialog",
+      value: dialog.findKey.value
+    }, {
+      handleError: function(aError) {
+        Components.utils.reportError("Saving find to form history failed: " +
+                                     aError.message);
+      }
+    });
+  }
 }