Some JS cleanup, including proper ownership of ONC properties.

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 8399a05feed9168d72b240444dff4876f399884a..4f096c3676de1d420fcf8071932a0f2d6ea1c996 100644
--- a/chrome/browser/resources/options/chromeos/internet_detail.js
+++ b/chrome/browser/resources/options/chromeos/internet_detail.js
@@ -31,19 +31,6 @@ cr.define('options.internet', function() {
     }
   }
 
-  /*
-   * Helper function to update the properties of the data object from the
-   * properties in the update object.
-   * @param {object} data object to update.
-   * @param {object} object containing the updated properties.
-   */
-  function updateDataObject(data, update) {
-    for (var prop in update) {
-      if (prop in data)
-        data[prop] = update[prop];
-    }
-  }
-
   /**
    * Monitor pref change of given element.
    * @param {Element} el Target element.
@@ -57,7 +44,7 @@ cr.define('options.internet', function() {
    * @param {Event} e The update event.
    */
   function handlePrefUpdate(e) {
-    DetailsInternetPage.getInstance().updateControls();
+    DetailsInternetPage.getInstance().updateControls_();

[armansito, 2014/09/06 00:38:56]

nit: It feels a bit awkward that this top-level function is calling what looks
like a private function (viz "_"). Should the function be called updateControls
like before?

[stevenjb, 2014/09/08 19:43:12]

Yeah, I think you're right. I will make updateControls "public".
Done.

   }
 
   /**
@@ -116,6 +103,9 @@ cr.define('options.internet', function() {
    * @constructor
    */
   function DetailsInternetPage() {
+    this.userApnIndex_ = -1;
+    this.selectedApnIndex_ = -1;
+    this.userApn_ = {};
     Page.call(this, 'detailsInternetPage', null, 'details-internet-page');
   }
 
@@ -174,128 +164,31 @@ cr.define('options.internet', function() {
       });
 
       $('buyplan-details').addEventListener('click', function(event) {
-        var data = $('connection-state').data;
-        chrome.send('buyDataPlan', [data.servicePath]);
+        chrome.send('buyDataPlan',
+                    [DetailsInternetPage.getInstance().servicePath_]);
         PageManager.closeOverlay();
       });
 
       $('view-account-details').addEventListener('click', function(event) {
-        var data = $('connection-state').data;
-        chrome.send('showMorePlanInfo', [data.servicePath]);
+        chrome.send('showMorePlanInfo',
+                    [DetailsInternetPage.getInstance().servicePath_]);
         PageManager.closeOverlay();
       });
 
       $('cellular-apn-use-default').addEventListener('click', function(event) {
-        var data = $('connection-state').data;
-        var onc = $('connection-state').onc;
-        var apnSelector = $('select-apn');
-
-        if (data.userApnIndex != -1) {
-          apnSelector.remove(data.userApnIndex);
-          data.userApnIndex = -1;
-        }
-
-        var activeApn;
-        var iApn = -1;
-        var apnList = onc.getActiveValue('Cellular.APNList');
-        if (apnList != undefined && apnList.length > 0) {
-          iApn = 0;
-          var defaultApn = apnList[iApn];
-          activeApn['AccessPointName'] =
-              stringFromValue(defaultApn['AccessPointName']);
-          activeApn['Username'] = stringFromValue(defaultApn['Username']);
-          activeApn['Password'] = stringFromValue(defaultApn['Password']);
-          chrome.send('setApn', [data.servicePath,
-                                 activeApn['AccessPointName'],
-                                 activeApn['Username'],
-                                 activeApn['Password']]);
-        }
-        onc.setManagedProperty('Cellular.APN', activeApn);
-        apnSelector.selectedIndex = iApn;
-        data.selectedApn = iApn;
-
-        updateHidden('.apn-list-view', false);
-        updateHidden('.apn-details-view', true);
+        DetailsInternetPage.getInstance().setDefaultApn_();
       });
 
       $('cellular-apn-set').addEventListener('click', function(event) {
-        if ($('cellular-apn').value == '')
-          return;
-
-        var data = $('connection-state').data;
-        var onc = $('connection-state').onc;
-        var apnSelector = $('select-apn');
-
-        var activeApn = {};
-        activeApn['AccessPointName'] =
-            stringFromValue($('cellular-apn').value);
-        activeApn['Username'] =
-            stringFromValue($('cellular-apn-username').value);
-        activeApn['Password'] =
-            stringFromValue($('cellular-apn-password').value);
-        onc.setManagedProperty('Cellular.APN', activeApn);
-        data.userApn = activeApn;
-        chrome.send('setApn', [data.servicePath,
-                               activeApn['AccessPointName'],
-                               activeApn['Username'],
-                               activeApn['Password']]);
-
-        if (data.userApnIndex != -1) {
-          apnSelector.remove(data.userApnIndex);
-          data.userApnIndex = -1;
-        }
-
-        var option = document.createElement('option');
-        option.textContent = activeApn['AccessPointName'];
-        option.value = -1;
-        option.selected = true;
-        apnSelector.add(option, apnSelector[apnSelector.length - 1]);
-        data.userApnIndex = apnSelector.length - 2;
-        data.selectedApn = data.userApnIndex;
-
-        updateHidden('.apn-list-view', false);
-        updateHidden('.apn-details-view', true);
+        DetailsInternetPage.getInstance().setApn_($('cellular-apn').value);
       });
 
       $('cellular-apn-cancel').addEventListener('click', function(event) {
-        $('select-apn').selectedIndex = $('connection-state').data.selectedApn;
-        updateHidden('.apn-list-view', false);
-        updateHidden('.apn-details-view', true);
+        DetailsInternetPage.getInstance().cancelApn_();
       });
 
       $('select-apn').addEventListener('change', function(event) {
-        var data = $('connection-state').data;
-        var onc = $('connection-state').onc;
-        var apnSelector = $('select-apn');
-        var apnDict;
-        if (apnSelector[apnSelector.selectedIndex].value != -1) {
-          var apnList = onc.getActiveValue('Cellular.APNList');
-          var apnIndex = apnSelector.selectedIndex;
-          assert(apnIndex < apnList.length);
-          apnDict = apnList[apnIndex];
-          chrome.send('setApn', [data.servicePath,
-                                 stringFromValue(apnDict['AccessPointName']),
-                                 stringFromValue(apnDict['Username']),
-                                 stringFromValue(apnDict['Password'])]);
-          data.selectedApn = apnIndex;
-        } else if (apnSelector.selectedIndex == data.userApnIndex) {
-          apnDict = data.userApn;
-          chrome.send('setApn', [data.servicePath,
-                                 stringFromValue(apnDict['AccessPointName']),
-                                 stringFromValue(apnDict['Username']),
-                                 stringFromValue(apnDict['Password'])]);
-          data.selectedApn = apnSelector.selectedIndex;
-        } else {
-          apnDict = onc.getActiveValue('Cellular.APN');
-          $('cellular-apn').value = stringFromValue(apnDict['AccessPointName']);
-          $('cellular-apn-username').value =
-              stringFromValue(apnDict['Username']);
-          $('cellular-apn-password').value =
-              stringFromValue(apnDict['Password']);
-
-          updateHidden('.apn-list-view', true);
-          updateHidden('.apn-details-view', false);
-        }
+        DetailsInternetPage.getInstance().selectApn_();
       });
 
       $('sim-card-lock-enabled').addEventListener('click', function(event) {
@@ -477,7 +370,7 @@ cr.define('options.internet', function() {
     /**
      * Update details page controls.
      */
-    updateControls: function() {
+    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.
@@ -489,12 +382,12 @@ cr.define('options.internet', function() {
 
       // Network type related.
       updateHidden('#details-internet-page .cellular-details',
-                   this.type != 'Cellular');
+                   this.type_ != 'Cellular');
       updateHidden('#details-internet-page .wifi-details',
-                   this.type != 'WiFi');
+                   this.type_ != 'WiFi');
       updateHidden('#details-internet-page .wimax-details',
-                   this.type != 'Wimax');
-      updateHidden('#details-internet-page .vpn-details', this.type != 'VPN');
+                   this.type_ != 'Wimax');
+      updateHidden('#details-internet-page .vpn-details', this.type_ != 'VPN');
       updateHidden('#details-internet-page .proxy-details', !this.showProxy);
 
       // Cellular
@@ -510,11 +403,11 @@ cr.define('options.internet', function() {
 
       // Hide network tab for VPN.
       updateHidden('#details-internet-page .network-details',
-                   this.type == 'VPN');
+                   this.type_ == 'VPN');
 
       // Password and shared.
       updateHidden('#details-internet-page #password-details',
-                   this.type != 'WiFi' || !this.hasSecurity);
+                   this.type_ != 'WiFi' || !this.hasSecurity);
       updateHidden('#details-internet-page #wifi-shared-network',
           !this.shared);
       updateHidden('#details-internet-page #prefer-network',
@@ -649,6 +542,233 @@ cr.define('options.internet', function() {
       $('manual-proxy-parms').hidden = !$('manual-proxy').checked;
       chrome.send('coreOptionsUserMetricsAction',
                   ['Options_NetworkManualProxy_Enable']);
+    },
+
+    updateConnectionButtonVisibilty_: function() {
+      var onc = this.onc_;
+      if (this.type_ == 'Ethernet') {
+        // Ethernet can never be connected or disconnected and can always be
+        // configured (e.g. to set security).
+        $('details-internet-login').hidden = true;
+        $('details-internet-disconnect').hidden = true;
+        $('details-internet-configure').hidden = false;
+        return;
+      }
+
+      var connectState = onc.getActiveValue('ConnectionState');
+      if (connectState == 'NotConnected') {
+        $('details-internet-login').hidden = false;
+        // Connecting to an unconfigured network might trigger certificate
+        // installation UI. Until that gets handled here, always enable the
+        // Connect button.
+        $('details-internet-login').disabled = false;
+        $('details-internet-disconnect').hidden = true;
+      } else {
+        $('details-internet-login').hidden = true;
+        $('details-internet-disconnect').hidden = false;
+      }
+
+      var connectable = onc.getActiveValue('Connectable');
+      if (connectState != 'Connected' &&
+          (!connectable || this.hasSecurity ||
+          (this.type_ == 'Wimax' || this.type_ == 'VPN'))) {
+        $('details-internet-configure').hidden = false;
+      } else {
+        $('details-internet-configure').hidden = true;
+      }
+    },
+
+    populateHeader_: function() {
+      var onc = this.onc_;
+
+      $('network-details-title').textContent = onc.getTranslatedValue('Name');
+      var connectionState = onc.getActiveValue('ConnectionState');
+      var connectionStateString = onc.getTranslatedValue('ConnectionState');
+      this.connected = connectionState == 'Connected';
+      $('network-details-subtitle-status').textContent = connectionStateString;
+      var typeKey;
+      var type = this.type_;
+      if (type == 'Ethernet')
+        typeKey = 'ethernetTitle';
+      else if (type == 'WiFi')
+        typeKey = 'wifiTitle';
+      else if (type == 'Wimax')
+        typeKey = 'wimaxTitle';
+      else if (type == 'Cellular')
+        typeKey = 'cellularTitle';
+      else if (type == 'VPN')
+        typeKey = 'vpnTitle';
+      else
+        typeKey = null;
+      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;
+      }
+    },
+
+    initializeApnList_: function(onc) {
+      var apnSelector = $('select-apn');
+      // Clear APN lists, keep only last element that "other".
+      while (apnSelector.length != 1) {
+        apnSelector.remove(0);
+      }
+      var otherOption = apnSelector[0];
+      var activeApn = onc.getActiveValue('Cellular.APN.AccessPointName');
+      var activeUsername = onc.getActiveValue('Cellular.APN.Username');
+      var activePassword = onc.getActiveValue('Cellular.APN.Password');
+      var lastGoodApn =
+          onc.getActiveValue('Cellular.LastGoodAPN.AccessPointName');
+      var lastGoodUsername =
+          onc.getActiveValue('Cellular.LastGoodAPN.Username');
+      var lastGoodPassword =
+          onc.getActiveValue('Cellular.LastGoodAPN.Password');
+      var apnList = onc.getActiveValue('Cellular.APNList');
+      for (var i = 0; i < apnList.length; i++) {
+        var apnDict = apnList[i];
+        var option = document.createElement('option');
+        var localizedName = apnDict['LocalizedName'];
+        var name = localizedName ? localizedName : apnDict['Name'];
+        var accessPointName = apnDict['AccessPointName'];
+        option.textContent =
+            name ? (name + ' (' + accessPointName + ')') : accessPointName;
+        option.value = i;
+        // If this matches the active Apn, or LastGoodApn, set it as the
+        // selected Apn.
+        if ((activeApn == accessPointName &&
+            activeUsername == apnDict['Username'] &&
+            activePassword == apnDict['Password']) ||
+            (!activeApn &&
+            lastGoodApn == accessPointName &&
+            lastGoodUsername == apnDict['Username'] &&
+            lastGoodPassword == apnDict['Password'])) {
+          this.selectedApnIndex_ = i;
+        }
+        // Insert new option before "other" option.
+        apnSelector.add(option, otherOption);
+      }
+      if (this.selectedApnIndex_ == -1 && activeApn) {
+        var activeOption = document.createElement('option');
+        activeOption.textContent = activeApn;
+        activeOption.value = -1;
+        apnSelector.add(activeOption, otherOption);
+        this.selectedApnIndex_ = apnSelector.length - 2;
+        this.userApnIndex_ = this.selectedApnIndex_;
+      }
+      assert(this.selectedApnIndex_ >= 0);
+      apnSelector.selectedIndex = this.selectedApnIndex_;
+      updateHidden('.apn-list-view', false);
+      updateHidden('.apn-details-view', true);
+    },
+
+    setDefaultApn_: function() {
+      var onc = this.onc_;
+      var apnSelector = $('select-apn');
+
+      if (this.userApnIndex_ != -1) {
+        apnSelector.remove(this.userApnIndex_);
+        this.userApnIndex_ = -1;
+      }
+
+      var activeApn;
+      var iApn = -1;
+      var apnList = onc.getActiveValue('Cellular.APNList');
+      if (apnList != undefined && apnList.length > 0) {
+        iApn = 0;
+        var defaultApn = apnList[iApn];
+        activeApn['AccessPointName'] =
+            stringFromValue(defaultApn['AccessPointName']);
+        activeApn['Username'] = stringFromValue(defaultApn['Username']);
+        activeApn['Password'] = stringFromValue(defaultApn['Password']);
+        chrome.send('setApn', [this.servicePath_,
+                               activeApn['AccessPointName'],
+                               activeApn['Username'],
+                               activeApn['Password']]);
+      }
+      onc.setManagedProperty('Cellular.APN', activeApn);

[pneubeck (no reviews), 2014/09/08 21:51:05]

maybe the original idea here was, that this property will be 'unset' in the else
case.

[stevenjb, 2014/09/08 23:25:16]

I thought of that, but I don't see any advantage to that behavior.
'Cellular.APN' is only ever relevant if 'Cellular.APNList' is non empty.

Eventually 'onc_' needs to be read-only so that the UI reflects whatever the
values actually get set to after and update gets sent.

+      apnSelector.selectedIndex = iApn;
+      this.selectedApnIndex_ = iApn;
+
+      updateHidden('.apn-list-view', false);
+      updateHidden('.apn-details-view', true);
+    },
+
+    setApn_: function(apnValue) {
+      if (apnValue == '')
+        return;
+
+      var onc = this.onc_;
+      var apnSelector = $('select-apn');
+
+      var activeApn = {};
+      activeApn['AccessPointName'] = stringFromValue(apnValue);
+      activeApn['Username'] = stringFromValue($('cellular-apn-username').value);
+      activeApn['Password'] = stringFromValue($('cellular-apn-password').value);
+      onc.setManagedProperty('Cellular.APN', activeApn);
+      this.userApn_ = activeApn;
+      chrome.send('setApn', [this.servicePath_,
+                             activeApn['AccessPointName'],
+                             activeApn['Username'],
+                             activeApn['Password']]);
+
+      if (this.userApnIndex_ != -1) {
+        apnSelector.remove(this.userApnIndex_);
+        this.userApnIndex_ = -1;
+      }
+
+      var option = document.createElement('option');
+      option.textContent = activeApn['AccessPointName'];
+      option.value = -1;
+      option.selected = true;
+      apnSelector.add(option, apnSelector[apnSelector.length - 1]);
+      this.userApnIndex_ = apnSelector.length - 2;
+      this.selectedApnIndex_ = this.userApnIndex_;
+
+      updateHidden('.apn-list-view', false);
+      updateHidden('.apn-details-view', true);
+    },
+
+    cancelApn_: function() {
+      $('select-apn').selectedIndex = this.selectedApnIndex_;
+      updateHidden('.apn-list-view', false);
+      updateHidden('.apn-details-view', true);
+    },
+
+    selectApn_: function() {
+      var onc = this.onc_;
+      var apnSelector = $('select-apn');
+      var apnDict;
+      if (apnSelector[apnSelector.selectedIndex].value != -1) {
+        var apnList = onc.getActiveValue('Cellular.APNList');
+        var apnIndex = apnSelector.selectedIndex;
+        assert(apnIndex < apnList.length);
+        apnDict = apnList[apnIndex];
+        chrome.send('setApn', [this.servicePath_,
+                               stringFromValue(apnDict['AccessPointName']),
+                               stringFromValue(apnDict['Username']),
+                               stringFromValue(apnDict['Password'])]);
+        this.selectedApnIndex_ = apnIndex;
+      } else if (apnSelector.selectedIndex == this.userApnIndex_) {
+        apnDict = this.userApn_;
+        chrome.send('setApn', [this.servicePath_,
+                               stringFromValue(apnDict['AccessPointName']),
+                               stringFromValue(apnDict['Username']),
+                               stringFromValue(apnDict['Password'])]);
+        this.selectedApnIndex_ = apnSelector.selectedIndex;
+      } else {
+        apnDict = onc.getActiveValue('Cellular.APN');
+        $('cellular-apn').value = stringFromValue(apnDict['AccessPointName']);
+        $('cellular-apn-username').value = stringFromValue(apnDict['Username']);
+        $('cellular-apn-password').value = stringFromValue(apnDict['Password']);
+
+        updateHidden('.apn-list-view', true);
+        updateHidden('.apn-details-view', false);
+      }
     }
   };
 
@@ -687,8 +807,8 @@ cr.define('options.internet', function() {
     var carrierSelector = $('select-carrier');
     var carrier = carrierSelector[carrierSelector.selectedIndex].textContent;
     DetailsInternetPage.showCarrierChangeSpinner(true);
-    var data = $('connection-state').data;
-    chrome.send('setCarrier', [data.servicePath, carrier]);
+    chrome.send('setCarrier', [
+      DetailsInternetPage.getInstance().servicePath_, carrier]);
   };
 
   /**
@@ -696,8 +816,7 @@ cr.define('options.internet', function() {
    * preparation for showing proxy-setttings.
    */
   DetailsInternetPage.initializeProxySettings = function() {
-    var detailsPage = DetailsInternetPage.getInstance();
-    detailsPage.initializePageContents_();
+    DetailsInternetPage.getInstance().initializePageContents_();
   };
 
   /**
@@ -714,7 +833,7 @@ cr.define('options.internet', function() {
     updateHidden('#internet-tab', true);
     updateHidden('#details-tab-strip', true);
     updateHidden('#details-internet-page .action-area', true);
-    detailsPage.updateControls();
+    detailsPage.updateControls_();
     detailsPage.visible = true;
     chrome.send('coreOptionsUserMetricsAction',
                 ['Options_NetworkShowProxyTab']);
@@ -765,49 +884,51 @@ cr.define('options.internet', function() {
   };
 
   DetailsInternetPage.loginFromDetails = function() {
-    var data = $('connection-state').data;
-    var servicePath = data.servicePath;
-    chrome.send('networkCommand', [data.type, servicePath, 'connect']);
+    var detailsPage = DetailsInternetPage.getInstance();
+    chrome.send('networkCommand',
+                [detailsPage.type_, detailsPage.servicePath_, 'connect']);
     PageManager.closeOverlay();
   };
 
   DetailsInternetPage.disconnectNetwork = function() {
-    var data = $('connection-state').data;
-    var servicePath = data.servicePath;
-    chrome.send('networkCommand', [data.type, servicePath, 'disconnect']);
+    var detailsPage = DetailsInternetPage.getInstance();
+    chrome.send('networkCommand',
+                [detailsPage.type_, detailsPage.servicePath_, 'disconnect']);
     PageManager.closeOverlay();
   };
 
   DetailsInternetPage.configureNetwork = function() {
-    var data = $('connection-state').data;
-    var servicePath = data.servicePath;
-    chrome.send('networkCommand', [data.type, servicePath, 'configure']);
+    var detailsPage = DetailsInternetPage.getInstance();
+    chrome.send('networkCommand',
+                [detailsPage.type_, detailsPage.servicePath_, 'configure']);
     PageManager.closeOverlay();
   };
 
   DetailsInternetPage.activateFromDetails = function() {
-    var data = $('connection-state').data;
-    var servicePath = data.servicePath;
-    if (data.type == 'Cellular')
-      chrome.send('networkCommand', [data.type, servicePath, 'activate']);
+    var detailsPage = DetailsInternetPage.getInstance();
+    if (detailsPage.type_ == 'Cellular') {
+      chrome.send('networkCommand',
+                  [detailsPage.type_, detailsPage.servicePath_, 'activate']);
+    }
     PageManager.closeOverlay();
   };
 
   DetailsInternetPage.setDetails = function() {
-    var data = $('connection-state').data;
-    var servicePath = data.servicePath;
-    if (data.type == 'WiFi') {
+    var detailsPage = DetailsInternetPage.getInstance();
+    var type = detailsPage.type_;
+    var servicePath = detailsPage.servicePath_;
+    if (type == 'WiFi') {
       sendCheckedIfEnabled(servicePath, 'setPreferNetwork',
                            $('prefer-network-wifi'));
       sendCheckedIfEnabled(servicePath, 'setAutoConnect',
                            $('auto-connect-network-wifi'));
-    } else if (data.type == 'Wimax') {
+    } else if (type == 'Wimax') {
       sendCheckedIfEnabled(servicePath, 'setAutoConnect',
                            $('auto-connect-network-wimax'));
-    } else if (data.type == 'Cellular') {
+    } else if (type == 'Cellular') {
       sendCheckedIfEnabled(servicePath, 'setAutoConnect',
                            $('auto-connect-network-cellular'));
-    } else if (data.type == 'VPN') {
+    } else if (type == 'VPN') {
       chrome.send('setServerHostname',
                   [servicePath,
                    $('inet-server-hostname').value]);
@@ -878,108 +999,44 @@ cr.define('options.internet', function() {
     }
   };
 
-  DetailsInternetPage.updateConnectionButtonVisibilty = function(onc) {
-    if (onc.type == 'Ethernet') {
-      // Ethernet can never be connected or disconnected and can always be
-      // configured (e.g. to set security).
-      $('details-internet-login').hidden = true;
-      $('details-internet-disconnect').hidden = true;
-      $('details-internet-configure').hidden = false;
-      return;
-    }
-
-    var connectState = onc.getActiveValue('ConnectionState');
-    if (connectState == 'NotConnected') {
-      $('details-internet-login').hidden = false;
-      // Connecting to an unconfigured network might trigger certificate
-      // installation UI. Until that gets handled here, always enable the
-      // Connect button.
-      $('details-internet-login').disabled = false;
-      $('details-internet-disconnect').hidden = true;
-    } else {
-      $('details-internet-login').hidden = true;
-      $('details-internet-disconnect').hidden = false;
-    }
-
-    var connectable = onc.getActiveValue('Connectable');
-    if (connectState != 'Connected' &&
-        (!connectable || this.hasSecurity ||
-        (onc.type == 'Wimax' || onc.type == 'VPN'))) {
-      $('details-internet-configure').hidden = false;
-    } else {
-      $('details-internet-configure').hidden = true;
-    }
-  };
-
-  DetailsInternetPage.populateHeader = function(detailsPage, onc) {
-    $('network-details-title').textContent = onc.getTranslatedValue('Name');
-    var connectionState = onc.getActiveValue('ConnectionState');
-    var connectionStateString = onc.getTranslatedValue('ConnectionState');
-    detailsPage.connected = connectionState == 'Connected';
-    $('network-details-subtitle-status').textContent = connectionStateString;
-    var typeKey;
-    if (onc.type == 'Ethernet')
-      typeKey = 'ethernetTitle';
-    else if (onc.type == 'WiFi')
-      typeKey = 'wifiTitle';
-    else if (onc.type == 'Wimax')
-      typeKey = 'wimaxTitle';
-    else if (onc.type == 'Cellular')
-      typeKey = 'cellularTitle';
-    else if (onc.type == 'VPN')
-      typeKey = 'vpnTitle';
-    else
-      typeKey = null;
-    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;
-    }
-  };
-
   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)
+    if (update.servicePath != detailsPage.servicePath_)
       return;
 
     // Update our cached data object.
-    updateDataObject(data, update);
-    var onc = new OncData(data);
+    var onc = detailsPage.onc_;
+    onc.updateData(update);
 
-    this.populateHeader(detailsPage, onc);
+    detailsPage.populateHeader_();
 
     var connectionState = onc.getActiveValue('ConnectionState');
     var connectionStateString = onc.getTranslatedValue('ConnectionState');
-    detailsPage.deviceConnected = data.deviceConnected;
+    if ('deviceConnected' in update)
+      detailsPage.deviceConnected = update.deviceConnected;
     detailsPage.connected = connectionState == 'Connected';
     $('connection-state').textContent = connectionStateString;
 
-    this.updateConnectionButtonVisibilty(onc);
+    detailsPage.updateConnectionButtonVisibilty_();
 
-    if (onc.type == 'WiFi') {
+    var type = detailsPage.type_;
+    if (type == 'WiFi') {
       $('wifi-connection-state').textContent = connectionStateString;
-    } else if (onc.type == 'Wimax') {
+    } else if (type == 'Wimax') {
       $('wimax-connection-state').textContent = connectionStateString;
-    } else if (onc.type == 'Cellular') {
+    } else if (type == 'Cellular') {
       $('activation-state').textContent =
           onc.getTranslatedValue('Cellular.ActivationState');
-      $('buyplan-details').hidden = !data.showBuyButton;
-      $('view-account-details').hidden = !data.showViewAccountButton;
-
-      $('activate-details').hidden = !data.showActivateButton;
-      if (data.showActivateButton)
+      if ('showBuyButton' in update)

[pneubeck (no reviews), 2014/09/08 09:18:00]

not sure whether that has the intended effect:

if showBuyButton _was_ true
and changes to not existent, then you will not hide the element anymore.

same for below.

[stevenjb, 2014/09/08 19:43:12]

Ugh. So, this variable isn't even used, I'll remove it, but the logic for the
others is not what one would expect - not present does in fact mean 'false'.
Since we want to remove that logic from the C++ code anyway, I'm not going to
fix that, I'll just restore the JS logic.

+        $('buyplan-details').hidden = !update.showBuyButton;
+      if ('showViewAccountButton' in update)
+        $('view-account-details').hidden = !update.showViewAccountButton;
+      if ('showActivateButton' in update)
+        $('activate-details').hidden = !update.showActivateButton;
+      if ('showActivateButton' in update && update.showActivateButton)
         $('details-internet-login').hidden = true;

[pneubeck (no reviews), 2014/09/08 09:18:00]

this seems to have been unreliable already in the old code?
i.e. if 'details-internet-login' was already hidden and should now be visible,
it will not appear.

IIUC, this seems to be a common problem in this function: there are several
guarded assignments to 'hidden'...

[stevenjb, 2014/09/08 19:43:12]

Yeah, I fixed this one here and in showDetailedInfo.

[stevenjb, 2014/09/08 21:31:18]

Actually, it turns out this overrides 'details-internet-login.hidden' set in
updateConnectionButtonVisibilty_() iff showActivateButton is true, so the
original logic was correct.

 
       if (detailsPage.gsm) {
@@ -989,28 +1046,24 @@ cr.define('options.internet', function() {
         $('change-pin').hidden = !lockEnabled;
       }
     }
-
-    $('connection-state').data = data;
-    $('connection-state').onc = onc;
   };
 
   DetailsInternetPage.showDetailedInfo = function(data) {
-    var detailsPage = DetailsInternetPage.getInstance();
-
     var onc = new OncData(data);
-    data.type = onc.type;
 
-    this.populateHeader(detailsPage, onc);
+    var detailsPage = DetailsInternetPage.getInstance();
+    detailsPage.onc_ = onc;
+    var type = onc.getActiveValue('Type');
+    detailsPage.type_ = type;
+    detailsPage.servicePath_ = data.servicePath;
 
-    // TODO(stevenjb): Find a more appropriate place to cache data.
-    $('connection-state').data = data;
-    $('connection-state').onc = onc;
+    detailsPage.populateHeader_();
 
     $('buyplan-details').hidden = true;
     $('activate-details').hidden = true;
     $('view-account-details').hidden = true;
 
-    this.updateConnectionButtonVisibilty(onc);
+    detailsPage.updateConnectionButtonVisibilty_();
 
     $('web-proxy-auto-discovery').hidden = true;
 
@@ -1021,7 +1074,7 @@ cr.define('options.internet', function() {
     // Only show proxy for remembered networks.
     if (data.remembered) {
       detailsPage.showProxy = true;
-      chrome.send('selectNetwork', [data.servicePath]);
+      chrome.send('selectNetwork', [detailsPage.servicePath_]);
     } else {
       detailsPage.showProxy = false;
     }
@@ -1038,12 +1091,12 @@ cr.define('options.internet', function() {
 
     var inetNameServersString;
 
-    if ('IPConfigs' in data) {
-      var ipconfigList = onc.getActiveValue('IPConfigs');
+    var ipconfigList = onc.getActiveValue('IPConfigs');
+    if (Array.isArray(ipconfigList)) {
       for (var i = 0; i < ipconfigList.length; ++i) {
         var ipconfig = ipconfigList[i];
-        var type = ipconfig['Type'];
-        if (type != 'IPv4') {
+        var ipType = ipconfig['Type'];
+        if (ipType != 'IPv4') {
           // TODO(stevenjb): Handle IPv6 properties.
           continue;
         }
@@ -1073,56 +1126,52 @@ cr.define('options.internet', function() {
     // Override the "automatic" values with the real saved DHCP values,
     // if they are set.
     var savedNameServersString;
-    if ('SavedIPConfig' in data) {
-      var savedIpAddress = onc.getActiveValue('SavedIPConfig.IPAddress');
-      if (savedIpAddress != undefined) {
-        inetAddress.automatic = savedIpAddress;
-        inetAddress.value = savedIpAddress;
-      }
-      var savedPrefix = onc.getActiveValue('SavedIPConfig.RoutingPrefix');
-      if (savedPrefix != undefined) {
-        var savedNetmask = PrefixLengthToNetmask(savedPrefix);
-        inetNetmask.automatic = savedNetmask;
-        inetNetmask.value = savedNetmask;
-      }
-      var savedGateway = onc.getActiveValue('SavedIPConfig.Gateway');
-      if (savedGateway != undefined) {
-        inetGateway.automatic = savedGateway;
-        inetGateway.value = savedGateway;
-      }
-      var savedNameServers = onc.getActiveValue('SavedIPConfig.NameServers');
-      if (savedNameServers) {
-        savedNameServers = savedNameServers.sort();
-        savedNameServersString = savedNameServers.join(',');
-      }
+    var savedIpAddress = onc.getActiveValue('SavedIPConfig.IPAddress');
+    if (savedIpAddress != undefined) {
+      inetAddress.automatic = savedIpAddress;
+      inetAddress.value = savedIpAddress;
+    }
+    var savedPrefix = onc.getActiveValue('SavedIPConfig.RoutingPrefix');
+    if (savedPrefix != undefined) {
+      var savedNetmask = PrefixLengthToNetmask(savedPrefix);
+      inetNetmask.automatic = savedNetmask;
+      inetNetmask.value = savedNetmask;
+    }
+    var savedGateway = onc.getActiveValue('SavedIPConfig.Gateway');
+    if (savedGateway != undefined) {
+      inetGateway.automatic = savedGateway;
+      inetGateway.value = savedGateway;
+    }
+    var savedNameServers = onc.getActiveValue('SavedIPConfig.NameServers');
+    if (savedNameServers) {
+      savedNameServers = savedNameServers.sort();
+      savedNameServersString = savedNameServers.join(',');
     }
 
     var ipAutoConfig = 'automatic';
 
     var staticNameServersString;
-    if ('StaticIPConfig' in data) {
-      var staticIpAddress = onc.getActiveValue('StaticIPConfig.IPAddress');
-      if (staticIpAddress != undefined) {
-        ipAutoConfig = 'user';
-        inetAddress.user = staticIpAddress;
-        inetAddress.value = staticIpAddress;
-      }
-      var staticPrefix = onc.getActiveValue('StaticIPConfig.RoutingPrefix');
-      if (staticPrefix != undefined) {
-        var staticNetmask = PrefixLengthToNetmask(staticPrefix);
-        inetNetmask.user = staticNetmask;
-        inetNetmask.value = staticNetmask;
-      }
-      var staticGateway = onc.getActiveValue('StaticIPConfig.Gateway');
-      if (staticGateway != undefined) {
-        inetGateway.user = staticGateway;
-        inetGateway.value = staticGateway;
-      }
-      var staticNameServers = onc.getActiveValue('StaticIPConfig.NameServers');
-      if (staticNameServers) {
-        staticNameServers = staticNameServers.sort();
-        staticNameServersString = staticNameServers.join(',');
-      }
+    var staticIpAddress = onc.getActiveValue('StaticIPConfig.IPAddress');
+    if (staticIpAddress != undefined) {
+      ipAutoConfig = 'user';
+      inetAddress.user = staticIpAddress;
+      inetAddress.value = staticIpAddress;
+    }
+    var staticPrefix = onc.getActiveValue('StaticIPConfig.RoutingPrefix');
+    if (staticPrefix != undefined) {
+      var staticNetmask = PrefixLengthToNetmask(staticPrefix);
+      inetNetmask.user = staticNetmask;
+      inetNetmask.value = staticNetmask;
+    }
+    var staticGateway = onc.getActiveValue('StaticIPConfig.Gateway');
+    if (staticGateway != undefined) {
+      inetGateway.user = staticGateway;
+      inetGateway.value = staticGateway;
+    }
+    var staticNameServers = onc.getActiveValue('StaticIPConfig.NameServers');
+    if (staticNameServers) {
+      staticNameServers = staticNameServers.sort();
+      staticNameServersString = staticNameServers.join(',');
     }
 
     $('ip-automatic-configuration-checkbox').checked =
@@ -1191,16 +1240,14 @@ cr.define('options.internet', function() {
 
     // Signal strength as percentage (for WiFi and Wimax).
     var signalStrength;
-    if (onc.type == 'WiFi' || onc.type == 'Wimax') {
-      signalStrength = onc.getActiveValue(onc.type + '.SignalStrength');
-    }
+    if (type == 'WiFi' || type == 'Wimax')
+      signalStrength = onc.getActiveValue(type + '.SignalStrength');
     if (!signalStrength)
       signalStrength = 0;
     var strengthFormat = loadTimeData.getString('inetSignalStrengthFormat');
     var strengthString = strengthFormat.replace('$1', signalStrength);
 
-    detailsPage.type = onc.type;
-    if (onc.type == 'WiFi') {
+    if (type == 'WiFi') {
       OptionsPage.showTab($('wifi-network-nav-tab'));
       detailsPage.gsm = false;
       detailsPage.shared = data.shared;
@@ -1231,7 +1278,7 @@ cr.define('options.internet', function() {
           onc.getActiveValue('AutoConnect');
       $('auto-connect-network-wifi').disabled = !data.remembered;
       detailsPage.hasSecurity = security != undefined;
-    } else if (onc.type == 'Wimax') {
+    } else if (type == 'Wimax') {
       OptionsPage.showTab($('wimax-network-nav-tab'));
       detailsPage.gsm = false;
       detailsPage.shared = data.shared;
@@ -1244,7 +1291,7 @@ cr.define('options.internet', function() {
       var identity = onc.getActiveValue('Wimax.EAP.Identity');
       setOrHideParent('wimax-eap-identity', identity);
       $('wimax-signal-strength').textContent = strengthString;
-    } else if (onc.type == 'Cellular') {
+    } else if (type == 'Cellular') {
       OptionsPage.showTab($('cellular-conn-nav-tab'));
       if (data.showCarrierSelect && data.currentCarrierIndex != -1) {
         var carrierSelector = $('select-carrier');
@@ -1307,58 +1354,7 @@ cr.define('options.internet', function() {
       if (detailsPage.gsm) {
         $('iccid').textContent = onc.getActiveValue('Cellular.ICCID');
         $('imsi').textContent = onc.getActiveValue('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 activeApn = onc.getActiveValue('Cellular.APN.AccessPointName');
-        var activeUsername = onc.getActiveValue('Cellular.APN.Username');
-        var activePassword = onc.getActiveValue('Cellular.APN.Password');
-        var lastGoodApn =
-            onc.getActiveValue('Cellular.LastGoodAPN.AccessPointName');
-        var lastGoodUsername =
-            onc.getActiveValue('Cellular.LastGoodAPN.Username');
-        var lastGoodPassword =
-            onc.getActiveValue('Cellular.LastGoodAPN.Password');
-        var apnList = onc.getActiveValue('Cellular.APNList');
-        for (var i = 0; i < apnList.length; i++) {
-          var apnDict = apnList[i];
-          var option = document.createElement('option');
-          var localizedName = apnDict['LocalizedName'];
-          var name = localizedName ? localizedName : apnDict['Name'];
-          var accessPointName = apnDict['AccessPointName'];
-          option.textContent =
-              name ? (name + ' (' + accessPointName + ')') : accessPointName;
-          option.value = i;
-          // If this matches the active Apn, or LastGoodApn, set it as the
-          // selected Apn.
-          if ((activeApn == accessPointName &&
-               activeUsername == apnDict['Username'] &&
-               activePassword == apnDict['Password']) ||
-              (!activeApn &&
-               lastGoodApn == accessPointName &&
-               lastGoodUsername == apnDict['Username'] &&
-               lastGoodPassword == apnDict['Password'])) {
-            data.selectedApn = i;
-          }
-          // Insert new option before "other" option.
-          apnSelector.add(option, otherOption);
-        }
-        if (data.selectedApn == -1 && activeApn) {
-          var option = document.createElement('option');
-          option.textContent = activeApn;
-          option.value = -1;
-          apnSelector.add(option, otherOption);
-          data.selectedApn = apnSelector.length - 2;
-          data.userApnIndex = data.selectedApn;
-        }
-        apnSelector.selectedIndex = data.selectedApn;
-        updateHidden('.apn-list-view', false);
-        updateHidden('.apn-details-view', true);
+        detailsPage.initializeApnList_(onc);
         var lockEnabled =
             onc.getActiveValue('Cellular.SIMLockStatus.LockEnabled');
         $('sim-card-lock-enabled').checked = lockEnabled;
@@ -1374,7 +1370,7 @@ cr.define('options.internet', function() {
       if (data.showActivateButton) {
         $('details-internet-login').hidden = true;
       }
-    } else if (onc.type == 'VPN') {
+    } else if (type == 'VPN') {
       OptionsPage.showTab($('vpn-nav-tab'));
       detailsPage.gsm = false;
       $('inet-service-name').textContent = networkName;
@@ -1428,7 +1424,7 @@ cr.define('options.internet', function() {
       }
     }
 
-    detailsPage.updateControls();
+    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.