Bug 1361965 - Provide access to a formautofill storage singleton. r=lchang draft
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 04 May 2017 18:25:46 +1000
changeset 575204 02f025cd9d91a0b5be7bbee095de572097cfccb8
parent 575133 120d8562d4a53e4f78bd86c6f5076f6db265e5a3
child 627861 ab096ff22f975a1f2f704f110a0b2edcff56578a
push id57996
push userbmo:markh@mozilla.com
push dateWed, 10 May 2017 04:30:59 +0000
reviewerslchang
bugs1361965
milestone55.0a1
Bug 1361965 - Provide access to a formautofill storage singleton. r=lchang MozReview-Commit-ID: 6IbeuOmONxb
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/bootstrap.js
browser/extensions/formautofill/test/unit/test_enabledStatus.js
browser/extensions/formautofill/test/unit/test_profileStorage.js
browser/extensions/formautofill/test/unit/test_savedFieldNames.js
browser/extensions/formautofill/test/unit/test_transformFields.js
tools/lint/eslint/modules.json
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -34,51 +34,44 @@ this.EXPORTED_SYMBOLS = ["FormAutofillPa
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
 Cu.import("resource://formautofill/FormAutofillUtils.jsm");
 
-XPCOMUtils.defineLazyModuleGetter(this, "OS",
-                                  "resource://gre/modules/osfile.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "ProfileStorage",
+XPCOMUtils.defineLazyModuleGetter(this, "profileStorage",
                                   "resource://formautofill/ProfileStorage.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FormAutofillPreferences",
                                   "resource://formautofill/FormAutofillPreferences.jsm");
 
 this.log = null;
 FormAutofillUtils.defineLazyLogGetter(this, this.EXPORTED_SYMBOLS[0]);
 
-const PROFILE_JSON_FILE_NAME = "autofill-profiles.json";
 const ENABLED_PREF = "browser.formautofill.enabled";
 
 function FormAutofillParent() {
 }
 
 FormAutofillParent.prototype = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports, Ci.nsIObserver]),
 
-  _profileStore: null,
-
   /**
    * Whether Form Autofill is enabled in preferences.
    * Caches the latest value of this._getStatus().
    */
   _enabled: false,
 
   /**
    * Initializes ProfileStorage and registers the message handler.
    */
-  init() {
+  async init() {
     log.debug("init");
-    let storePath = OS.Path.join(OS.Constants.Path.profileDir, PROFILE_JSON_FILE_NAME);
-    this._profileStore = new ProfileStorage(storePath);
-    this._profileStore.initialize();
+    await profileStorage.initialize();
 
     Services.obs.addObserver(this, "advanced-pane-loaded");
     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);
@@ -155,17 +148,17 @@ FormAutofillParent.prototype = {
    *
    * @returns {boolean} status of form autofill feature
    */
   _getStatus() {
     if (!Services.prefs.getBoolPref(ENABLED_PREF)) {
       return false;
     }
 
-    return this._profileStore.getAll().length > 0;
+    return profileStorage.getAll().length > 0;
   },
 
   /**
    * Set status and trigger _onStatusChanged.
    *
    * @param {boolean} newStatus The latest status we want to set for _enabled
    */
   _setStatus(newStatus) {
@@ -183,50 +176,36 @@ FormAutofillParent.prototype = {
   receiveMessage({name, data, target}) {
     switch (name) {
       case "FormAutofill:GetAddresses": {
         this._getAddresses(data, target);
         break;
       }
       case "FormAutofill:SaveAddress": {
         if (data.guid) {
-          this.getProfileStore().update(data.guid, data.address);
+          profileStorage.update(data.guid, data.address);
         } else {
-          this.getProfileStore().add(data.address);
+          profileStorage.add(data.address);
         }
         break;
       }
       case "FormAutofill:RemoveAddresses": {
-        data.guids.forEach(guid => this.getProfileStore().remove(guid));
+        data.guids.forEach(guid => profileStorage.remove(guid));
         break;
       }
     }
   },
 
   /**
-   * Returns the instance of ProfileStorage. To avoid syncing issues, anyone
-   * who needs to access the profile should request the instance by this instead
-   * of creating a new one.
-   *
-   * @returns {ProfileStorage}
-   */
-  getProfileStore() {
-    return this._profileStore;
-  },
-
-  /**
    * Uninitializes FormAutofillParent. This is for testing only.
    *
    * @private
    */
   _uninit() {
-    if (this._profileStore) {
-      this._profileStore._saveImmediately();
-      this._profileStore = null;
-    }
+    profileStorage._saveImmediately();
 
     Services.ppmm.removeMessageListener("FormAutofill:GetAddresses", this);
     Services.ppmm.removeMessageListener("FormAutofill:SaveAddress", this);
     Services.ppmm.removeMessageListener("FormAutofill:RemoveAddresses", this);
     Services.obs.removeObserver(this, "advanced-pane-loaded");
     Services.prefs.removeObserver(ENABLED_PREF, this);
   },
 
@@ -241,41 +220,41 @@ FormAutofillParent.prototype = {
    *         The input autocomplete property's information.
    * @param  {nsIFrameMessageManager} target
    *         Content's message manager.
    */
   _getAddresses({searchString, info}, target) {
     let addresses = [];
 
     if (info && info.fieldName) {
-      addresses = this._profileStore.getByFilter({searchString, info});
+      addresses = profileStorage.getByFilter({searchString, info});
     } else {
-      addresses = this._profileStore.getAll();
+      addresses = profileStorage.getAll();
     }
 
     target.sendAsyncMessage("FormAutofill:Addresses", addresses);
   },
 
   _updateSavedFieldNames() {
     if (!Services.ppmm.initialProcessData.autofillSavedFieldNames) {
       Services.ppmm.initialProcessData.autofillSavedFieldNames = new Set();
     } else {
       Services.ppmm.initialProcessData.autofillSavedFieldNames.clear();
     }
 
-    this._profileStore.getAll().forEach((address) => {
+    profileStorage.getAll().forEach((address) => {
       Object.keys(address).forEach((fieldName) => {
         if (!address[fieldName]) {
           return;
         }
         Services.ppmm.initialProcessData.autofillSavedFieldNames.add(fieldName);
       });
     });
 
     // Remove the internal guid and metadata fields.
-    this._profileStore.INTERNAL_FIELDS.forEach((fieldName) => {
+    profileStorage.INTERNAL_FIELDS.forEach((fieldName) => {
       Services.ppmm.initialProcessData.autofillSavedFieldNames.delete(fieldName);
     });
 
     Services.ppmm.broadcastAsyncMessage("FormAutofill:savedFieldNames",
                                         Services.ppmm.initialProcessData.autofillSavedFieldNames);
   },
 };
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -45,38 +45,42 @@
  *       // ...
  *     }
  *   ]
  * }
  */
 
 "use strict";
 
-this.EXPORTED_SYMBOLS = ["ProfileStorage"];
+// We expose a singleton from this module. Some tests may import the
+// constructor via a backstage pass.
+this.EXPORTED_SYMBOLS = ["profileStorage"];
 
 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/Task.jsm");
+Cu.import("resource://gre/modules/osfile.jsm");
 
 Cu.import("resource://formautofill/FormAutofillUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "JSONFile",
                                   "resource://gre/modules/JSONFile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FormAutofillNameUtils",
                                   "resource://formautofill/FormAutofillNameUtils.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "gUUIDGenerator",
                                    "@mozilla.org/uuid-generator;1",
                                    "nsIUUIDGenerator");
 
 this.log = null;
 FormAutofillUtils.defineLazyLogGetter(this, this.EXPORTED_SYMBOLS[0]);
 
+const PROFILE_JSON_FILE_NAME = "autofill-profiles.json";
+
 const SCHEMA_VERSION = 1;
 
 const VALID_FIELDS = [
   "given-name",
   "additional-name",
   "family-name",
   "organization",
   "street-address",
@@ -85,35 +89,39 @@ const VALID_FIELDS = [
   "postal-code",
   "country",
   "tel",
   "email",
 ];
 
 function ProfileStorage(path) {
   this._path = path;
+  this._initializePromise = null;
 }
 
 ProfileStorage.prototype = {
   // These fields are defined internally for each record.
   INTERNAL_FIELDS:
     ["guid", "timeCreated", "timeLastUsed", "timeLastModified", "timesUsed"],
   /**
    * Loads the profile data from file to memory.
    *
    * @returns {Promise}
    * @resolves When the operation finished successfully.
    * @rejects  JavaScript exception.
    */
   initialize() {
-    this._store = new JSONFile({
-      path: this._path,
-      dataPostProcessor: this._dataPostProcessor.bind(this),
-    });
-    return this._store.load();
+    if (!this._initializePromise) {
+      this._store = new JSONFile({
+        path: this._path,
+        dataPostProcessor: this._dataPostProcessor.bind(this),
+      });
+      this._initializePromise = this._store.load();
+    }
+    return this._initializePromise;
   },
 
   /**
    * Adds a new address.
    *
    * @param {Address} address
    *        The new address for saving.
    */
@@ -373,8 +381,12 @@ ProfileStorage.prototype = {
     return data;
   },
 
   // For test only.
   _saveImmediately() {
     return this._store._save();
   },
 };
+
+// The singleton exposed by this module.
+this.profileStorage = new ProfileStorage(
+  OS.Path.join(OS.Constants.Path.profileDir, PROFILE_JSON_FILE_NAME));
--- a/browser/extensions/formautofill/bootstrap.js
+++ b/browser/extensions/formautofill/bootstrap.js
@@ -54,17 +54,17 @@ function startup() {
     let win = enumerator.getNext();
     let domWindow = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindow);
 
     insertStyleSheet(domWindow, STYLESHEET_URI);
   }
 
   Services.wm.addListener(windowListener);
 
-  parent.init();
+  parent.init().catch(Cu.reportError);
   Services.ppmm.loadProcessScript("data:,new " + function() {
     Components.utils.import("resource://formautofill/FormAutofillContent.jsm");
   }, true);
   Services.mm.loadFrameScript("chrome://formautofill/content/FormAutofillFrameScript.js", true);
 }
 
 function shutdown() {
   Services.wm.removeListener(windowListener);
--- a/browser/extensions/formautofill/test/unit/test_enabledStatus.js
+++ b/browser/extensions/formautofill/test/unit/test_enabledStatus.js
@@ -1,25 +1,26 @@
 /*
  * Test for status handling in Form Autofill Parent.
  */
 
 "use strict";
 
 Cu.import("resource://formautofill/FormAutofillParent.jsm");
+Cu.import("resource://formautofill/ProfileStorage.jsm");
 
-add_task(function* test_enabledStatus_init() {
+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);
 
-  formAutofillParent.init();
+  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();
@@ -58,30 +59,28 @@ add_task(function* test_enabledStatus_ob
 });
 
 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,
-  };
+  sinon.stub(profileStorage, "getAll");
+  profileStorage.getAll.returns([]);
 
   // 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"];
+  profileStorage.getAll.returns(["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
@@ -1,16 +1,16 @@
 /**
  * Tests ProfileStorage object.
  */
 
 "use strict";
 
 Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://formautofill/ProfileStorage.jsm");
+const {ProfileStorage} = Cu.import("resource://formautofill/ProfileStorage.jsm", {});
 
 const TEST_STORE_FILE_NAME = "test-profile.json";
 
 const TEST_ADDRESS_1 = {
   "given-name": "Timothy",
   "additional-name": "John",
   "family-name": "Berners-Lee",
   organization: "World Wide Web Consortium",
--- a/browser/extensions/formautofill/test/unit/test_savedFieldNames.js
+++ b/browser/extensions/formautofill/test/unit/test_savedFieldNames.js
@@ -2,56 +2,56 @@
  * Test for keeping the valid fields information in initialProcessData.
  */
 
 "use strict";
 
 Cu.import("resource://formautofill/FormAutofillParent.jsm");
 Cu.import("resource://formautofill/ProfileStorage.jsm");
 
-add_task(function* test_profileSavedFieldNames_init() {
+add_task(async function test_profileSavedFieldNames_init() {
   let formAutofillParent = new FormAutofillParent();
   sinon.stub(formAutofillParent, "_updateSavedFieldNames");
 
-  formAutofillParent.init();
+  await formAutofillParent.init();
   do_check_eq(formAutofillParent._updateSavedFieldNames.called, true);
 
   formAutofillParent._uninit();
 });
 
-add_task(function* test_profileSavedFieldNames_observe() {
+add_task(async function test_profileSavedFieldNames_observe() {
   let formAutofillParent = new FormAutofillParent();
   sinon.stub(formAutofillParent, "_updateSavedFieldNames");
 
-  formAutofillParent.init();
+  await formAutofillParent.init();
 
   // profile added => Need to trigger updateValidFields
   formAutofillParent.observe(null, "formautofill-storage-changed", "add");
   do_check_eq(formAutofillParent._updateSavedFieldNames.called, true);
 
   // profile removed => Need to trigger updateValidFields
   formAutofillParent._updateSavedFieldNames.reset();
   formAutofillParent.observe(null, "formautofill-storage-changed", "remove");
   do_check_eq(formAutofillParent._updateSavedFieldNames.called, true);
 
   // profile updated => no need to trigger updateValidFields
   formAutofillParent._updateSavedFieldNames.reset();
   formAutofillParent.observe(null, "formautofill-storage-changed", "update");
   do_check_eq(formAutofillParent._updateSavedFieldNames.called, false);
 });
 
-add_task(function* test_profileSavedFieldNames_update() {
+add_task(async function test_profileSavedFieldNames_update() {
   let formAutofillParent = new FormAutofillParent();
-  formAutofillParent.init();
+  await formAutofillParent.init();
   do_register_cleanup(function cleanup() {
     Services.prefs.clearUserPref("browser.formautofill.enabled");
   });
 
-  sinon.stub(formAutofillParent._profileStore, "getAll");
-  formAutofillParent._profileStore.getAll.returns([]);
+  sinon.stub(profileStorage, "getAll");
+  profileStorage.getAll.returns([]);
 
   // The set is empty if there's no profile in the store.
   formAutofillParent._updateSavedFieldNames();
   do_check_eq(Services.ppmm.initialProcessData.autofillSavedFieldNames.size, 0);
 
   // 2 profiles with 4 valid fields.
   let fakeStorage = [{
     guid: "test-guid-1",
@@ -69,17 +69,17 @@ add_task(function* test_profileSavedFiel
     "street-address": "331 E. Evelyn Avenue",
     tel: "1-650-903-0800",
     country: "US",
     timeCreated: 0,
     timeLastUsed: 0,
     timeLastModified: 0,
     timesUsed: 0,
   }];
-  formAutofillParent._profileStore.getAll.returns(fakeStorage);
+  profileStorage.getAll.returns(fakeStorage);
   formAutofillParent._updateSavedFieldNames();
 
   let autofillSavedFieldNames = Services.ppmm.initialProcessData.autofillSavedFieldNames;
   do_check_eq(autofillSavedFieldNames.size, 4);
   do_check_eq(autofillSavedFieldNames.has("organization"), true);
   do_check_eq(autofillSavedFieldNames.has("street-address"), true);
   do_check_eq(autofillSavedFieldNames.has("tel"), true);
   do_check_eq(autofillSavedFieldNames.has("email"), false);
--- a/browser/extensions/formautofill/test/unit/test_transformFields.js
+++ b/browser/extensions/formautofill/test/unit/test_transformFields.js
@@ -1,16 +1,16 @@
 /**
  * Tests the transform algorithm in profileStorage.
  */
 
 "use strict";
 
 Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://formautofill/ProfileStorage.jsm");
+const {ProfileStorage} = Cu.import("resource://formautofill/ProfileStorage.jsm", {});
 
 const TEST_STORE_FILE_NAME = "test-profile.json";
 
 const COMPUTE_TESTCASES = [
   // Empty
   {
     description: "Empty address",
     address: {
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -161,16 +161,17 @@
   "PhoneNumberMetaData.jsm": ["PHONE_NUMBER_META_DATA"],
   "PlacesUtils.jsm": ["PlacesUtils", "PlacesAggregatedTransaction", "PlacesCreateFolderTransaction", "PlacesCreateBookmarkTransaction", "PlacesCreateSeparatorTransaction", "PlacesCreateLivemarkTransaction", "PlacesMoveItemTransaction", "PlacesRemoveItemTransaction", "PlacesEditItemTitleTransaction", "PlacesEditBookmarkURITransaction", "PlacesEditLivemarkFeedURITransaction", "PlacesEditLivemarkSiteURITransaction", "PlacesSetItemAnnotationTransaction", "PlacesSetPageAnnotationTransaction", "PlacesEditBookmarkKeywordTransaction", "PlacesEditBookmarkPostDataTransaction", "PlacesEditItemDateAddedTransaction", "PlacesEditItemLastModifiedTransaction", "PlacesSortFolderByNameTransaction", "PlacesTagURITransaction", "PlacesUntagURITransaction"],
   "PluginProvider.jsm": [],
   "PointerAdapter.jsm": ["PointerRelay", "PointerAdapter"],
   "policies.js": ["ErrorHandler", "SyncScheduler"],
   "prefs.js": ["PrefsEngine", "PrefRec"],
   "prefs.jsm": ["Preference"],
   "PresentationDeviceInfoManager.jsm": ["PresentationDeviceInfoService"],
+  "ProfileStorage.jsm": ["profileStorage"],
   "PromiseWorker.jsm": ["BasePromiseWorker"],
   "PushCrypto.jsm": ["PushCrypto", "concatArray"],
   "quit.js": ["goQuitApplication"],
   "Readability.js": ["Readability"],
   "record.js": ["WBORecord", "RecordManager", "CryptoWrapper", "CollectionKeyManager", "Collection"],
   "recursive_importA.jsm": ["foo", "bar"],
   "recursive_importB.jsm": ["baz", "qux"],
   "reflect.jsm": ["Reflect"],