MD-Settings: convert cr-settings-prefs to a singleton model

chrome/browser/resources/settings/prefs/prefs_types.js

Index: chrome/browser/resources/settings/prefs/prefs_types.js
diff --git a/chrome/browser/resources/settings/prefs/prefs_types.js b/chrome/browser/resources/settings/prefs/prefs_types.js
index 3ac53ba967be5c3193a1b6b8a7087894c368ba4d..a2eb3763e5ceaef4a72718dc336eca8690d81906 100644
--- a/chrome/browser/resources/settings/prefs/prefs_types.js
+++ b/chrome/browser/resources/settings/prefs/prefs_types.js
@@ -6,22 +6,44 @@
  * @fileoverview Types for CrSettingsPrefsElement.
  */
 
-/** @type {{INITIALIZED: string, isInitialized: boolean}} */
-var CrSettingsPrefs;
+/**
+ * Global state for prefs status.
+ */
+var CrSettingsPrefs = (function() {

[Dan Beam, 2015/09/21 22:25:21]

nit:

var CrSettingsPrefs = function() {
  var CrSettingsPrefsInternal = {
    setInitialized: function() {
      /** @public {boolean} */
      this.isIntialized = true;
      this.res_();
    },
    reset: function() {
      this.isInitialized = false;
      /** @public {!Promise} */
      this.initialized = new Promise(function(res) {
        this.res_ = res;
      }.bind(this));
    },
  };

  CrSettingsPrefsInternal.reset();

  return CrSettingsPrefsInternal;
}();

feel free to eradicate my perilous use of |this| in a static context

[michaelpg, 2015/09/22 00:34:07]

@public is not documented... does it do something different from @type?
Done. Much nicer.
I've done it both ways, patches 5 and 6, which is preferable?

[Dan Beam, 2015/09/22 06:12:31]

whatever you have currently is fine

+  var resolveInitialized;
 
-// TODO(michaelpg): provide a Promise for initialization.
-if (CrSettingsPrefs === undefined) {
-  CrSettingsPrefs = {
+  return {
     /**
-     * The name of the event fired when prefs have been fetched and initialized.
-     * @const {string}
+     * Set to true when all settings have been initialized.
+     * @type {boolean}
      */
-    INITIALIZED: 'cr-settings-prefs-initialized',
+    isInitialized: false,
 
     /**
-     * Global boolean set to true when all settings have been initialized.
-     * @type {boolean}
+     * Promise to be resolved when all settings have been initialized.
+     * @type {Promise}
      */
-    isInitialized: false,
+    initialized: Promise.resolve(),
+
+    /**
+     * Rresolves the CrSettingsPrefs.initialized promise.
+     */
+    setInitialized: function() {
+      CrSettingsPrefs.isInitialized = true;
+      resolveInitialized();
+    },
+
+    /**
+     * Called to set up the promise and resolve methods, and to restore
+     * state for testing.
+     */
+    reset: function() {
+      CrSettingsPrefs.initialized = new Promise(function(resolve) {
+        resolveInitialized = resolve;

[Dan Beam, 2015/09/21 22:25:21]

why shouldn't this be a private static rather than a local?  e.g.
CrSettingsPrefs.resolve_

[michaelpg, 2015/09/22 00:34:07]

Done. But what do you have against environment locals?

[Dan Beam, 2015/09/22 06:12:31]

harder to mock/change when testing

+      });
+      CrSettingsPrefs.isInitialized = false;
+    },
   };
-}
+})();
+
+CrSettingsPrefs.reset();

[Dan Beam, 2015/09/21 22:25:21]

maybe i'm being paranoid, but why can't we run reset() before this class is
exposed?

[michaelpg, 2015/09/22 00:34:07]

Done.