Bug 1364477 - Only getAll Address Profiles once at initialization. r=steveck
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 23 May 2017 00:47:59 -0400
changeset 360146 c4d9c5751a2f92bbf1eee0153a713cb15f245db5
parent 360145 a2c656d53aac3983b3940e35ec3a2ca8682b3d56
child 360147 8766c76d6c6ae1f9eb93d0d3e0747091b35ad4eb
push id31871
push userryanvm@gmail.com
push dateTue, 23 May 2017 22:02:07 +0000
treeherdermozilla-central@545ffce30eac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssteveck
bugs1364477
milestone55.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 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]