Bug 1364477 - Only getAll Address Profiles once at initialization. r=steveck

MozReview-Commit-ID: Hw1twbznDsY

--HG--
rename : browser/extensions/formautofill/test/unit/test_enabledStatus.js => browser/extensions/formautofill/test/unit/test_activeStatus.js
extra : rebase_source : 098db1d42c1811ef27302725f7ab7f528dcf7bc3

browser/extensions/formautofill/FormAutofillParent.jsm

@@ -56,10 +56,9 @@ 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.
@@ -77,9 +76,7 @@ FormAutofillParent.prototype = {
     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();
   },
 
@@ -103,11 +100,8 @@ FormAutofillParent.prototype = {
       }
 
       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;
       }
 
@@ -118,10 +112,6 @@ FormAutofillParent.prototype = {
         }
 
         this._updateSavedFieldNames();
-        let currentStatus = this._getStatus();
-        if (currentStatus !== this._enabled) {
-          this._setStatus(currentStatus);
-        }
         break;
       }
 
@@ -135,35 +125,36 @@ FormAutofillParent.prototype = {
    * 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();
+    }
   },
 
   /**
@@ -234,6 +225,7 @@ FormAutofillParent.prototype = {
   },
 
   _updateSavedFieldNames() {
+    log.debug("_updateSavedFieldNames");
     if (!Services.ppmm.initialProcessData.autofillSavedFieldNames) {
       Services.ppmm.initialProcessData.autofillSavedFieldNames = new Set();
     } else {
@@ -256,5 +248,6 @@ FormAutofillParent.prototype = {
 
     Services.ppmm.broadcastAsyncMessage("FormAutofill:savedFieldNames",
                                         Services.ppmm.initialProcessData.autofillSavedFieldNames);
+    this._updateStatus();
   },
 };

browser/extensions/formautofill/test/unit/test_activeStatus.js

@@ -0,0 +1,87 @@
+/*
+ * 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_activeStatus_init() {
+  let formAutofillParent = new FormAutofillParent();
+  sinon.spy(formAutofillParent, "_updateStatus");
+
+  // 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._updateStatus.called, true);
+  do_check_eq(Services.ppmm.initialProcessData.autofillEnabled, false);
+
+  formAutofillParent._uninit();
+});
+
+add_task(async function test_activeStatus_observe() {
+  let formAutofillParent = new FormAutofillParent();
+  sinon.stub(formAutofillParent, "_computeStatus");
+  sinon.spy(formAutofillParent, "_onStatusChanged");
+
+  // _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._onStatusChanged.called, 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._onStatusChanged.called, true);
+
+  // 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._onStatusChanged.called, true);
+
+  // 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._onStatusChanged.called, true);
+
+  // 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._onStatusChanged.called, false);
+});
+
+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._computeStatus(), false);
+
+  // pref is disabled and profile is empty.
+  Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", false);
+  do_check_eq(formAutofillParent._computeStatus(), false);
+
+  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._computeStatus(), true);
+
+  // pref is disabled and profile is not empty.
+  Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", false);
+  do_check_eq(formAutofillParent._computeStatus(), false);
+});

browser/extensions/formautofill/test/unit/test_enabledStatus.js

@@ -1,102 +0,0 @@
-/*
- * 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() {
-  let formAutofillParent = new FormAutofillParent();
-  sinon.spy(formAutofillParent, "_setStatus");
-
-  // Default status is false before initialization
-  do_check_eq(formAutofillParent._enabled, false);
-  do_check_eq(Services.ppmm.initialProcessData.autofillEnabled, undefined);
-
-  await formAutofillParent.init();
-  do_check_eq(formAutofillParent._setStatus.called, true);
-  do_check_eq(Services.ppmm.initialProcessData.autofillEnabled, false);
-
-  formAutofillParent._uninit();
-});
-
-add_task(function* test_enabledStatus_observe() {
-  let formAutofillParent = new FormAutofillParent();
-  sinon.stub(formAutofillParent, "_getStatus");
-  sinon.spy(formAutofillParent, "_setStatus");
-  sinon.stub(formAutofillParent, "_updateSavedFieldNames");
-
-  // _enabled = _getStatus() => No need to trigger onStatusChanged
-  formAutofillParent._enabled = true;
-  formAutofillParent._getStatus.returns(true);
-  formAutofillParent.observe(null, "nsPref:changed", "extensions.formautofill.addresses.enabled");
-  do_check_eq(formAutofillParent._setStatus.called, false);
-
-  // _enabled != _getStatus() => Need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(false);
-  formAutofillParent.observe(null, "nsPref:changed", "extensions.formautofill.addresses.enabled");
-  do_check_eq(formAutofillParent._setStatus.called, true);
-
-  // profile added => Need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
-  formAutofillParent._setStatus.reset();
-  formAutofillParent.observe(null, "formautofill-storage-changed", "add");
-  do_check_eq(formAutofillParent._setStatus.called, true);
-
-  // profile removed => Need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
-  formAutofillParent._setStatus.reset();
-  formAutofillParent.observe(null, "formautofill-storage-changed", "remove");
-  do_check_eq(formAutofillParent._setStatus.called, true);
-
-  // profile updated => no need to trigger onStatusChanged
-  formAutofillParent._getStatus.returns(!formAutofillParent._enabled);
-  formAutofillParent._setStatus.reset();
-  formAutofillParent.observe(null, "formautofill-storage-changed", "update");
-  do_check_eq(formAutofillParent._setStatus.called, false);
-});
-
-add_task(function* test_enabledStatus_getStatus() {
-  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);
-
-  // pref is disabled and profile is empty.
-  Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", false);
-  do_check_eq(formAutofillParent._getStatus(), false);
-
-  profileStorage.addresses.getAll.returns(["test-profile"]);
-  // pref is enabled and profile is not empty.
-  Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", true);
-  do_check_eq(formAutofillParent._getStatus(), true);
-
-  // pref is disabled and profile is not empty.
-  Services.prefs.setBoolPref("extensions.formautofill.addresses.enabled", false);
-  do_check_eq(formAutofillParent._getStatus(), 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);
-});

browser/extensions/formautofill/test/unit/xpcshell.ini

@@ -16,11 +16,11 @@ support-files =
 [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]