Support Managed NetworkState format dictionaries for controlled settings.

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

 // Copyright (c) 2012 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.
 
 // NOTE(stevenjb): This code is in the process of being converted to be
 // compatible with the networkingPrivate extension API:
 // * The network property dictionaries are being converted to use ONC values.
 // * chrome.send calls will be replaced with an API object that simulates the
 //   networkingPrivate API. See network_config.js.
 // See crbug.com/279351 for more info.
 
 cr.define('options.internet', function() {
   var OptionsPage = options.OptionsPage;
   /** @const */ var ArrayDataModel = cr.ui.ArrayDataModel;
   /** @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.
    */
   function updateHidden(selector, hidden) {
     var elements = cr.doc.querySelectorAll(selector);
     for (var i = 0, el; el = elements[i]; i++) {
       el.hidden = hidden;
     }
   }
@@ skipping 60 matching lines @@
     else if (state == 'Connected')
       return loadTimeData.getString('OncStateConnected');
     return loadTimeData.getString('OncStateUnknown');
   }
 
   /**
    * Returns the display name for the network represented by 'data'.
    * @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');
   }
 
   /////////////////////////////////////////////////////////////////////////////
   // DetailsInternetPage class:
 
   /**
    * Encapsulated handling of ChromeOS internet details overlay page.
    * @constructor
    */
   function DetailsInternetPage() {
@@ skipping 24 matching lines @@
      * is included in the URL.
      */
     showNetworkDetails_: function(params) {
       var servicePath = params.servicePath;
       if (!servicePath || !servicePath.length)
         return;
       var networkType = '';  // ignored for 'options'
       chrome.send('networkCommand', [networkType, servicePath, 'options']);
     },
 
-
     /**
      * Initializes the contents of the page.
      */
     initializePageContents_: function(params) {
       $('details-internet-dismiss').addEventListener('click', function(event) {
         DetailsInternetPage.setDetails();
       });
 
       $('details-internet-login').addEventListener('click', function(event) {
         DetailsInternetPage.setDetails();
@@ skipping 244 matching lines @@
      * Handler for when the name server selection changes.
      * @param {Event} e The click event.
      * @private
      */
     handleNameServerTypeChange_: function(event) {
       var type = event.target.value;
       DetailsInternetPage.updateNameServerDisplay(type);
     },
 
     /**
+     * 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.
-     * @private
      */
     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
       // if you can't connect to any network.
       // TODO(chocobo): Once ipconfig is moved to flimflam service objects,
       //   we need to redo this logic to allow configuration of all networks.
       $('ipconfig-section').hidden = !this.connected && this.deviceConnected;
       $('ipconfig-dns-section').hidden =
         !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
 
       // Conditionally call updateHidden on .gsm-only, so that we don't unhide
       // a previously hidden element.
       if (this.gsm)
         updateHidden('#details-internet-page .cdma-only', true);
       else
         updateHidden('#details-internet-page .gsm-only', true);
 
       // 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',
                    !this.showPreferred);
 
       // WiMAX.
       updateHidden('#details-internet-page #wimax-shared-network',
-        !this.shared);
+                   !this.shared);
 
       // Proxy
       this.updateProxyBannerVisibility_();
       this.toggleSingleProxy_();
       if ($('manual-proxy').checked)
         this.enableManualProxy_();
       else
         this.disableManualProxy_();
     },
 
@@ skipping 106 matching lines @@
       $('ftp-proxy').disabled = allDisabled;
       $('ftp-proxy-port').disabled = allDisabled;
       $('socks-host').disabled = allDisabled;
       $('socks-port').disabled = allDisabled;
       $('proxy-use-pac-url').disabled = true;
       $('proxy-pac-url').disabled = true;
       $('auto-proxy-parms').hidden = !$('auto-proxy').checked;
       $('manual-proxy-parms').hidden = !$('manual-proxy').checked;
       chrome.send('coreOptionsUserMetricsAction',
                   ['Options_NetworkManualProxy_Enable']);
-    },
+    }
   };
 
   /**
    * Enables or Disables all buttons that provide operations on the cellular
    * network.
    */
   DetailsInternetPage.changeCellularButtonsState = function(disable) {
     var buttonsToDisableList =
         new Array('details-internet-login',
                   'details-internet-disconnect',
@@ skipping 40 matching lines @@
   /**
    * Displays the InternetDetails dialog with only the proxy settings visible.
    */
   DetailsInternetPage.showProxySettings = function() {
     var detailsPage = DetailsInternetPage.getInstance();
     $('network-details-header').hidden = true;
     $('buyplan-details').hidden = true;
     $('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);
     updateHidden('#details-internet-page .action-area', true);
     detailsPage.updateControls();
     detailsPage.visible = true;
     chrome.send('coreOptionsUserMetricsAction',
                 ['Options_NetworkShowProxyTab']);
   };
 
@@ skipping 37 matching lines @@
       }
   };
 
   DetailsInternetPage.updateCarrier = function() {
     DetailsInternetPage.showCarrierChangeSpinner(false);
   };
 
   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]);
       sendCheckedIfEnabled(servicePath, 'setAutoConnect',
                            $('auto-connect-network-vpn'));
     }
 
     var nameServerTypes = ['automatic', 'google', 'user'];
     var nameServerType = 'automatic';
     for (var i = 0; i < nameServerTypes.length; ++i) {
@@ skipping 52 matching lines @@
         break;
       case 'user':
         automaticDns.removeAttribute('selected');
         googleDns.removeAttribute('selected');
         userDns.setAttribute('selected', '');
         break;
     }
   };
 
   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;
   };
 
   DetailsInternetPage.updateConnectionData = function(update) {
     var detailsPage = DetailsInternetPage.getInstance();
     if (!detailsPage.visible)
       return;
 
     var data = $('connection-state').data;
     if (!data)
       return;
 
     if (update.servicePath != data.servicePath)
       return;
 
     // 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;
       $('view-account-details').hidden = !data.showViewAccountButton;
 
       $('activate-details').hidden = !data.showActivateButton;
       if (data.showActivateButton)
         $('details-internet-login').hidden = true;
 
       if (detailsPage.gsm) {
         // TODO(stevenjb): Use managed properties for policy controlled values.
         var lockEnabled = data.simCardLockEnabled.value;
         $('sim-card-lock-enabled').checked = lockEnabled;
         $('change-pin').hidden = !lockEnabled;
       }
     }
 
     $('connection-state').data = data;
   };
 
   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');
     if (typeKey) {
       typeLabel.textContent = loadTimeData.getString(typeKey);
       typeLabel.hidden = false;
       typeSeparator.hidden = false;
     } else {
       typeLabel.hidden = true;
       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;
     $('activate-details').hidden = true;
     $('view-account-details').hidden = true;
 
     this.updateConnectionButtonVisibilty(data);
 
     $('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) {
       detailsPage.showProxy = true;
       chrome.send('selectNetwork', [data.servicePath]);
     } else {
       detailsPage.showProxy = false;
     }
-    detailsPage.showStaticIPConfig = data.showStaticIPConfig;
     $('connection-state').textContent = connectionStateString;
 
     var ipAutoConfig = data.ipAutoConfig ? 'automatic' : 'user';
     $('ip-automatic-configuration-checkbox').checked = data.ipAutoConfig;
     var inetAddress = {autoConfig: ipAutoConfig};
     var inetNetmask = {autoConfig: ipAutoConfig};
     var inetGateway = {autoConfig: ipAutoConfig};
 
     if (data.ipconfig.value) {
       inetAddress.automatic = data.ipconfig.value.address;
@@ skipping 70 matching lines @@
       nameServerModels.push({value: nameServersUser[i] || ''});
 
     $(data.nameServerType + '-dns-radio').checked = true;
     configureAddressField($('ipconfig-dns1'), nameServerModels[0]);
     configureAddressField($('ipconfig-dns2'), nameServerModels[1]);
     configureAddressField($('ipconfig-dns3'), nameServerModels[2]);
     configureAddressField($('ipconfig-dns4'), nameServerModels[3]);
 
     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;
         carrierSelector.options.length = 0;
         for (var i = 0; i < data.carriers.length; ++i) {
           var option = document.createElement('option');
           option.textContent = data.carriers[i];
           carrierSelector.add(option);
         }
         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;
       }
       // Make sure that GSM/CDMA specific properties that shouldn't be hidden
       // are visible.
       updateHidden('#details-internet-page .gsm-only', false);
       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".
         while (apnSelector.length != 1)
           apnSelector.remove(0);
         var otherOption = apnSelector[0];
         data.selectedApn = -1;
         data.userApnIndex = -1;
         var apnList = data.providerApnList.value;
         for (var i = 0; i < apnList.length; i++) {
@@ skipping 28 matching lines @@
           data.userApnIndex = data.selectedApn;
         }
         apnSelector.selectedIndex = data.selectedApn;
         updateHidden('.apn-list-view', false);
         updateHidden('.apn-details-view', true);
         // TODO(stevenjb): Used managed properties for policy controlled value.
         var lockEnabled = data.simCardLockEnabled.value;
         $('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;
       $('view-account-details').hidden = !data.showViewAccountButton;
       $('activate-details').hidden = !data.showActivateButton;
       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');
       inetServerHostname.value = data.serverHostname.value;
       inetServerHostname.resetHandler = 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;
       }
     }
 
     detailsPage.updateControls();
 
     // Don't show page name in address bar and in history to prevent people
     // navigate here by hand and solve issue with page session restore.
     OptionsPage.showPageByName('detailsInternetPage', false);
   };
 
   return {
     DetailsInternetPage: DetailsInternetPage
   };
 });