Add settings for keyboard auto-repeat to options page.

chrome/browser/resources/options/chromeos/keyboard_overlay.js

Index: chrome/browser/resources/options/chromeos/keyboard_overlay.js
diff --git a/chrome/browser/resources/options/chromeos/keyboard_overlay.js b/chrome/browser/resources/options/chromeos/keyboard_overlay.js
index 4514adaff8dd3b886a4048180682f8cce62f70f0..e7b09a0c9f5a04198c3c9ee3446729207b95d44e 100644
--- a/chrome/browser/resources/options/chromeos/keyboard_overlay.js
+++ b/chrome/browser/resources/options/chromeos/keyboard_overlay.js
@@ -5,6 +5,50 @@
 cr.define('options', function() {
 
   /**
+   * The preference is a boolean.
+   * @type {string}
+   * @const
+   */
+  var AUTO_REPEAT_ENABLED_PREF = 'settings.language.xkb_auto_repeat_enabled_r2';
+
+  /**
+   * The preference is an integer number of milliseconds to delay before auto-
+   * repeating begins.
+   * @type {string}
+   * @const
+   */
+  var AUTO_REPEAT_DELAY_PREF = 'settings.language.xkb_auto_repeat_delay_r2';
+
+  /**
+   * The preference is the integer number of milliseconds between each repeat of
+   * an auto-repeating key.
+   * @type {string}
+   * @const
+   */
+  var AUTO_REPEAT_INTERVAL_PREF =
+      'settings.language.xkb_auto_repeat_interval_r2';
+
+  /**
+   * Auto-repeat delays (in ms) for the corresponding slider values, from
+   * long to short. The values were chosen to provide a large range while giving
+   * several options near the defaults.
+   * @type {Array}

[Dan Beam, 2014/07/16 16:59:46]

!Array.<number>

[michaelpg, 2014/07/18 21:33:06]

Done.

+   * @const
+   */
+  var kAutoRepeatDelays =

[Dan Beam, 2014/07/16 16:59:46]

kAutoRepeatDelays => AUTO_REPEAT_DELAYS

[michaelpg, 2014/07/18 21:33:05]

Done.

+      [2000, 1500, 1000, 500, 300, 200, 150];
+
+  /**
+   * Auto-repeat intervals (in ms) for the corresponding slider values, from
+   * long to short. The slider itself is labeled "rate", the inverse of
+   * interval, and goes from slow (long interval) to fast (short interval).
+   * @type {Array}

[Dan Beam, 2014/07/16 16:59:46]

!Array.<number>

[michaelpg, 2014/07/18 21:33:06]

Done.

+   * @const
+   */
+  var kAutoRepeatIntervals =

[Dan Beam, 2014/07/16 16:59:45]

AUTO_REPEAT_INTERVALS

[michaelpg, 2014/07/18 21:33:06]

Done.

+      [2000, 1000, 500, 300, 200, 100, 50, 30, 20];
+
+  /**
    * Encapsulated handling of the keyboard overlay.
    * @constructor
    */
@@ -21,11 +65,36 @@ cr.define('options', function() {
     __proto__: options.SettingsDialog.prototype,
 
     /**
+     * Keeps track of whether the user has changed the auto-repeat delay slider.
+     * Only update the pref if the slider is dirty, so we don't override a value
+     * that the slider doesn't support.
+     * @type {boolean}
+     * @private
+     */
+    autoRepeatDelayChanged_: false,
+
+    /**
+     * Keeps track of whether the user has changed the auto-repeat interval
+     * slider. See |autoRepeatDelayChanged_|.
+     * @type {boolean}
+     * @private
+     */
+    autoRepeatIntervalChanged_: false,
+
+    /**
      * Initializes the page. This method is called in initialize.
      */
     initializePage: function() {
       options.SettingsDialog.prototype.initializePage.call(this);
 
+      $('auto-repeat-delay-range').max = kAutoRepeatDelays.length - 1;
+      $('auto-repeat-interval-range').max = kAutoRepeatIntervals.length - 1;
+
+      $('auto-repeat-delay-range').onchange =
+          this.handleAutoRepeatDelaySliderChange_.bind(this);
+      $('auto-repeat-interval-range').onchange =
+          this.handleAutoRepeatIntervalSliderChange_.bind(this);
+
       $('languages-and-input-settings').onclick = function(e) {
         OptionsPage.navigateToPage('languages');
         chrome.send('coreOptionsUserMetricsAction',
@@ -37,6 +106,98 @@ cr.define('options', function() {
         chrome.send('coreOptionsUserMetricsAction',
                     ['Options_KeyboardShowKeyboardShortcuts']);
       };
+
+      Preferences.getInstance().addEventListener(
+          AUTO_REPEAT_ENABLED_PREF,
+          this.handleAutoRepeatEnabledPrefChange_.bind(this));
+      Preferences.getInstance().addEventListener(
+          AUTO_REPEAT_DELAY_PREF,
+          this.handleAutoRepeatDelayPrefChange_.bind(this));
+      Preferences.getInstance().addEventListener(
+          AUTO_REPEAT_INTERVAL_PREF,
+          this.handleAutoRepeatIntervalPrefChange_.bind(this));
+    },
+
+    /**
+     * Saves the dialog prefs and sets the auto-repeat prefs.
+     * @override
+     */
+    handleConfirm: function() {
+      options.SettingsDialog.prototype.handleConfirm.call(this);

[Dan Beam, 2014/07/16 16:59:45]

nit: \n

[michaelpg, 2014/07/18 21:33:06]

Removed code.

+      if (!$('enable-auto-repeat').checked)
+        return;

[Dan Beam, 2014/07/16 16:59:45]

nit: \n

[michaelpg, 2014/07/18 21:33:06]

Removed code.

+      if (this.autoRepeatDelayChanged_) {

[Dan Beam, 2014/07/16 16:59:45]

can we just remove this.*Changed_ and send the prefs every time?

[michaelpg, 2014/07/18 21:33:05]

No, but now we're using PrefRange which works the same way -- the preference is
only committed if the input is changed. For clarity, my answer to the original
question is below:

-------------

In normal use, the prefs starts out with the default values (which are valid
options for these sliders), and can only be changed using these sliders, so it's
safe to always update the preference.

On the other hand, there are circumstances where this doesn't hold. In Dev mode,
I can manually set a different repeat delay/interval that doesn't correspond to
one of the slider's notches. Coming here to change function key settings would
then also force a change of the auto-repeat settings, coercing them to whichever
value in the const array is closest.

Similarly, if we decide to change these values or introduce a new default, a
user's preference -- and what options are valid here -- could be out of sync
across multiple versions of Chrome. Again it makes sense to me that the values
represented by the sliders should only be forced if the user actually touches
the slider.

Neither of those are compelling arguments for adding these *Changed_ flags, but
if I get used to a particular repeat rate I wouldn't want it to change on me
without my input/knowledge.

+        // Use the slider's position as the index in the array of delay values.
+        Preferences.setIntegerPref(
+          AUTO_REPEAT_DELAY_PREF,
+          kAutoRepeatDelays[$('auto-repeat-delay-range').value],
+          true,
+          'Options_KeyboardAutoRepeat_Delay');
+      }

[Dan Beam, 2014/07/16 16:59:46]

nit: \n

[michaelpg, 2014/07/18 21:33:05]

Removed code.

+      if (this.autoRepeatIntervalChanged_) {
+        Preferences.setIntegerPref(
+          AUTO_REPEAT_INTERVAL_PREF,
+          kAutoRepeatIntervals[$('auto-repeat-interval-range').value],
+          true,
+          'Options_KeyboardAutoRepeat_Interval');
+      }
+    },
+
+    /**
+     * Handles auto-repeat enabled pref change.
+     * @param {Event} e Change event.
+     * @private
+     */
+    handleAutoRepeatEnabledPrefChange_: function(e) {
+      var section = $('auto-repeat-settings-section');
+      if (e.value.value)
+        section.classList.remove('disabled');
+      else
+        section.classList.add('disabled');

[Dan Beam, 2014/07/16 16:59:45]

section.classList.toggle('disabled', e.value.value);

[michaelpg, 2014/07/18 21:33:06]

Done.

+
+      $('auto-repeat-delay-range').disabled =
+          $('auto-repeat-interval-range').disabled = !e.value.value;
+    },
+
+    /**
+     * Handles auto-repeat delay pref change.
+     * @param {Event} e Change event.
+     * @private
+     */
+    handleAutoRepeatDelayPrefChange_: function(e) {
+      var index = this.indexOfClosestValue_(e.value.value, kAutoRepeatDelays);
+      if (index != -1)

[Dan Beam, 2014/07/16 16:59:46]

can this be an assert() instead?

[michaelpg, 2014/07/18 21:33:05]

Done.

+        $('auto-repeat-delay-range').value = index;
+    },
+
+    /**
+     * Handles auto-repeat interval pref change.
+     * @param {Event} e Change event.
+     * @private
+     */
+    handleAutoRepeatIntervalPrefChange_: function(e) {
+      var index =
+          this.indexOfClosestValue_(e.value.value, kAutoRepeatIntervals);
+      if (index != -1)

[Dan Beam, 2014/07/16 16:59:46]

same

[michaelpg, 2014/07/18 21:33:06]

Done.

+        $('auto-repeat-interval-range').value = index;
+    },
+
+    /**
+     * Handles the auto-repeat delay slider's change event.
+     * @param {Event} e Change event.
+     * @private
+     */
+    handleAutoRepeatDelaySliderChange_: function(e) {
+      this.autoRepeatDelayChanged_ = true;
+    },
+
+    /**
+     * Handles the auto-repeat interval slider's change event.
+     * @param {Event} e Change event.
+     * @private
+     */
+    handleAutoRepeatIntervalSliderChange_: function(e) {
+      this.autoRepeatIntervalChanged_ = true;
     },
 
     /**
@@ -54,6 +215,29 @@ cr.define('options', function() {
     showDiamondKeyOptions_: function(show) {
       $('diamond-key-remapping-section').hidden = !show;
     },
+
+    /**
+     * Finds the closest value to |target| in |arr| and returns its index.
+     * @param {number} target The value to find.

[Dan Beam, 2014/07/16 16:59:45]

nit: s/target/value

[michaelpg, 2014/07/18 21:33:05]

Done.

+     * @param {Array} arr The array to search.

[Dan Beam, 2014/07/16 16:59:45]

nit: s/arr/values

[Dan Beam, 2014/07/16 16:59:45]

!Array.<number>

[michaelpg, 2014/07/18 21:33:06]

Done and done.

+     * @return {number} The index of the closest value, or -1 if |arr| has no
+     *     (finite) values.
+     * @private
+     */
+    indexOfClosestValue_: function(target, arr) {
+      var index = arr.indexOf(target);
+      if (index != -1)
+        return index;
+      var closestValue = Infinity;
+      for (var i = 0; i < arr.length; i++) {
+        if (Math.abs(arr[i] - target) <
+            Math.abs(closestValue - target)) {
+          closestValue = arr[i];
+          index = i;
+        }
+      }
+      return index;
+    },
   };
 
   // Forward public APIs to private implementations.