Bug 1364477 - Only getAll Address Profiles once at initialization. r=steveck draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 23 May 2017 00:47:59 -0400
changeset 582807 098db1d42c1811ef27302725f7ab7f528dcf7bc3
parent 581015 c973f678908e6118a02ebbfab921b89e0f87c600
child 582808 2789e74d440f252730d2b2598a2b762ec8356aac
push id60184
push usermozilla@noorenberghe.ca
push dateTue, 23 May 2017 04:48:58 +0000
reviewerssteveck
bugs1364477
milestone55.0a1
Bug 1364477 - Only getAll Address Profiles once at initialization. r=steveck MozReview-Commit-ID: Hw1twbznDsY
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/test/unit/test_activeStatus.js
browser/extensions/formautofill/test/unit/test_enabledStatus.js
browser/extensions/formautofill/test/unit/xpcshell.ini
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -51,20 +51,19 @@ const ENABLED_PREF = "extensions.formaut
 
 function FormAutofillParent() {
 }
 
 FormAutofillParent.prototype = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports, Ci.nsIObserver]),
 
   /**
-   * Whether Form Autofill is enabled in preferences.
-   * Caches the latest value of this._getStatus().
+   * Cache of the Form Autofill status (considering preferences and storage).
    */
-  _enabled: false,
+  _active: null,
 
   /**
    * Initializes ProfileStorage and registers the message handler.
    */
   async init() {
     log.debug("init");
     await profileStorage.initialize();
 
@@ -72,19 +71,17 @@ FormAutofillParent.prototype = {
     Services.ppmm.addMessageListener("FormAutofill:GetAddresses", this);
     Services.ppmm.addMessageListener("FormAutofill:SaveAddress", this);
     Services.ppmm.addMessageListener("FormAutofill:RemoveAddresses", this);
 
     // Observing the pref and storage changes
     Services.prefs.addObserver(ENABLED_PREF, this);
     Services.obs.addObserver(this, "formautofill-storage-changed");
 
-    // Force to trigger the onStatusChanged function for setting listeners properly
-    // while initizlization
-    this._setStatus(this._getStatus());
+    // Update the saved field names to compute the status and update child processes.
     this._updateSavedFieldNames();
   },
 
   observe(subject, topic, data) {
     log.debug("observe:", topic, "with data:", data);
     switch (topic) {
       case "advanced-pane-loaded": {
         let useOldOrganization = Services.prefs.getBoolPref("browser.preferences.useOldOrganization",
@@ -98,77 +95,71 @@ FormAutofillParent.prototype = {
         let insertBeforeNode = useOldOrganization ?
                                document.getElementById("locationBarGroup") :
                                document.getElementById("masterPasswordRow");
         parentNode.insertBefore(prefGroup, insertBeforeNode);
         break;
       }
 
       case "nsPref:changed": {
-        // Observe pref changes and update _enabled cache if status is changed.
-        let currentStatus = this._getStatus();
-        if (currentStatus !== this._enabled) {
-          this._setStatus(currentStatus);
-        }
+        // Observe pref changes and update _active cache if status is changed.
+        this._updateStatus();
         break;
       }
 
       case "formautofill-storage-changed": {
         // Early exit if the action is not "add" nor "remove"
         if (data != "add" && data != "remove") {
           break;
         }
 
         this._updateSavedFieldNames();
-        let currentStatus = this._getStatus();
-        if (currentStatus !== this._enabled) {
-          this._setStatus(currentStatus);
-        }
         break;
       }
 
       default: {
         throw new Error(`FormAutofillParent: Unexpected topic observed: ${topic}`);
       }
     }
   },
 
   /**
    * Broadcast the status to frames when the form autofill status changes.
    */
   _onStatusChanged() {
-    log.debug("_onStatusChanged: Status changed to", this._enabled);
-    Services.ppmm.broadcastAsyncMessage("FormAutofill:enabledStatus", this._enabled);
+    log.debug("_onStatusChanged: Status changed to", this._active);
+    Services.ppmm.broadcastAsyncMessage("FormAutofill:enabledStatus", this._active);
     // Sync process data autofillEnabled to make sure the value up to date
     // no matter when the new content process is initialized.
-    Services.ppmm.initialProcessData.autofillEnabled = this._enabled;
+    Services.ppmm.initialProcessData.autofillEnabled = this._active;
   },
 
   /**
-   * Query pref and storage status to determine the overall status for
+   * Query preference and storage status to determine the overall status of the
    * form autofill feature.
    *
-   * @returns {boolean} status of form autofill feature
+   * @returns {boolean} whether form autofill is active (enabled and has data)
    */
-  _getStatus() {
+  _computeStatus() {
     if (!Services.prefs.getBoolPref(ENABLED_PREF)) {
       return false;
     }
 
-    return profileStorage.addresses.getAll({noComputedFields: true}).length > 0;
+    return Services.ppmm.initialProcessData.autofillSavedFieldNames.size > 0;
   },
 
   /**
-   * Set status and trigger _onStatusChanged.
-   *
-   * @param {boolean} newStatus The latest status we want to set for _enabled
+   * Update the status and trigger _onStatusChanged, if necessary.
    */
-  _setStatus(newStatus) {
-    this._enabled = newStatus;
-    this._onStatusChanged();
+  _updateStatus() {
+    let wasActive = this._active;
+    this._active = this._computeStatus();
+    if (this._active !== wasActive) {
+      this._onStatusChanged();
+    }
   },
 
   /**
    * 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.
@@ -229,16 +220,17 @@ FormAutofillParent.prototype = {
     } else {
       addresses = profileStorage.addresses.getAll();
     }
 
     target.sendAsyncMessage("FormAutofill:Addresses", addresses);
   },
 
   _updateSavedFieldNames() {
+    log.debug("_updateSavedFieldNames");
     if (!Services.ppmm.initialProcessData.autofillSavedFieldNames) {
       Services.ppmm.initialProcessData.autofillSavedFieldNames = new Set();
     } else {
       Services.ppmm.initialProcessData.autofillSavedFieldNames.clear();
     }
 
     profileStorage.addresses.getAll().forEach((address) => {
       Object.keys(address).forEach((fieldName) => {
@@ -251,10 +243,11 @@ FormAutofillParent.prototype = {
 
     // Remove the internal guid and metadata fields.
     profileStorage.INTERNAL_FIELDS.forEach((fieldName) => {
       Services.ppmm.initialProcessData.autofillSavedFieldNames.delete(fieldName);
     });
 
     Services.ppmm.broadcastAsyncMessage("FormAutofill:savedFieldNames",
                                         Services.ppmm.initialProcessData.autofillSavedFieldNames);
+    this._updateStatus();
   },
 };
rename from browser/extensions/formautofill/test/unit/test_enabledStatus.js
rename to browser/extensions/formautofill/test/unit/test_activeStatus.js
--- a/browser/extensions/formautofill/test/unit/test_enabledStatus.js
+++ b/browser/extensions/formautofill/test/unit/test_activeStatus.js
@@ -2,101 +2,86 @@
  * Test for status handling in Form Autofill Parent.
  */
 
 "use strict";
 
 Cu.import("resource://formautofill/FormAutofillParent.jsm");
 Cu.import("resource://formautofill/ProfileStorage.jsm");
 
-add_task(async function test_enabledStatus_init() {
+add_task(async function test_activeStatus_init() {
   let formAutofillParent = new FormAutofillParent();
-  sinon.spy(formAutofillParent, "_setStatus");
+  sinon.spy(formAutofillParent, "_updateStatus");
 
-  // Default status is false before initialization
-  do_check_eq(formAutofillParent._enabled, false);
+  // Default status is null before initialization
+  do_check_eq(formAutofillParent._active, null);
   do_check_eq(Services.ppmm.initialProcessData.autofillEnabled, undefined);
 
   await formAutofillParent.init();
-  do_check_eq(formAutofillParent._setStatus.called, true);
+  do_check_eq(formAutofillParent._updateStatus.called, true);
   do_check_eq(Services.ppmm.initialProcessData.autofillEnabled, false);
 
   formAutofillParent._uninit();
 });
 
-add_task(function* test_enabledStatus_observe() {
+add_task(async function test_activeStatus_observe() {
   let formAutofillParent = new FormAutofillParent();
-  sinon.stub(formAutofillParent, "_getStatus");
-  sinon.spy(formAutofillParent, "_setStatus");
-  sinon.stub(formAutofillParent, "_updateSavedFieldNames");
+  sinon.stub(formAutofillParent, "_computeStatus");
+  sinon.spy(formAutofillParent, "_onStatusChanged");
 
-  // _enabled = _getStatus() => No need to trigger onStatusChanged
-  formAutofillParent._enabled = true;
-  formAutofillParent._getStatus.returns(true);
+  // _active = _computeStatus() => No need to trigger _onStatusChanged
+  formAutofillParent._active = true;
+  formAutofillParent._computeStatus.returns(true);
   formAutofillParent.observe(null, "nsPref:changed", "extensions.formautofill.addresses.enabled");
-  do_check_eq(formAutofillParent._setStatus.called, false);
+  do_check_eq(formAutofillParent._onStatusChanged.called, false);
 
-  // _enabled != _getStatus() => Need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(false);
+  // _active != _computeStatus() => Need to trigger _onStatusChanged
+  formAutofillParent._computeStatus.returns(false);
+  formAutofillParent._onStatusChanged.reset();
   formAutofillParent.observe(null, "nsPref:changed", "extensions.formautofill.addresses.enabled");
-  do_check_eq(formAutofillParent._setStatus.called, true);
+  do_check_eq(formAutofillParent._onStatusChanged.called, true);
 
-  // profile added => Need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
-  formAutofillParent._setStatus.reset();
+  // profile added => Need to trigger _onStatusChanged
+  formAutofillParent._computeStatus.returns(!formAutofillParent._active);
+  formAutofillParent._onStatusChanged.reset();
   formAutofillParent.observe(null, "formautofill-storage-changed", "add");
-  do_check_eq(formAutofillParent._setStatus.called, true);
+  do_check_eq(formAutofillParent._onStatusChanged.called, true);
 
-  // profile removed => Need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
-  formAutofillParent._setStatus.reset();
+  // profile removed => Need to trigger _onStatusChanged
+  formAutofillParent._computeStatus.returns(!formAutofillParent._active);
+  formAutofillParent._onStatusChanged.reset();
   formAutofillParent.observe(null, "formautofill-storage-changed", "remove");
-  do_check_eq(formAutofillParent._setStatus.called, true);
+  do_check_eq(formAutofillParent._onStatusChanged.called, true);
 
-  // profile updated => no need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
-  formAutofillParent._setStatus.reset();
+  // profile updated => no need to trigger _onStatusChanged
+  formAutofillParent._computeStatus.returns(!formAutofillParent._active);
+  formAutofillParent._onStatusChanged.reset();
   formAutofillParent.observe(null, "formautofill-storage-changed", "update");
-  do_check_eq(formAutofillParent._setStatus.called, false);
+  do_check_eq(formAutofillParent._onStatusChanged.called, false);
 });
 
-add_task(function* test_enabledStatus_getStatus() {
+add_task(async function test_activeStatus_computeStatus() {
   let formAutofillParent = new FormAutofillParent();
   do_register_cleanup(function cleanup() {
     Services.prefs.clearUserPref("extensions.formautofill.addresses.enabled");
   });
 
   sinon.stub(profileStorage.addresses, "getAll");
   profileStorage.addresses.getAll.returns([]);
 
   // pref is enabled and profile is empty.
   Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", true);
-  do_check_eq(formAutofillParent._getStatus(), false);
+  do_check_eq(formAutofillParent._computeStatus(), false);
 
   // pref is disabled and profile is empty.
   Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", false);
-  do_check_eq(formAutofillParent._getStatus(), false);
+  do_check_eq(formAutofillParent._computeStatus(), false);
 
-  profileStorage.addresses.getAll.returns(["test-profile"]);
+  profileStorage.addresses.getAll.returns([{"given-name": "John"}]);
+  formAutofillParent.observe(null, "formautofill-storage-changed", "add");
   // pref is enabled and profile is not empty.
   Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", true);
-  do_check_eq(formAutofillParent._getStatus(), true);
+  do_check_eq(formAutofillParent._computeStatus(), true);
 
   // pref is disabled and profile is not empty.
   Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", false);
-  do_check_eq(formAutofillParent._getStatus(), false);
+  do_check_eq(formAutofillParent._computeStatus(), false);
 });
-
-add_task(function* test_enabledStatus_setStatus() {
-  let formAutofillParent = new FormAutofillParent();
-  sinon.spy(formAutofillParent, "_onStatusChanged");
-
-  formAutofillParent._setStatus(true);
-  do_check_eq(formAutofillParent._enabled, true);
-  do_check_eq(Services.ppmm.initialProcessData.autofillEnabled, true);
-  do_check_eq(formAutofillParent._onStatusChanged.called, true);
-
-  formAutofillParent._onStatusChanged.reset();
-  formAutofillParent._setStatus(false);
-  do_check_eq(formAutofillParent._enabled, false);
-  do_check_eq(Services.ppmm.initialProcessData.autofillEnabled, false);
-  do_check_eq(formAutofillParent._onStatusChanged.called, true);
-});
--- a/browser/extensions/formautofill/test/unit/xpcshell.ini
+++ b/browser/extensions/formautofill/test/unit/xpcshell.ini
@@ -11,21 +11,21 @@ support-files =
 [heuristics/third_party/test_HomeDepot.js]
 [heuristics/third_party/test_Macys.js]
 [heuristics/third_party/test_NewEgg.js]
 [heuristics/third_party/test_OfficeDepot.js]
 [heuristics/third_party/test_QVC.js]
 [heuristics/third_party/test_Sears.js]
 [heuristics/third_party/test_Staples.js]
 [heuristics/third_party/test_Walmart.js]
+[test_activeStatus.js]
 [test_addressRecords.js]
 [test_autofillFormFields.js]
 [test_collectFormFields.js]
 [test_creditCardRecords.js]
-[test_enabledStatus.js]
 [test_findLabelElements.js]
 [test_getFormInputDetails.js]
 [test_isCJKName.js]
 [test_markAsAutofillField.js]
 [test_nameUtils.js]
 [test_onFormSubmitted.js]
 [test_profileAutocompleteResult.js]
 [test_savedFieldNames.js]