MD Settings: Internet: Make detail page layout more robust

chrome/browser/resources/settings/internet_page/internet_detail_page.js

Index: chrome/browser/resources/settings/internet_page/internet_detail_page.js
diff --git a/chrome/browser/resources/settings/internet_page/internet_detail_page.js b/chrome/browser/resources/settings/internet_page/internet_detail_page.js
index 312cb6946ad3699239e0744a4a478af66e06b7aa..a5284864534e870b52a186ed80e1c0b85868d327 100644
--- a/chrome/browser/resources/settings/internet_page/internet_detail_page.js
+++ b/chrome/browser/resources/settings/internet_page/internet_detail_page.js
@@ -163,6 +163,16 @@ Polymer({
       console.error('No guid specified for page:' + route);
       this.close_();
     }
+    // Set basic networkProperties until the are loaded.

[michaelpg, 2017/01/31 21:47:46]

the => they

[stevenjb, 2017/01/31 22:23:55]

Done.

+    var type = /** @type {!chrome.networkingPrivate.NetworkType} */ (
+                   queryParams.get('type')) ||
+        CrOnc.Type.WI_FI;
+    this.networkProperties = {
+      GUID: this.guid,
+      Type: type,
+      ConnectionState: CrOnc.ConnectionState.NOT_CONNECTED,
+      Name: {Active: queryParams.get('name') || type},

[michaelpg, 2017/01/31 21:47:46]

nit: var name = ... (as above)

the... dependency chain of these properties requires too much effort to follow.

[stevenjb, 2017/01/31 22:23:55]

Done.

+    };
     this.getNetworkDetails_();
   },
 
@@ -344,6 +354,16 @@ Polymer({
 
   /**
    * @param {!CrOnc.NetworkProperties} networkProperties
+   * @return {boolean}
+   * @private
+   */
+  isCellular_: function(networkProperties) {
+    return networkProperties.Type == CrOnc.Type.CELLULAR &&
+        !!networkProperties.Cellular;
+  },
+
+  /**
+   * @param {!CrOnc.NetworkProperties} networkProperties
    * @param {!chrome.networkingPrivate.GlobalPolicy} globalPolicy
    * @return {boolean}
    * @private
@@ -397,7 +417,7 @@ Polymer({
    * @private
    */
   showActivate_: function(networkProperties) {
-    if (networkProperties.Type != CrOnc.Type.CELLULAR)
+    if (!this.isCellular_(networkProperties))
       return false;
     var activation = networkProperties.Cellular.ActivationState;
     return activation == CrOnc.ActivationState.NOT_ACTIVATED ||
@@ -430,12 +450,9 @@ Polymer({
    * @private
    */
   showViewAccount_: function(networkProperties) {
-    // Show either the 'Activate' or the 'View Account' button.
-    if (this.showActivate_(networkProperties))
-      return false;
-
-    if (networkProperties.Type != CrOnc.Type.CELLULAR ||
-        !networkProperties.Cellular) {
+    // Show either the 'Activate' or the 'View Account' button (Cellular only).
+    if (!this.isCellular_(networkProperties) ||
+        this.showActivate_(networkProperties)) {
       return false;
     }
 
@@ -722,12 +739,12 @@ Polymer({
    */
   getInfoFields_: function() {
     /** @type {!Array<string>} */ var fields = [];
-    if (this.networkProperties.Type == CrOnc.Type.CELLULAR) {
+    var type = this.networkProperties.Type;
+    if (type == CrOnc.Type.CELLULAR && !!this.networkProperties.Cellular) {

[michaelpg, 2017/01/31 21:47:46]

maybe put this logic in a helper function, e.g. isNetworkType?

[stevenjb, 2017/01/31 22:23:55]

Well, I have a helper, but we've already extracted type so this is more
efficient.

       fields.push(
           'Cellular.ActivationState', 'Cellular.RoamingState',
           'RestrictedConnectivity', 'Cellular.ServingOperator.Name');
-    }
-    if (this.networkProperties.Type == CrOnc.Type.VPN) {
+    } else if (type == CrOnc.Type.VPN && !!this.networkProperties.VPN) {
       var vpnType = CrOnc.getActiveValue(this.networkProperties.VPN.Type);
       if (vpnType == 'ThirdPartyVPN') {
         fields.push('VPN.ThirdPartyVPN.ProviderName');
@@ -738,10 +755,9 @@ Polymer({
         else if (vpnType == 'L2TP-IPsec')
           fields.push('VPN.L2TP.Username');
       }
-    }
-    if (this.networkProperties.Type == CrOnc.Type.WI_FI)
+    } else if (type == CrOnc.Type.WI_FI) {
       fields.push('RestrictedConnectivity');
-    if (this.networkProperties.Type == CrOnc.Type.WI_MAX) {
+    } else if (type == CrOnc.Type.WI_MAX) {
       fields.push('RestrictedConnectivity', 'WiMAX.EAP.Identity');
     }
     return fields;
@@ -754,18 +770,18 @@ Polymer({
   getAdvancedFields_: function() {
     /** @type {!Array<string>} */ var fields = [];
     fields.push('MacAddress');
-    if (this.networkProperties.Type == CrOnc.Type.CELLULAR) {
+    var type = this.networkProperties.Type;
+    if (type == CrOnc.Type.CELLULAR && !!this.networkProperties.Cellular) {
       fields.push(
           'Cellular.Carrier', 'Cellular.Family', 'Cellular.NetworkTechnology',
           'Cellular.ServingOperator.Code');
-    }
-    if (this.networkProperties.Type == CrOnc.Type.WI_FI) {
+    } else if (type == CrOnc.Type.WI_FI) {
       fields.push(
           'WiFi.SSID', 'WiFi.BSSID', 'WiFi.Security', 'WiFi.SignalStrength',
           'WiFi.Frequency');
-    }
-    if (this.networkProperties.Type == CrOnc.Type.WI_MAX)
+    } else if (type == CrOnc.Type.WI_MAX) {
       fields.push('WiFi.SignalStrength');
+    }
     return fields;
   },