Bug 1325724 - Fallback to form history if there is no form autofill profile saved, r?MattN draft
authorsteveck-chung <schung@mozilla.com>
Wed, 08 Feb 2017 13:42:27 +0800
changeset 485162 884c05b7c25951a9ca59944250cb113043c61a6f
parent 484187 1060668405a9399774c205430de4a7001d3f27ac
child 485187 7f552af908f2f048431444c31394db995c3262b2
push id45657
push userbmo:schung@mozilla.com
push dateThu, 16 Feb 2017 08:14:42 +0000
reviewersMattN
bugs1325724
milestone54.0a1
Bug 1325724 - Fallback to form history if there is no form autofill profile saved, r?MattN MozReview-Commit-ID: FAUJK1wTvLn
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/head.js
browser/extensions/formautofill/test/unit/test_enabledStatus.js
browser/extensions/formautofill/test/unit/test_profileStorage.js
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -64,18 +64,19 @@ FormAutofillParent.prototype = {
    */
   init() {
     let storePath = OS.Path.join(OS.Constants.Path.profileDir, PROFILE_JSON_FILE_NAME);
     this._profileStore = new ProfileStorage(storePath);
     this._profileStore.initialize();
 
     Services.obs.addObserver(this, "advanced-pane-loaded", false);
 
-    // Observing the pref (and storage) changes
+    // Observing the pref and storage changes
     Services.prefs.addObserver(ENABLED_PREF, this, false);
+    Services.obs.addObserver(this, "formautofill-storage-changed", false);
     this._enabled = this._getStatus();
     // Force to trigger the onStatusChanged function for setting listeners properly
     // while initizlization
     this._onStatusChanged();
     Services.ppmm.addMessageListener("FormAutofill:getEnabledStatus", this);
   },
 
   observe(subject, topic, data) {
@@ -95,16 +96,30 @@ FormAutofillParent.prototype = {
         let currentStatus = this._getStatus();
         if (currentStatus !== this._enabled) {
           this._enabled = currentStatus;
           this._onStatusChanged();
         }
         break;
       }
 
+      case "formautofill-storage-changed": {
+        // Early exit if the action is not "add" nor "remove"
+        if (data != "add" && data != "remove") {
+          break;
+        }
+
+        let currentStatus = this._getStatus();
+        if (currentStatus !== this._enabled) {
+          this._enabled = currentStatus;
+          this._onStatusChanged();
+        }
+        break;
+      }
+
       default: {
         throw new Error(`FormAutofillParent: Unexpected topic observed: ${topic}`);
       }
     }
   },
 
   /**
    * Add/remove message listener and broadcast the status to frames while the
@@ -116,23 +131,27 @@ FormAutofillParent.prototype = {
     } else {
       Services.ppmm.removeMessageListener("FormAutofill:GetProfiles", this);
     }
 
     Services.ppmm.broadcastAsyncMessage("FormAutofill:enabledStatus", this._enabled);
   },
 
   /**
-   * Query pref (and storage) status to determine the overall status for
+   * Query pref and storage status to determine the overall status for
    * form autofill feature.
    *
    * @returns {boolean} status of form autofill feature
    */
   _getStatus() {
-    return Services.prefs.getBoolPref(ENABLED_PREF);
+    if (!Services.prefs.getBoolPref(ENABLED_PREF)) {
+      return false;
+    }
+
+    return this._profileStore.getAll().length > 0;
   },
 
   /**
    * Handles the message coming from FormAutofillContent.
    *
    * @param   {string} message.name The name of the message.
    * @param   {object} message.data The data of the message.
    * @param   {nsIFrameMessageManager} message.target Caller's message manager.
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -119,16 +119,17 @@ ProfileStorage.prototype = {
     profileToSave.timeCreated = now;
     profileToSave.timeLastModified = now;
     profileToSave.timeLastUsed = 0;
     profileToSave.timesUsed = 0;
 
     this._store.data.profiles.push(profileToSave);
 
     this._store.saveSoon();
+    Services.obs.notifyObservers(null, "formautofill-storage-changed", "add");
   },
 
   /**
    * Update the specified profile.
    *
    * @param  {string} guid
    *         Indicates which profile to update.
    * @param  {Profile} profile
@@ -149,16 +150,17 @@ ProfileStorage.prototype = {
       } else {
         delete profileFound[field];
       }
     }
 
     profileFound.timeLastModified = Date.now();
 
     this._store.saveSoon();
+    Services.obs.notifyObservers(null, "formautofill-storage-changed", "update");
   },
 
   /**
    * Notifies the stroage of the use of the specified profile, so we can update
    * the metadata accordingly.
    *
    * @param  {string} guid
    *         Indicates which profile to be notified.
@@ -170,30 +172,32 @@ ProfileStorage.prototype = {
     if (!profileFound) {
       throw new Error("No matching profile.");
     }
 
     profileFound.timesUsed++;
     profileFound.timeLastUsed = Date.now();
 
     this._store.saveSoon();
+    Services.obs.notifyObservers(null, "formautofill-storage-changed", "notifyUsed");
   },
 
   /**
    * Removes the specified profile. No error occurs if the profile isn't found.
    *
    * @param  {string} guid
    *         Indicates which profile to remove.
    */
   remove(guid) {
     this._store.ensureDataReady();
 
     this._store.data.profiles =
       this._store.data.profiles.filter(profile => profile.guid != guid);
     this._store.saveSoon();
+    Services.obs.notifyObservers(null, "formautofill-storage-changed", "remove");
   },
 
   /**
    * Returns the profile with the specified GUID.
    *
    * @param   {string} guid
    *          Indicates which profile to retrieve.
    * @returns {Profile}
@@ -241,17 +245,17 @@ ProfileStorage.prototype = {
     return Object.assign({}, profile);
   },
 
   _findByGUID(guid) {
     return this._store.data.profiles.find(profile => profile.guid == guid);
   },
 
   _findByFilter({info, searchString}) {
-    let profiles = MOCK_MODE ? MOCK_STORAGE : this._store.data.profiles;
+    let profiles = this._store.data.profiles;
     let lcSearchString = searchString.toLowerCase();
 
     return profiles.filter(profile => {
       // Return true if string is not provided and field exists.
       // TODO: We'll need to check if the address is for billing or shipping.
       let name = profile[info.fieldName];
 
       if (!searchString) {
@@ -276,17 +280,17 @@ ProfileStorage.prototype = {
       result[key] = profile[key];
     }
     return result;
   },
 
   _dataPostProcessor(data) {
     data.version = SCHEMA_VERSION;
     if (!data.profiles) {
-      data.profiles = [];
+      data.profiles = MOCK_MODE ? MOCK_STORAGE : [];
     }
     return data;
   },
 
   // For test only.
   _saveImmediately() {
     return this._store._save();
   },
--- a/browser/extensions/formautofill/test/unit/head.js
+++ b/browser/extensions/formautofill/test/unit/head.js
@@ -7,16 +7,17 @@
 "use strict";
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/NetUtil.jsm");
 Cu.import("resource://testing-common/MockDocument.jsm");
+Cu.import("resource://testing-common/TestUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "DownloadPaths",
                                   "resource://gre/modules/DownloadPaths.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
                                   "resource://gre/modules/FileUtils.jsm");
 
 do_get_profile();
 
--- a/browser/extensions/formautofill/test/unit/test_enabledStatus.js
+++ b/browser/extensions/formautofill/test/unit/test_enabledStatus.js
@@ -29,22 +29,56 @@ add_task(function* test_enabledStatus_ob
   formAutofillParent._getStatus.returns(true);
   formAutofillParent.observe(null, "nsPref:changed", "browser.formautofill.enabled");
   do_check_eq(formAutofillParent._onStatusChanged.called, false);
 
   // _enabled != _getStatus() => Need to trigger onStatusChanged
   formAutofillParent._getStatus.returns(false);
   formAutofillParent.observe(null, "nsPref:changed", "browser.formautofill.enabled");
   do_check_eq(formAutofillParent._onStatusChanged.called, true);
+
+  // profile added => Need to trigger onStatusChanged
+  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
+  formAutofillParent._onStatusChanged.reset();
+  formAutofillParent.observe(null, "formautofill-storage-changed", "add");
+  do_check_eq(formAutofillParent._onStatusChanged.called, true);
+
+  // profile removed => Need to trigger onStatusChanged
+  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
+  formAutofillParent._onStatusChanged.reset();
+  formAutofillParent.observe(null, "formautofill-storage-changed", "remove");
+  do_check_eq(formAutofillParent._onStatusChanged.called, true);
+
+  // profile updated => no need to trigger onStatusChanged
+  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
+  formAutofillParent._onStatusChanged.reset();
+  formAutofillParent.observe(null, "formautofill-storage-changed", "update");
+  do_check_eq(formAutofillParent._onStatusChanged.called, false);
 });
 
 add_task(function* test_enabledStatus_getStatus() {
   let formAutofillParent = new FormAutofillParent();
   do_register_cleanup(function cleanup() {
     Services.prefs.clearUserPref("browser.formautofill.enabled");
   });
 
+  let fakeStorage = [];
+  formAutofillParent._profileStore = {
+    getAll: () => fakeStorage,
+  };
+
+  // pref is enabled and profile is empty.
+  Services.prefs.setBoolPref("browser.formautofill.enabled", true);
+  do_check_eq(formAutofillParent._getStatus(), false);
+
+  // pref is disabled and profile is empty.
+  Services.prefs.setBoolPref("browser.formautofill.enabled", false);
+  do_check_eq(formAutofillParent._getStatus(), false);
+
+  fakeStorage = ["test-profile"];
+  // pref is enabled and profile is not empty.
   Services.prefs.setBoolPref("browser.formautofill.enabled", true);
   do_check_eq(formAutofillParent._getStatus(), true);
 
+  // pref is disabled and profile is not empty.
   Services.prefs.setBoolPref("browser.formautofill.enabled", false);
   do_check_eq(formAutofillParent._getStatus(), false);
 });
--- a/browser/extensions/formautofill/test/unit/test_profileStorage.js
+++ b/browser/extensions/formautofill/test/unit/test_profileStorage.js
@@ -34,17 +34,20 @@ const TEST_PROFILE_WITH_INVALID_FIELD = 
   "street-address": "Another Address",
   invalidField: "INVALID",
 };
 
 let prepareTestProfiles = Task.async(function* (path) {
   let profileStorage = new ProfileStorage(path);
   yield profileStorage.initialize();
 
+  let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
+                                          (subject, data) => data == "add");
   profileStorage.add(TEST_PROFILE_1);
+  yield onChanged;
   profileStorage.add(TEST_PROFILE_2);
   yield profileStorage._saveImmediately();
 });
 
 let do_check_profile_matches = (profileWithMeta, profile) => {
   for (let key in profile) {
     do_check_eq(profileWithMeta[key], profile[key]);
   }
@@ -169,19 +172,23 @@ add_task(function* test_update() {
 
   let profileStorage = new ProfileStorage(path);
   yield profileStorage.initialize();
 
   let profiles = profileStorage.getAll();
   let guid = profiles[1].guid;
   let timeLastModified = profiles[1].timeLastModified;
 
+  let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
+                                          (subject, data) => data == "update");
+
   do_check_neq(profiles[1].country, undefined);
 
   profileStorage.update(guid, TEST_PROFILE_3);
+  yield onChanged;
   yield profileStorage._saveImmediately();
 
   profileStorage = new ProfileStorage(path);
   yield profileStorage.initialize();
 
   let profile = profileStorage.get(guid);
 
   do_check_eq(profile.country, undefined);
@@ -206,17 +213,21 @@ add_task(function* test_notifyUsed() {
   let profileStorage = new ProfileStorage(path);
   yield profileStorage.initialize();
 
   let profiles = profileStorage.getAll();
   let guid = profiles[1].guid;
   let timeLastUsed = profiles[1].timeLastUsed;
   let timesUsed = profiles[1].timesUsed;
 
+  let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
+                                          (subject, data) => data == "notifyUsed");
+
   profileStorage.notifyUsed(guid);
+  yield onChanged;
   yield profileStorage._saveImmediately();
 
   profileStorage = new ProfileStorage(path);
   yield profileStorage.initialize();
 
   let profile = profileStorage.get(guid);
 
   do_check_eq(profile.timesUsed, timesUsed + 1);
@@ -231,19 +242,23 @@ add_task(function* test_remove() {
   yield prepareTestProfiles(path);
 
   let profileStorage = new ProfileStorage(path);
   yield profileStorage.initialize();
 
   let profiles = profileStorage.getAll();
   let guid = profiles[1].guid;
 
+  let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
+                                          (subject, data) => data == "remove");
+
   do_check_eq(profiles.length, 2);
 
   profileStorage.remove(guid);
+  yield onChanged;
   yield profileStorage._saveImmediately();
 
   profileStorage = new ProfileStorage(path);
   yield profileStorage.initialize();
 
   profiles = profileStorage.getAll();
 
   do_check_eq(profiles.length, 1);