Rework quick unlock settings to follow new specs.

chrome/browser/resources/settings/people_page/lock_screen.js

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * @fileoverview
+ * 'settings-lock-screen' allows the user to change how they unlock their
+ * device.
+ *
+ * Example:
+ *
+ * <settings-lock-screen
+ *   set-modes="[[quickUnlockSetModes]]"

[tommycli, 2016/08/05 17:46:19]

Is this example usage still up to date? It doesn't seem like set-modes attr is
used in people_page.

[jdufault, 2016/08/05 20:59:51]

Done.

+ *   prefs="{{prefs}}">
+ * </settings-lock-screen>
+ */
+
+(function() {

[tommycli, 2016/08/05 17:46:19]

Since there's no variables here you want to keep out of the global scope, i
think this (function() { business is unnecessary. Also I don't think 'use
strict' does anything, but I might be wrong there.

[jdufault, 2016/08/05 20:59:51]

Done.

+'use strict';
+
+Polymer({
+  is: 'settings-lock-screen',
+
+  behaviors: [I18nBehavior, LockStateBehavior],
+
+  properties: {
+    /** @type {!settings.Route} */

[tommycli, 2016/08/05 17:46:19]

Since this is new code, I recommend using the new pattern:
- Removing the currentRoute property
- No longer passing the attribute in via current-route="{{currentRoute}}" during
element creation.
- Instead add settings.RouteObserverBehavior. It should call the
currentRouteChanged method you can override.
- You can access the current route via "settings.getCurrentRoute()"

[jdufault, 2016/08/05 20:59:51]

Done.

+    currentRoute: {
+      type: Object,
+      observer: 'onRouteChanged_',
+    },
+
+    /** Preferences state. */
+    prefs: {
+      type: Object
+    },
+
+    /**
+     * @type {Object|undefined}
+     * @private
+     */
+    setModes_: {

[tommycli, 2016/08/05 17:46:19]

Since this setModes_ usage is subtle, it could use a comment. Maybe something
like:

setModes_ is a partially applied function that stores the previously entered
password. It's defined only when the user has already entered a valid password.

Maybe there's a shorter way of describing it.

[jdufault, 2016/08/05 20:59:51]

Done.

+      type: Object,
+      observer: 'onSetModesChanged_'
+    },
+  },
+
+  // selectedUnlockType is defined in LockStateBehavior.

[tommycli, 2016/08/05 17:46:19]

nit: Comment should probably be like: /** Foo */

[jdufault, 2016/08/05 20:59:51]

Done.

+  observers: ['selectedUnlockTypeChanged_(selectedUnlockType)'],
+
+  /** @private */
+  onRouteChanged_: function(currentRoute) {

[tommycli, 2016/08/05 17:46:19]

If you go with the settings.RouteObserverBehavior technique, It should be:

  /** @protected */
  currentRouteChanged: function() {

[jdufault, 2016/08/05 20:59:51]

Done.

+    if (this.currentRoute == settings.Route.LOCK_SCREEN && !this.setModes_) {
+      this.$.passwordPrompt.open();
+    } else {
+      // If the user navigated away from the lock screen settings page they will
+      // have to re-enter their password.
+      this.setModes_ = undefined;
+    }
+  },
+
+  /**
+   * Called when the unlock type has changed.
+   * @param {!string} selected The current unlock type.
+   * @private
+   */
+  selectedUnlockTypeChanged_: function(selected) {
+    if (selected == LockScreenUnlockType.VALUE_PENDING)
+      return;
+
+    if (selected != LockScreenUnlockType.PIN_PASSWORD && this.setModes_) {
+      this.setModes_.call(null, [], [], function(didSet) {
+        assert(didSet, 'Failed to clear quick unlock modes');
+      });
+    }
+  },
+
+  /** @private */
+  onSetModesChanged_: function() {
+    if (this.currentRoute == settings.Route.LOCK_SCREEN && !this.setModes_) {
+      this.$.setupPin.close();
+      this.$.passwordPrompt.open();
+    }
+  },
+
+  /** @private */
+  onPasswordClosed_: function() {
+    if (!this.setModes_)
+      settings.navigateTo(settings.Route.PEOPLE);
+  },
+
+  /** @private */
+  onPinSetupDone_: function() {
+    this.$.setupPin.close();
+  },
+
+  /**
+   * Retruns true if the setup pin section should be shown.

[tommycli, 2016/08/05 17:46:19]

typo: Returns

[jdufault, 2016/08/05 20:59:51]

Done.

+   * @param {!string} selectedUnlockType The current unlock type. Used to let
+   * polymer know about the dependency.

[tommycli, 2016/08/05 17:46:19]

nit: indent four spaces, then Polymer

[jdufault, 2016/08/05 20:59:51]

Done.

+   * @private
+   */
+  showSetupPin_: function(selectedUnlockType) {

[tommycli, 2016/08/05 17:46:18]

nit: maybe rename: showConfigurePinButton_ ?

[jdufault, 2016/08/05 20:59:51]

Done.

+    return selectedUnlockType === LockScreenUnlockType.PIN_PASSWORD;
+  },
+
+  /** @private */
+  getSetupPinText_: function() {

[tommycli, 2016/08/05 17:46:19]

My interpretation of the mocks was that selecting the "Pin or Password" option
would automatically open the pin setup dialog. If that's the case, there's no
need for this fn.

[jdufault, 2016/08/05 20:59:51]

That part of the mocks is likely to change (UI review didn't like it).

+    if (this.hasPin)
+      return this.i18n('lockScreenChangePinButton');
+    return this.i18n('lockScreenSetupPinButton');
+  },
+
+  /** @private */
+  onConfigurePin_: function() {
+    this.$.setupPin.open();
+  },
+});
+
+})();