Support Managed NetworkState format dictionaries for controlled settings.

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

Index: chrome/browser/resources/options/chromeos/internet_detail.js
diff --git a/chrome/browser/resources/options/chromeos/internet_detail.js b/chrome/browser/resources/options/chromeos/internet_detail.js
index 0803e1168156c69978d4b2ffee005eac8d085920..dcc28d81cecbca6446a93e718111022f219a89e2 100644
--- a/chrome/browser/resources/options/chromeos/internet_detail.js
+++ b/chrome/browser/resources/options/chromeos/internet_detail.js
@@ -15,7 +15,42 @@ cr.define('options.internet', function() {
   /** @const */ var IPAddressField = options.internet.IPAddressField;
 
   /**
-  /*
+   * Helper function to get a property from a dictionary that includes
+   * ONC Managed Property sub-dictionary, e.g. getActiveValue(data, 'Name').

[pneubeck (no reviews), 2014/07/30 20:12:45]

How about:
 Helper function to read the 'active' value of a property from an ONC with
managed information, e.g. getActive(network.WiFi, 'AutoConnect') .

[stevenjb, 2014/07/30 21:57:48]

Done.

+   * @param {object|string} property The property which may be a dictionary
+   *   object or a plain data type.
+   * @return {*} the property value or undefined.
+   */
+  function getActiveValue(data, key) {

[pneubeck (no reviews), 2014/07/30 20:12:46]

The @param section doesn't mention data or key, but property.
Seems that you originally had   getActiveValue(property).
Why is the single argument not sufficient? Seems that it could provide exactly
the same functionality because you never check anything on the |data| argument.

[stevenjb, 2014/07/30 21:57:47]

I went with this format because I wanted to keep open the possibility of adding
ONC validation at some point, e.g. verifying that key is an actual ONC property.

Fixed the @param comments.

+    if (!(key in data))
+      return undefined;
+    var property = data[key];
+    if (typeof property != 'object')
+      return property;
+    if ('Active' in property)
+      return property['Active'];
+    if ('Effective' in property) {

[pneubeck (no reviews), 2014/07/30 20:12:45]

i would drop this block.

active should be always set, no need to look at the effective value except if
you anticipate deviations between the value in Shill and the enforce policy
value. In that case it depends on what value we prefer to show...

[stevenjb, 2014/07/30 21:57:48]

As discussed in chat, 'Active' may not be set.

+      var effective = property.Effective;
+      if (effective in property)
+        return property[effective];
+    }
+    return undefined;
+  }
+
+  /**
+   * Helper function to get a property from a dictionary in an ONC Managed

[pneubeck (no reviews), 2014/07/30 20:12:45]

again i'd drop 'a dictionary in'

[stevenjb, 2014/07/30 21:57:47]

Simplified comment.

+   * Property dictionary, e.g. getActiveDictionaryValue(data, WiFi, Strength).
+   * @param {object|string} property The property which must ether be
+   *   a dictionary object or not be present.
+   * @return {*} the property value or undefined.
+   */
+  function getActiveDictionaryValue(data, dict, key) {

[pneubeck (no reviews), 2014/07/30 20:12:45]

hm. again could be one argument less
get..(data, key) {
   if (typeof data != 'object')
       return undefined;
   return getActiveValue(data[key]);
}

[stevenjb, 2014/07/30 21:57:47]

Same reason.

+    if (!(dict in data))
+      return undefined;
+    return getActiveValue(data[dict], key);
+  }
+
+  /**
    * Helper function to set hidden attribute for elements matching a selector.
    * @param {string} selector CSS selector for extracting a list of elements.
    * @param {bool} hidden New hidden value.
@@ -96,17 +131,9 @@ cr.define('options.internet', function() {
    * @param {Object} data The network ONC dictionary.
    */
   function getNetworkName(data) {
-    if (data.Type == 'Ethernet')
+    if (data.type == 'Ethernet')
       return loadTimeData.getString('ethernetName');
-    return data.Name;
-  }
-
-  /**
-   * Returns True if the network represented by 'data' is a secure WiFi network.
-   * @param {Object} data The network ONC dictionary.
-   */
-  function isSecureWiFiNetwork(data) {
-    return data.WiFi && data.WiFi.Security && data.WiFi.Security != 'None';
+    return getActiveValue(data, 'Name');
   }
 
   /////////////////////////////////////////////////////////////////////////////
@@ -151,7 +178,6 @@ cr.define('options.internet', function() {
       chrome.send('networkCommand', [networkType, servicePath, 'options']);
     },
 
-
     /**
      * Initializes the contents of the page.
      */
@@ -416,9 +442,51 @@ cr.define('options.internet', function() {
     },
 
     /**
-     * Update details page controls.
+     * Creates an indicator event for controlled properties using
+     * the same dictionary format as CoreOptionsHandler::CreateValueForPref.
+     * @param {string} name The name for the Event.
+     * @param {Object} data Property dictionary with |value|, |controlledBy|,
+     *  and |recommendedValue| properties set.
+     * @private
+     */
+    createControlledEvent_: function(name, propData) {
+      var event = new Event(name);
+      event.value = {
+        value: propData.value,
+        controlledBy: propData.controlledBy,
+        recommendedValue: propData.recommendedValue
+      };
+      return event;
+    },
+
+    /**
+     * Creates an indicator event for controlled properties using
+     * the ONC getManagedProperties dictionary format.
+     * @param {string} name The name for the Event.
+     * @param {Object} data ONC managed network property dictionary.
      * @private
      */
+    createManagedEvent_: function(name, propData) {

[pneubeck (no reviews), 2014/07/30 20:12:45]

nit: replace propData['abc'] by propData.abc

[stevenjb, 2014/07/30 21:57:48]

So, I kept you out of a long email thread that wanted all of the ONC property
values to use javaScriptCase instead of PascalCase since that is the convention
in JS.

Eventually I was able to make the case that these are just property
dictionaries, and it is a quirk of JS that it treats Objects and Dictionaries
the same.

In order to make that more clear, I have intentionally used [] instead of '.' as
I switch to ONC properties. 

+      var event = new Event(name);
+      event.value = {};
+      if ('Effective' in propData) {
+        var effective = propData['Effective'];
+        if (effective != 'Unmanaged')

[pneubeck (no reviews), 2014/07/30 20:12:45]

var policy;
if (effective == UserPolicy || effective == DevicePolicy)
  policy = effective;
var is_editable = propData.UserEditable || propData.DeviceEditable;

if (policy && !is_editable)
  ..controlledBy = 'policy'
else if (policy && is_editable) {
  ..controlledBy = 'recommended'
  ..recommendedValue = propData[policy];
}
..value = propData.Active;

[stevenjb, 2014/07/30 21:57:47]

I wasn't sure whether there might be other policy types added, but I guess if
there are we will need to change this anyway.

I also didn't see any sign of {User|Device}Editable being used in the existing
code and was wary of adding that at this stage, but perhaps I missed it? If this
would be new, I would rather add it in a followup.

[pneubeck (no reviews), 2014/07/31 20:29:36]

You should see the *Editable fields already if you apply a policy with
recommended settings.
E.g. our OpenVPN has an editable hostname. Google-A has an editable
EAP.Identity. Or you could set something up on managedchrome.com.

[stevenjb, 2014/08/01 00:53:41]

OK, I think I get it now, documenting the properties of each structure helped.
The missing piece was that we need to set the *Editable property in the C++
code.

+          event.value.controlledBy = 'policy';
+        else
+          event.value.controlledBy = 'recommended';
+        event.value.recommendedValue = propData[effective];
+      }
+      if ('Active' in propData)

[pneubeck (no reviews), 2014/07/30 20:12:45]

'Active' should always be present.

[stevenjb, 2014/07/30 21:57:47]

See previous comment.

+        event.value.value = propData['Active'];
+      else
+        event.value.value = event.value.recommendedValue;
+      return event;
+    },
+
+    /**
+     * Update details page controls.
+     */
     updateControls: function() {
       // Only show ipconfig section if network is connected OR if nothing on
       // this device is connected. This is so that you can fix the ip configs
@@ -430,10 +498,13 @@ cr.define('options.internet', function() {
         !this.connected && this.deviceConnected;
 
       // Network type related.
-      updateHidden('#details-internet-page .cellular-details', !this.cellular);
-      updateHidden('#details-internet-page .wifi-details', !this.wireless);
-      updateHidden('#details-internet-page .wimax-details', !this.wimax);
-      updateHidden('#details-internet-page .vpn-details', !this.vpn);
+      updateHidden('#details-internet-page .cellular-details',
+                   this.type != 'Cellular');
+      updateHidden('#details-internet-page .wifi-details',
+                   this.type != 'WiFi');
+      updateHidden('#details-internet-page .wimax-details',
+                   this.type != 'Wimax');
+      updateHidden('#details-internet-page .vpn-details', this.type != 'VPN');
       updateHidden('#details-internet-page .proxy-details', !this.showProxy);
 
       // Cellular
@@ -447,16 +518,13 @@ cr.define('options.internet', function() {
 
       // Wifi
 
-      // Network information merged into the Wifi tab for wireless networks
-      // unless the option is set for enabling a static IP configuration.
+      // Hide network tab for VPN.
       updateHidden('#details-internet-page .network-details',
-                   (this.wireless && !this.showStaticIPConfig) || this.vpn);

[stevenjb, 2014/07/29 23:21:11]

We were setting showStaticIPConfig = true for wifi networks (this.wireless) so
this is just type == VPN. I think maybe 'wireless' use to include cellular
and/or wimax?

[stevenjb, 2014/07/30 21:57:47]

Acknowledged.

-      updateHidden('#details-internet-page .wifi-network-setting',
-                   this.showStaticIPConfig);

[stevenjb, 2014/07/29 23:21:11]

Always true, so removed elements with that class.

[stevenjb, 2014/07/30 21:57:47]

Acknowledged.

+                   this.type == 'VPN');
 
       // Password and shared.
       updateHidden('#details-internet-page #password-details',
-                   !this.wireless || !this.hasSecurity);
+                   this.type != 'WiFi' || !this.hasSecurity);
       updateHidden('#details-internet-page #wifi-shared-network',
           !this.shared);
       updateHidden('#details-internet-page #prefer-network',
@@ -464,7 +532,7 @@ cr.define('options.internet', function() {
 
       // WiMAX.
       updateHidden('#details-internet-page #wimax-shared-network',
-        !this.shared);
+                   !this.shared);
 
       // Proxy
       this.updateProxyBannerVisibility_();
@@ -591,7 +659,7 @@ cr.define('options.internet', function() {
       $('manual-proxy-parms').hidden = !$('manual-proxy').checked;
       chrome.send('coreOptionsUserMetricsAction',
                   ['Options_NetworkManualProxy_Enable']);
-    },
+    }
   };
 
   /**
@@ -652,9 +720,6 @@ cr.define('options.internet', function() {
     $('activate-details').hidden = true;
     $('view-account-details').hidden = true;
     $('web-proxy-auto-discovery').hidden = true;
-    detailsPage.cellular = false;
-    detailsPage.wireless = false;
-    detailsPage.vpn = false;
     detailsPage.showProxy = true;
     updateHidden('#internet-tab', true);
     updateHidden('#details-tab-strip', true);
@@ -712,47 +777,47 @@ cr.define('options.internet', function() {
   DetailsInternetPage.loginFromDetails = function() {
     var data = $('connection-state').data;
     var servicePath = data.servicePath;
-    chrome.send('networkCommand', [data.Type, servicePath, 'connect']);
+    chrome.send('networkCommand', [data.type, servicePath, 'connect']);
     OptionsPage.closeOverlay();
   };
 
   DetailsInternetPage.disconnectNetwork = function() {
     var data = $('connection-state').data;
     var servicePath = data.servicePath;
-    chrome.send('networkCommand', [data.Type, servicePath, 'disconnect']);
+    chrome.send('networkCommand', [data.type, servicePath, 'disconnect']);
     OptionsPage.closeOverlay();
   };
 
   DetailsInternetPage.configureNetwork = function() {
     var data = $('connection-state').data;
     var servicePath = data.servicePath;
-    chrome.send('networkCommand', [data.Type, servicePath, 'configure']);
+    chrome.send('networkCommand', [data.type, servicePath, 'configure']);
     OptionsPage.closeOverlay();
   };
 
   DetailsInternetPage.activateFromDetails = function() {
     var data = $('connection-state').data;
     var servicePath = data.servicePath;
-    if (data.Type == 'Cellular')
-      chrome.send('networkCommand', [data.Type, servicePath, 'activate']);
+    if (data.type == 'Cellular')
+      chrome.send('networkCommand', [data.type, servicePath, 'activate']);
     OptionsPage.closeOverlay();
   };
 
   DetailsInternetPage.setDetails = function() {
     var data = $('connection-state').data;
     var servicePath = data.servicePath;
-    if (data.Type == 'WiFi') {
+    if (data.type == 'WiFi') {
       sendCheckedIfEnabled(servicePath, 'setPreferNetwork',
                            $('prefer-network-wifi'));
       sendCheckedIfEnabled(servicePath, 'setAutoConnect',
                            $('auto-connect-network-wifi'));
-    } else if (data.Type == 'Wimax') {
+    } else if (data.type == 'Wimax') {
       sendCheckedIfEnabled(servicePath, 'setAutoConnect',
                            $('auto-connect-network-wimax'));
-    } else if (data.Type == 'Cellular') {
+    } else if (data.type == 'Cellular') {
       sendCheckedIfEnabled(servicePath, 'setAutoConnect',
                            $('auto-connect-network-cellular'));
-    } else if (data.Type == 'VPN') {
+    } else if (data.type == 'VPN') {
       chrome.send('setServerHostname',
                   [servicePath,
                    $('inet-server-hostname').value]);
@@ -825,22 +890,23 @@ cr.define('options.internet', function() {
   };
 
   DetailsInternetPage.updateConnectionButtonVisibilty = function(data) {
-    var connected = data.ConnectionState == 'Connected';
+    var connected = getActiveValue(data, 'ConnectionState') == 'Connected';
+    var connectable = getActiveValue(data, 'Connectable');
     $('details-internet-login').hidden = connected;
-    $('details-internet-login').disabled = !data.Connectable;
+    $('details-internet-login').disabled = !connectable;
 
-    if (data.Type == 'Ethernet') {
+    if (data.type == 'Ethernet') {
       // Ethernet can be configured while connected (e.g. to set security).
       $('details-internet-configure').hidden = false;
     } else if (!connected &&
-               (!data.Connectable || isSecureWiFiNetwork(data) ||
-                (data.Type == 'Wimax' || data.Type == 'VPN'))) {
+               (!connectable || this.hasSecurity ||
+                (data.type == 'Wimax' || data.type == 'VPN'))) {
       $('details-internet-configure').hidden = false;
     } else {
       $('details-internet-configure').hidden = true;
     }
 
-    if (data.Type == 'Ethernet')
+    if (data.type == 'Ethernet')
       $('details-internet-disconnect').hidden = true;
     else
       $('details-internet-disconnect').hidden = !connected;
@@ -861,19 +927,19 @@ cr.define('options.internet', function() {
     // Update our cached data object.
     updateDataObject(data, update);
 
+    var connectionState = getActiveValue(data, 'ConnectionState');
+    var connectionStateString = networkOncStateString(connectionState);
     detailsPage.deviceConnected = data.deviceConnected;
-    detailsPage.connecting = data.ConnectionState == 'Connecting';
-    detailsPage.connected = data.ConnectionState == 'Connected';
-    var connectionStateString = networkOncStateString(data.ConnectionState);
+    detailsPage.connected = connectionState == 'Connected';
     $('connection-state').textContent = connectionStateString;
 
     this.updateConnectionButtonVisibilty(data);
 
-    if (data.Type == 'WiFi') {
+    if (data.type == 'WiFi') {
       $('wifi-connection-state').textContent = connectionStateString;
-    } else if (data.Type == 'Wimax') {
+    } else if (data.type == 'Wimax') {
       $('wimax-connection-state').textContent = connectionStateString;
-    } else if (data.Type == 'Cellular') {
+    } else if (data.type == 'Cellular') {
       $('activation-state').textContent = data.activationState;
 
       $('buyplan-details').hidden = !data.showBuyButton;
@@ -897,27 +963,31 @@ cr.define('options.internet', function() {
   DetailsInternetPage.showDetailedInfo = function(data) {
     var detailsPage = DetailsInternetPage.getInstance();
 
+    data.type = getActiveValue(data, 'Type');  // Get Active Type value.
+
     // Populate header
     $('network-details-title').textContent = getNetworkName(data);
-    var connectionStateString = networkOncStateString(data.ConnectionState);
+    var connectionState = getActiveValue(data, 'ConnectionState');
+    var connectionStateString = networkOncStateString(connectionState);
+    detailsPage.connected = connectionState == 'Connected';
     $('network-details-subtitle-status').textContent = connectionStateString;
     var typeKey = null;
-    switch (data.Type) {
-    case 'Ethernet':
-      typeKey = 'ethernetTitle';
-      break;
-    case 'WiFi':
-      typeKey = 'wifiTitle';
-      break;
-    case 'Wimax':
-      typeKey = 'wimaxTitle';
-      break;
-    case 'Cellular':
-      typeKey = 'cellularTitle';
-      break;
-    case 'VPN':
-      typeKey = 'vpnTitle';
-      break;
+    switch (data.type) {
+      case 'Ethernet':
+        typeKey = 'ethernetTitle';
+        break;
+      case 'WiFi':
+        typeKey = 'wifiTitle';
+        break;
+      case 'Wimax':
+        typeKey = 'wimaxTitle';
+        break;
+      case 'Cellular':
+        typeKey = 'cellularTitle';
+        break;
+      case 'VPN':
+        typeKey = 'vpnTitle';
+        break;
     }
     var typeLabel = $('network-details-subtitle-type');
     var typeSeparator = $('network-details-subtitle-separator');
@@ -930,8 +1000,7 @@ cr.define('options.internet', function() {
       typeSeparator.hidden = true;
     }
 
-    // TODO(chocobo): Is this hack to cache the data here reasonable?
-    // TODO(kevers): Find more appropriate place to cache data.
+    // TODO(stevenjb): Find a more appropriate place to cache data.
     $('connection-state').data = data;
 
     $('buyplan-details').hidden = true;
@@ -943,8 +1012,7 @@ cr.define('options.internet', function() {
     $('web-proxy-auto-discovery').hidden = true;
 
     detailsPage.deviceConnected = data.deviceConnected;
-    detailsPage.connecting = data.ConnectionState == 'Connecting';
-    detailsPage.connected = data.ConnectionState == 'Connected';
+    detailsPage.connected = connectionState == 'Connected';
 
     // Only show proxy for remembered networks.
     if (data.remembered) {
@@ -953,7 +1021,6 @@ cr.define('options.internet', function() {
     } else {
       detailsPage.showProxy = false;
     }
-    detailsPage.showStaticIPConfig = data.showStaticIPConfig;
     $('connection-state').textContent = connectionStateString;
 
     var ipAutoConfig = data.ipAutoConfig ? 'automatic' : 'user';
@@ -1044,94 +1111,98 @@ cr.define('options.internet', function() {
 
     DetailsInternetPage.updateNameServerDisplay(data.nameServerType);
 
-    if (data.MacAddress) {
-      $('hardware-address').textContent = data.MacAddress;
+    var macAddress = getActiveValue(data, 'MacAddress');
+    if (macAddress) {
+      $('hardware-address').textContent = macAddress;
       $('hardware-address-row').style.display = 'table-row';
     } else {
       // This is most likely a device without a hardware address.
       $('hardware-address-row').style.display = 'none';
     }
 
+    var setOrHideEntry = function(field, property) {
+      var fieldEntryName = field + '-entry';

[pneubeck (no reviews), 2014/07/30 20:12:45]

Please consider this very thoroughly.
I usually prefer the searchability of element ids instead of saving some code.
Is there maybe another solution that does not have to construct ids?
E.g. in the next function the parent is hidden instead.

(fine to punt)

[stevenjb, 2014/07/30 21:57:48]

I generally agree with you on the importance of searchability. It turns out
though that we can just use setOrHideParent in all cases.

+      if (property) {
+        $(field).textContent = property;
+        $(fieldEntryName).hidden = false;
+      } else {
+        $(fieldEntryName).hidden = true;
+      }
+    };
+
+    var setOrHideParent = function(field, property) {
+      if (property) {
+        $(field).textContent = property;
+        $(field).parentElement.hidden = false;
+      } else {
+        $(field).parentElement.hidden = true;
+      }
+    };
+
+    var networkName = getNetworkName(data);
+
     // Signal strength as percentage (for WiFi and Wimax).
-    var signalStrength =
-        (data.WiFi && data.WiFi.SignalStrength) ? data.WiFi.SignalStrength : 0;
+    var signalStrength;
+    if (data.type == 'WiFi' || data.type == 'Wimax') {
+      signalStrength =
+          getActiveDictionaryValue(data, data.type, 'SignalStrength');
+    }
+    if (!signalStrength)
+      signalStrength = 0;
     var strengthFormat = loadTimeData.getString('inetSignalStrengthFormat');
-    strengthFormat = strengthFormat.replace('$1', signalStrength);
+    var strengthString = strengthFormat.replace('$1', signalStrength);
 
-    if (data.Type == 'WiFi') {
+    detailsPage.type = data.type;
+    if (data.type == 'WiFi') {
+      assert(data.WiFi, 'WiFi network has no WiFi object' + networkName);
       OptionsPage.showTab($('wifi-network-nav-tab'));
-      detailsPage.wireless = true;
-      detailsPage.vpn = false;
-      detailsPage.ethernet = false;
-      detailsPage.cellular = false;
       detailsPage.gsm = false;
-      detailsPage.wimax = false;
       detailsPage.shared = data.shared;
       $('wifi-connection-state').textContent = connectionStateString;
-      $('wifi-ssid').textContent = data.WiFi ? data.WiFi.SSID : data.Name;
-      if (data.WiFi && data.WiFi.BSSID) {
-        $('wifi-bssid').textContent = data.WiFi.BSSID;
-        $('wifi-bssid-entry').hidden = false;
-      } else {
-        $('wifi-bssid-entry').hidden = true;
-      }
-      $('wifi-ip-address').textContent = inetAddress.value;
-      $('wifi-netmask').textContent = inetNetmask.value;
-      $('wifi-gateway').textContent = inetGateway.value;
-      $('wifi-name-servers').textContent = inetNameServers;
-      var hasSecurity = isSecureWiFiNetwork(data);
-      if (hasSecurity) {
-        $('wifi-security').textContent = data.WiFi.Security;
-        $('wifi-security-entry').hidden = false;
-      } else {
-        $('wifi-security-entry').hidden = true;
-      }
+      var ssid = getActiveDictionaryValue(data, 'WiFi', 'SSID');
+      $('wifi-ssid').textContent = ssid ? ssid : networkName;
+      setOrHideEntry('wifi-bssid',
+                     getActiveDictionaryValue(data, 'WiFi', 'BSSID'));
+      var security = getActiveDictionaryValue(data, 'WiFi', 'Security');
+      if (security == 'None')
+        security = undefined;
+      setOrHideEntry('wifi-security', security);
       // Frequency is in MHz.
-      var frequency =
-          data.WiFi && data.WiFi.Frequency ? data.WiFi.Frequency : 0;
+      var frequency = getActiveDictionaryValue(data, 'WiFi', 'Frequency');
+      if (!frequency)
+        frequency = 0;
       var frequencyFormat = loadTimeData.getString('inetFrequencyFormat');
       frequencyFormat = frequencyFormat.replace('$1', frequency);
       $('wifi-frequency').textContent = frequencyFormat;
-      $('wifi-signal-strength').textContent = strengthFormat;
-      if (data.MacAddress) {
-        $('wifi-hardware-address').textContent = data.MacAddress;
-        $('wifi-hardware-address-entry').hidden = false;
-      } else {
-        $('wifi-hardware-address-entry').hidden = true;
-      }
+      $('wifi-signal-strength').textContent = strengthString;
+      setOrHideEntry('wifi-hardware-address',
+                     getActiveValue(data, 'MacAddress'));
       detailsPage.showPreferred = data.remembered;
       $('prefer-network-wifi').checked = data.preferred.value;
       $('prefer-network-wifi').disabled = !data.remembered;
-      $('auto-connect-network-wifi').checked = data.autoConnect.value;
+      $('auto-connect-network-wifi').checked =
+          getActiveValue(data, 'AutoConnect');
       $('auto-connect-network-wifi').disabled = !data.remembered;
-      detailsPage.hasSecurity = hasSecurity;
-    } else if (data.Type == 'Wimax') {
+      detailsPage.hasSecurity = security != undefined;
+    } else if (data.type == 'Wimax') {
+      assert(data.Wimax, 'Wimax network has no Wimax object' + networkName);
       OptionsPage.showTab($('wimax-network-nav-tab'));
-      detailsPage.wimax = true;
-      detailsPage.wireless = false;
-      detailsPage.vpn = false;
-      detailsPage.ethernet = false;
-      detailsPage.cellular = false;
       detailsPage.gsm = false;
       detailsPage.shared = data.shared;
       detailsPage.showPreferred = data.remembered;
       $('wimax-connection-state').textContent = connectionStateString;
-      $('auto-connect-network-wimax').checked = data.autoConnect.value;
+      $('auto-connect-network-wimax').checked =
+          getActiveValue(data, 'AutoConnect');
       $('auto-connect-network-wimax').disabled = !data.remembered;
-      if (data.identity) {
-        $('wimax-eap-identity').textContent = data.identity;
-        $('wimax-eap-identity-entry').hidden = false;
-      } else {
-        $('wimax-eap-identity-entry').hidden = true;
-      }
-      $('wimax-signal-strength').textContent = strengthFormat;
-    } else if (data.Type == 'Cellular') {
+      var identity;
+      if (data.Wimax.EAP)
+        identity = getActiveValue(data.Wimax.EAP, 'Identity');
+      setOrHideEntry('wifi-eap-identity', identity);

[pneubeck (no reviews), 2014/07/30 20:12:46]

could actually be setOrHideParent in this case.

[stevenjb, 2014/07/30 21:57:47]

So, this should actually be wimax-eap-identity, oops, but yes, it looks like all
of these can use setOrHideParent.

+      $('wimax-signal-strength').textContent = strengthString;
+    } else if (data.type == 'Cellular') {
+      assert(data.Cellular,
+             'Cellular network has no Cellular object' + networkName);
       OptionsPage.showTab($('cellular-conn-nav-tab'));
-      detailsPage.ethernet = false;
-      detailsPage.wireless = false;
-      detailsPage.wimax = false;
-      detailsPage.vpn = false;
-      detailsPage.cellular = true;
       if (data.showCarrierSelect && data.currentCarrierIndex != -1) {
         var carrierSelector = $('select-carrier');
         carrierSelector.onchange = DetailsInternetPage.handleCarrierChanged;
@@ -1143,24 +1214,32 @@ cr.define('options.internet', function() {
         }
         carrierSelector.selectedIndex = data.currentCarrierIndex;
       } else {
-        $('service-name').textContent = getNetworkName(data);
+        $('service-name').textContent = networkName;
       }
 
-      $('network-technology').textContent = data.Cellular.NetworkTechnology;
+      $('network-technology').textContent =
+          getActiveDictionaryValue(data, 'Cellular', 'NetworkTechnology');
       $('activation-state').textContent = data.activationState;
       $('roaming-state').textContent = data.roamingState;
       $('restricted-pool').textContent = data.restrictedPool;
-      $('error-state').textContent = data.errorState;
-      $('manufacturer').textContent = data.Cellular.Manufacturer;
-      $('model-id').textContent = data.Cellular.ModelID;
-      $('firmware-revision').textContent = data.Cellular.FirmwareRevision;
-      $('hardware-revision').textContent = data.Cellular.HardwareRevision;
-      $('mdn').textContent = data.Cellular.MDN;
+      $('error-state').textContent = data.errorMessage;
+      $('manufacturer').textContent =
+          getActiveDictionaryValue(data, 'Cellular', 'Manufacturer');
+      $('model-id').textContent =
+          getActiveDictionaryValue(data, 'Cellular', 'ModelID');
+      $('firmware-revision').textContent =
+          getActiveDictionaryValue(data, 'Cellular', 'FirmwareRevision');
+      $('hardware-revision').textContent =
+          getActiveDictionaryValue(data, 'Cellular', 'HardwareRevision');
+      $('mdn').textContent = getActiveDictionaryValue(data, 'Cellular', 'MDN');
 
       // Show ServingOperator properties only if available.
+      var servingOperator;

[stevenjb, 2014/07/29 23:21:11]

Removed locally.

[stevenjb, 2014/07/30 21:57:47]

Done.

       if (data.Cellular.ServingOperator) {
-        $('operator-name').textContent = data.Cellular.ServingOperator.Name;
-        $('operator-code').textContent = data.Cellular.ServingOperator.Code;
+        $('operator-name').textContent =
+            getActiveValue(data.Cellular.ServingOperator, 'Name');
+        $('operator-code').textContent =
+            getActiveValue(data.Cellular.ServingOperator, 'Code');
       } else {
         $('operator-name').parentElement.hidden = true;
         $('operator-code').parentElement.hidden = true;
@@ -1171,23 +1250,23 @@ cr.define('options.internet', function() {
       updateHidden('#details-internet-page .cdma-only', false);
 
       // Show IMEI/ESN/MEID/MIN/PRL only if they are available.
-      (function() {
-        var setContentOrHide = function(field, value) {
-          if (value)
-            $(field).textContent = value;
-          else
-            $(field).parentElement.hidden = true;
-        };
-        setContentOrHide('esn', data.Cellular.ESN);
-        setContentOrHide('imei', data.Cellular.IMEI);
-        setContentOrHide('meid', data.Cellular.MEID);
-        setContentOrHide('min', data.Cellular.MIN);
-        setContentOrHide('prl-version', data.Cellular.PRLVersion);
-      })();
-      detailsPage.gsm = data.Cellular.Family == 'GSM';
+      setOrHideParent('esn', getActiveDictionaryValue(data, 'Cellular', 'ESN'));
+      setOrHideParent(
+          'imei', getActiveDictionaryValue(data, 'Cellular', 'IMEI'));
+      setOrHideParent(
+          'meid', getActiveDictionaryValue(data, 'Cellular', 'MEID'));
+      setOrHideParent('min', getActiveDictionaryValue(data, 'Cellular', 'MIN'));
+      setOrHideParent(
+          'prl-version',
+          getActiveDictionaryValue(data, 'Cellular', 'PRLVersion'));
+
+      var family = getActiveDictionaryValue(data, 'Cellular', 'GSM');
+      detailsPage.gsm = family == 'GSM';
       if (detailsPage.gsm) {
-        $('iccid').textContent = data.Cellular.ICCID;
-        $('imsi').textContent = data.Cellular.IMSI;
+        $('iccid').textContent =
+            getActiveDictionaryValue(data, 'Cellular', 'ICCID');
+        $('imsi').textContent =
+            getActiveDictionaryValue(data, 'Cellular', 'IMSI');
 
         var apnSelector = $('select-apn');
         // Clear APN lists, keep only last element that "other".
@@ -1236,7 +1315,8 @@ cr.define('options.internet', function() {
         $('sim-card-lock-enabled').checked = lockEnabled;
         $('change-pin').hidden = !lockEnabled;
       }
-      $('auto-connect-network-cellular').checked = data.autoConnect.value;
+      $('auto-connect-network-cellular').checked =
+          getActiveValue(data, 'AutoConnect');
       $('auto-connect-network-cellular').disabled = false;
 
       $('buyplan-details').hidden = !data.showBuyButton;
@@ -1245,15 +1325,10 @@ cr.define('options.internet', function() {
       if (data.showActivateButton) {
         $('details-internet-login').hidden = true;
       }
-    } else if (data.Type == 'VPN') {
+    } else if (data.type == 'VPN') {
       OptionsPage.showTab($('vpn-nav-tab'));
-      detailsPage.wireless = false;
-      detailsPage.wimax = false;
-      detailsPage.vpn = true;
-      detailsPage.ethernet = false;
-      detailsPage.cellular = false;
       detailsPage.gsm = false;
-      $('inet-service-name').textContent = getNetworkName(data);
+      $('inet-service-name').textContent = networkName;
       $('inet-provider-type').textContent = data.providerType;
       $('inet-username').textContent = data.username;
       var inetServerHostname = $('inet-server-hostname');
@@ -1262,38 +1337,30 @@ cr.define('options.internet', function() {
         OptionsPage.hideBubble();
         inetServerHostname.value = data.serverHostname.recommendedValue;
       };
-      $('auto-connect-network-vpn').checked = data.autoConnect.value;
+      $('auto-connect-network-vpn').checked =
+          getActiveValue(data, 'AutoConnect');
       $('auto-connect-network-vpn').disabled = false;
     } else {
       OptionsPage.showTab($('internet-nav-tab'));
-      detailsPage.ethernet = true;
-      detailsPage.wireless = false;
-      detailsPage.wimax = false;
-      detailsPage.vpn = false;
-      detailsPage.cellular = false;
-      detailsPage.gsm = false;
     }
 
     // Update controlled option indicators.
-    indicators = cr.doc.querySelectorAll(
+    var indicators = cr.doc.querySelectorAll(
         '#details-internet-page .controlled-setting-indicator');
     for (var i = 0; i < indicators.length; i++) {
-      var propName = indicators[i].getAttribute('data');
+      var prop = indicators[i].hasAttribute('managed') ? 'managed' : 'data';

[pneubeck (no reviews), 2014/07/30 20:12:45]

prop -> attributeName or managedType ?

"prop" indicates that it's the value of something with the name |propName|.
confusing.

[stevenjb, 2014/07/30 21:57:48]

Done.

+      var propName = indicators[i].getAttribute(prop);
       if (!propName || !data[propName])
         continue;
-      var propData = data[propName];
-      // Create a synthetic pref change event decorated as
-      // CoreOptionsHandler::CreateValueForPref() does.
-      var event = new Event(name);
-      event.value = {
-        value: propData.value,
-        controlledBy: propData.controlledBy,
-        recommendedValue: propData.recommendedValue
-      };
+      var event;
+      if (prop == 'managed')
+        event = detailsPage.createManagedEvent_(propName, data[propName]);
+      else
+        event = detailsPage.createControlledEvent_(propName, data[propName]);
       indicators[i].handlePrefChange(event);
       var forElement = $(indicators[i].getAttribute('for'));
       if (forElement) {
-        if (propData.controlledBy == 'policy')
+        if (event.value.controlledBy == 'policy')
           forElement.disabled = true;
         if (forElement.resetHandler)
           indicators[i].resetHandler = forElement.resetHandler;