Some additional network settings cleanup

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 4878ec4346d7188c2ad34cabe250c336c2951e70..ccb5ce67ef5718b07bfcc247a44fa84000c43921 100644
--- a/chrome/browser/resources/options/chromeos/internet_detail.js
+++ b/chrome/browser/resources/options/chromeos/internet_detail.js
@@ -86,7 +86,7 @@ cr.define('options.internet', function() {
    * @param {string} prefixLength The ONC routing prefix length.
    * @return {string} The corresponding netmask.
    */
-  function PrefixLengthToNetmask(prefixLength) {
+  function prefixLengthToNetmask(prefixLength) {
     // Return the empty string for invalid inputs.
     if (prefixLength < 0 || prefixLength > 32)
       return '';
@@ -117,9 +117,15 @@ cr.define('options.internet', function() {
    * @constructor
    */
   function DetailsInternetPage() {
+    // Cached Apn properties
     this.userApnIndex_ = -1;
     this.selectedApnIndex_ = -1;
     this.userApn_ = {};
+    // We show the Proxy configuration tab for remembered networks and when
+    // configuring a proxy from the login screen.
+    this.showProxy_ = false;
+    // TODO(stevenjb): Use networkingPrivate.getNetworks to set this.
+    this.deviceConnected_ = false;
     Page.call(this, 'detailsInternetPage', null, 'details-internet-page');
   }
 
@@ -380,40 +386,45 @@ cr.define('options.internet', function() {
      */
     updateControls: function() {
       var onc = this.onc_;
+      if (onc == undefined)
+        return;  // May get called from a pref update before initialized.
 
       // 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.
+      // TODO(stevenjb): Support IP configuration (and improve the display)
+      // for non connected networks.
+
       var connected = onc.getActiveValue('ConnectionState') == 'Connected';
-      $('ipconfig-section').hidden = !connected && this.deviceConnected;
-      $('ipconfig-dns-section').hidden = !connected && this.deviceConnected;
+      $('ipconfig-section').hidden = !connected && this.deviceConnected_;
+      $('ipconfig-dns-section').hidden = !connected && this.deviceConnected_;
+
+      var isCellular = this.type_ == 'Cellular';
+      var isWiFi = this.type_ == 'WiFi';
+      var isWimax = this.type_ == 'Wimax';
+      var isVPN = this.type_ == 'VPN';

[armansito, 2014/09/11 17:01:15]

Maybe add instance methods for these, which you can then reuse in updateDetails_
below. (e.g. if (this.isCellular())).

[stevenjb, 2014/09/11 17:22:33]

Hm. I really only pulled these out for brevity in some of the code here;
this.isCellular() would defeat that purpose. I'd rather just eliminate these if
they are confusing.

[armansito, 2014/09/11 17:28:12]

Up to you then. I'd just be consistent here and below, i.e. either always use
the explicit comparison or have the temporaries.

[stevenjb, 2014/09/11 17:38:23]

Removed.

 
       // Network type related.
-      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);
+      updateHidden('#details-internet-page .cellular-details', !isCellular);
+      updateHidden('#details-internet-page .wifi-details', !isWiFi);
+      updateHidden('#details-internet-page .wimax-details', !isWimax);
+      updateHidden('#details-internet-page .vpn-details', !isVPN);
 
-      // Cellular
+      updateHidden('#details-internet-page .proxy-details', !this.showProxy_);
 
-      // 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);
+      // Cellular
+      if (isCellular) {
+        // Hide gsm/cdma specific elements.
+        if (onc.getActiveValue('Cellular.Family') == 'GSM')
+          updateHidden('#details-internet-page .cdma-only', true);
+        else
+          updateHidden('#details-internet-page .gsm-only', true);
+      }
 
       // Wifi
 
       // Hide network tab for VPN.
-      updateHidden('#details-internet-page .network-details',
-                   this.type_ == 'VPN');
+      updateHidden('#details-internet-page .network-details', isVPN);
 
       // Password and shared.
       var source = onc.getSource();
@@ -587,6 +598,44 @@ cr.define('options.internet', function() {
       }
     },
 
+    updateDetails_: function(data) {
+      var onc = this.onc_;
+
+      if ('deviceConnected' in data)
+        this.deviceConnected_ = data.deviceConnected;
+
+      var connectionStateString = onc.getTranslatedValue('ConnectionState');
+      $('connection-state').textContent = connectionStateString;
+
+      var type = this.type_;
+      var showViewAccount = false;
+      var showActivate = false;
+      if (type == 'WiFi') {
+        $('wifi-connection-state').textContent = connectionStateString;
+      } else if (type == 'Wimax') {
+        $('wimax-connection-state').textContent = connectionStateString;
+      } else if (type == 'Cellular') {
+        $('activation-state').textContent =
+            onc.getTranslatedValue('Cellular.ActivationState');
+        if (onc.getActiveValue('Cellular.Family') == 'GSM') {
+          var lockEnabled =
+              onc.getActiveValue('Cellular.SIMLockStatus.LockEnabled');
+          $('sim-card-lock-enabled').checked = lockEnabled;
+          $('change-pin').hidden = !lockEnabled;
+        }
+        showViewAccount = data.showViewAccountButton;
+        var activationState = onc.getActiveValue('Cellular.ActivationState');
+        showActivate = activationState == 'NotActivated' ||
+            activationState == 'PartiallyActivated';
+      }
+
+      $('view-account-details').hidden = !showViewAccount;
+      $('activate-details').hidden = !showActivate;
+      // If activation is not complete, hide the login button.
+      if (showActivate)
+        $('details-internet-login').hidden = true;
+    },
+
     populateHeader_: function() {
       var onc = this.onc_;
 
@@ -594,6 +643,7 @@ cr.define('options.internet', function() {
       var connectionState = onc.getActiveValue('ConnectionState');
       var connectionStateString = onc.getTranslatedValue('ConnectionState');
       $('network-details-subtitle-status').textContent = connectionStateString;
+
       var typeKey;
       var type = this.type_;
       if (type == 'Ethernet')
@@ -836,7 +886,7 @@ cr.define('options.internet', function() {
     $('activate-details').hidden = true;
     $('view-account-details').hidden = true;
     $('web-proxy-auto-discovery').hidden = true;
-    detailsPage.showProxy = true;
+    detailsPage.showProxy_ = true;
     updateHidden('#internet-tab', true);
     updateHidden('#details-tab-strip', true);
     updateHidden('#details-internet-page .action-area', true);
@@ -1019,36 +1069,8 @@ cr.define('options.internet', function() {
     onc.updateData(update);
 
     detailsPage.populateHeader_();
-
-    var connectionState = onc.getActiveValue('ConnectionState');
-    var connectionStateString = onc.getTranslatedValue('ConnectionState');
-    if ('deviceConnected' in update)
-      detailsPage.deviceConnected = update.deviceConnected;
-    $('connection-state').textContent = connectionStateString;
-
     detailsPage.updateConnectionButtonVisibilty_();
-
-    var type = detailsPage.type_;
-    if (type == 'WiFi') {
-      $('wifi-connection-state').textContent = connectionStateString;
-    } else if (type == 'Wimax') {
-      $('wimax-connection-state').textContent = connectionStateString;
-    } else if (type == 'Cellular') {
-      $('activation-state').textContent =
-          onc.getTranslatedValue('Cellular.ActivationState');
-      // These properties are only defined if they are true.
-      $('view-account-details').hidden = !update.showViewAccountButton;
-      $('activate-details').hidden = !update.showActivateButton;
-      if (update.showActivateButton)
-        $('details-internet-login').hidden = true;
-
-      if (detailsPage.gsm) {
-        var lockEnabled =
-            onc.getActiveValue('Cellular.SIMLockStatus.LockEnabled');
-        $('sim-card-lock-enabled').checked = lockEnabled;
-        $('change-pin').hidden = !lockEnabled;
-      }
-    }
+    detailsPage.updateDetails_(update);
   };
 
   DetailsInternetPage.showDetailedInfo = function(data) {
@@ -1061,27 +1083,24 @@ cr.define('options.internet', function() {
     detailsPage.servicePath_ = data.servicePath;
 
     detailsPage.populateHeader_();
-
-    $('activate-details').hidden = true;
-    $('view-account-details').hidden = true;
-
     detailsPage.updateConnectionButtonVisibilty_();
+    detailsPage.updateDetails_(data);
 
-    $('web-proxy-auto-discovery').hidden = true;
-
-    detailsPage.deviceConnected = data.deviceConnected;
+    // TODO(stevenjb): Some of the setup below should be moved to
+    // updateDetails_() so that updates are reflected in the UI.
 
     // Only show proxy for remembered networks.
     var remembered = onc.getSource() != 'None';
     if (remembered) {
-      detailsPage.showProxy = true;
+      detailsPage.showProxy_ = true;
+      // Inform Chrome which network to use for proxy configuration.
       chrome.send('selectNetwork', [detailsPage.servicePath_]);
     } else {
-      detailsPage.showProxy = false;
+      detailsPage.showProxy_ = false;
     }
 
-    var connectionStateString = onc.getTranslatedValue('ConnectionState');
-    $('connection-state').textContent = connectionStateString;
+    $('web-proxy-auto-discovery').hidden = true;
+
     var restricted = onc.getActiveValue('RestrictedConnectivity');
     var restrictedString = loadTimeData.getString(
         restricted ? 'restrictedYes' : 'restrictedNo');
@@ -1104,7 +1123,7 @@ cr.define('options.internet', function() {
         var address = ipconfig['IPAddress'];
         inetAddress.automatic = address;
         inetAddress.value = address;
-        var netmask = PrefixLengthToNetmask(ipconfig['RoutingPrefix']);
+        var netmask = prefixLengthToNetmask(ipconfig['RoutingPrefix']);
         inetNetmask.automatic = netmask;
         inetNetmask.value = netmask;
         var gateway = ipconfig['Gateway'];
@@ -1134,7 +1153,7 @@ cr.define('options.internet', function() {
     }
     var savedPrefix = onc.getActiveValue('SavedIPConfig.RoutingPrefix');
     if (savedPrefix != undefined) {
-      var savedNetmask = PrefixLengthToNetmask(savedPrefix);
+      var savedNetmask = prefixLengthToNetmask(savedPrefix);
       inetNetmask.automatic = savedNetmask;
       inetNetmask.value = savedNetmask;
     }
@@ -1160,7 +1179,7 @@ cr.define('options.internet', function() {
     }
     var staticPrefix = onc.getActiveValue('StaticIPConfig.RoutingPrefix');
     if (staticPrefix != undefined) {
-      var staticNetmask = PrefixLengthToNetmask(staticPrefix);
+      var staticNetmask = prefixLengthToNetmask(staticPrefix);
       inetNetmask.user = staticNetmask;
       inetNetmask.value = staticNetmask;
     }
@@ -1250,8 +1269,6 @@ cr.define('options.internet', function() {
 
     if (type == 'WiFi') {
       OptionsPage.showTab($('wifi-network-nav-tab'));
-      detailsPage.gsm = false;
-      $('wifi-connection-state').textContent = connectionStateString;
       $('wifi-restricted-connectivity').textContent = restrictedString;
       var ssid = onc.getActiveValue('WiFi.SSID');
       $('wifi-ssid').textContent = ssid ? ssid : networkName;
@@ -1278,8 +1295,6 @@ cr.define('options.internet', function() {
       $('auto-connect-network-wifi').disabled = !remembered;
     } else if (type == 'Wimax') {
       OptionsPage.showTab($('wimax-network-nav-tab'));
-      detailsPage.gsm = false;
-      $('wimax-connection-state').textContent = connectionStateString;
       $('wimax-restricted-connectivity').textContent = restrictedString;
       $('auto-connect-network-wimax').checked =
           onc.getActiveValue('AutoConnect');
@@ -1305,8 +1320,6 @@ cr.define('options.internet', function() {
 
       $('network-technology').textContent =
           onc.getActiveValue('Cellular.NetworkTechnology');
-      $('activation-state').textContent =
-          onc.getTranslatedValue('Cellular.ActivationState');
       $('roaming-state').textContent =
           onc.getTranslatedValue('Cellular.RoamingState');
       $('cellular-restricted-connectivity').textContent = restrictedString;
@@ -1345,28 +1358,16 @@ cr.define('options.internet', function() {
       setOrHideParent('min', onc.getActiveValue('Cellular.MIN'));
       setOrHideParent('prl-version', onc.getActiveValue('Cellular.PRLVersion'));
 
-      var family = onc.getActiveValue('Cellular.Family');
-      detailsPage.gsm = family == 'GSM';
-      if (detailsPage.gsm) {
+      if (onc.getActiveValue('Cellular.Family') == 'GSM') {
         $('iccid').textContent = onc.getActiveValue('Cellular.ICCID');
         $('imsi').textContent = onc.getActiveValue('Cellular.IMSI');
         detailsPage.initializeApnList_(onc);
-        var lockEnabled =
-            onc.getActiveValue('Cellular.SIMLockStatus.LockEnabled');
-        $('sim-card-lock-enabled').checked = lockEnabled;
-        $('change-pin').hidden = !lockEnabled;
       }
       $('auto-connect-network-cellular').checked =
           onc.getActiveValue('AutoConnect');
       $('auto-connect-network-cellular').disabled = false;
-
-      $('view-account-details').hidden = !data.showViewAccountButton;
-      $('activate-details').hidden = !data.showActivateButton;
-      if (data.showActivateButton)
-        $('details-internet-login').hidden = true;
     } else if (type == 'VPN') {
       OptionsPage.showTab($('vpn-nav-tab'));
-      detailsPage.gsm = false;
       $('inet-service-name').textContent = networkName;
       $('inet-provider-type').textContent =
           onc.getTranslatedValue('VPN.Type');