ChromeOS: Time zone setting dropdown should be grayed out once we enable the Automatically resolve …

chrome/browser/resources/options/browser_options.js

Index: chrome/browser/resources/options/browser_options.js
diff --git a/chrome/browser/resources/options/browser_options.js b/chrome/browser/resources/options/browser_options.js
index 5ee926d1a48f50bc89b7540dcac159484c5c2853..3832b80e0c6de933c66e227a4b2762e017ecdc0f 100644
--- a/chrome/browser/resources/options/browser_options.js
+++ b/chrome/browser/resources/options/browser_options.js
@@ -109,6 +109,16 @@ cr.define('options', function() {
      */
     initializationComplete_: false,
 
+    /**
+     * Current status of "Resolve Timezone by Geolocation" checkbox.

[michaelpg, 2015/07/20 21:40:51]

Annotate: @private {boolean}

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

+     */
+    resolveTimezoneByGeolocation_: false,
+
+    /**
+     * True if system timezone is managed by policy.

[michaelpg, 2015/07/20 21:40:51]

Annotate.

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

+     */
+    systemTimezoneIsManaged_: false,
+
     /** @override */
     initializePage: function() {
       Page.prototype.initializePage.call(this);
@@ -407,9 +417,6 @@ cr.define('options', function() {
         // Timezone
         if (loadTimeData.getBoolean('enableTimeZoneTrackingOption')) {
           $('resolve-timezone-by-geolocation-selection').hidden = false;
-          this.setSystemTimezoneManaged_(false);
-          $('timezone-value-select').disabled = loadTimeData.getBoolean(
-              'resolveTimezoneByGeolocationInitialValue');
         }
       }
 
@@ -1652,14 +1659,12 @@ cr.define('options', function() {
     },
 
     /**
-     * This is called from chromium code when system timezone "managed" state
-     * is changed. Enables or disables dependent settings.
-     * @param {boolean} managed Is true when system Timezone is managed by
-     *     enterprise policy. False otherwize.
+     * This enables or disables dependent settings in timezone section.
+     * @private
      */
-    setSystemTimezoneManaged_: function(managed) {
+    updateTimezoneSectionVisibility_: function() {

[michaelpg, 2015/07/20 21:40:51]

rename: this changes enabled state, not visibility

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

       if (loadTimeData.getBoolean('enableTimeZoneTrackingOption')) {
-        if (managed) {
+        if (this.systemTimezoneIsManaged_) {

[michaelpg, 2015/07/20 21:40:51]

If this is only used here, can you make it a parameter instead of a member
variable?

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

[michaelpg, 2015/07/21 23:47:36]

Sorry, I think my logic was wrong! There are 2 variables that can change
separately, and this function needs to know both of them, so what you had made
sense.

So this function doesn't need parameters after all. You can remove them (like
the code here) or leave them in (as they are in your latest patch), I don't care
much one way or the other.

           $('resolve-timezone-by-geolocation-selection').disabled = true;
           $('resolve-timezone-by-geolocation').onclick = function(event) {};
         } else {
@@ -1669,12 +1674,34 @@ cr.define('options', function() {
             $('timezone-value-select').disabled = event.currentTarget.checked;
           };
           $('timezone-value-select').disabled =
-              $('resolve-timezone-by-geolocation').checked;
+              this.resolveTimezoneByGeolocation_;

[michaelpg, 2015/07/20 21:40:51]

Same request.

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

         }
       }
     },
 
     /**
+     * This is called from chromium code when system timezone "managed" state
+     * is changed. Enables or disables dependent settings.
+     * @param {boolean} managed Is true when system Timezone is managed by
+     *     enterprise policy. False otherwize.
+     */
+    setSystemTimezoneManaged_: function(managed) {
+      this.systemTimezoneIsManaged_ = managed;
+      this.updateTimezoneSectionVisibility_();
+    },
+
+    /**
+     * This is called from chromium code when kResolveTimezoneByGeolocation
+     * preference is changed. Enables or disables dependent settings.
+     * @param {boolean} value Is true or false when
+     *   kResolveTimezoneByGeolocation preference is true or false.

[michaelpg, 2015/07/20 21:40:51]

nit: indent 4 spaces

[Alexander Alekseev, 2015/07/21 05:34:29]

Done.

+     */
+    onResolveTimezoneByGeolocationChanged_: function(value) {

[michaelpg, 2015/07/20 21:40:51]

This is a plain preference, so you can trigger this function in JavaScript
instead of involving the C++ handler.

Preferences.getInstance().addEventListener(
    kResolveTimezoneByGeolocation,
    this.onResolveTimezoneByGeolocationChanged_.bind(this));

See preferences.js for definition and this file for examples.

[Alexander Alekseev, 2015/07/21 05:34:29]

This is what I was looking for! Thank you.

+      this.resolveTimezoneByGeolocation_ = value;
+      this.updateTimezoneSectionVisibility_();
+    },
+
+    /**
      * Handle the 'add device' button click.
      * @private
      */
@@ -2146,6 +2173,7 @@ cr.define('options', function() {
     'getStartStopSyncButton',
     'hideBluetoothSettings',
     'notifyInitializationComplete',
+    'onResolveTimezoneByGeolocationChanged',
     'removeBluetoothDevice',
     'scrollToSection',
     'setAccountPictureManaged',