MD Settings: Internet: Make detail page layout more robust

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

 // Copyright 2015 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.
 
 /**
  * @fileoverview
  * 'settings-internet-detail' is the settings subpage containing details
  * for a network.
  */
 (function() {
@@ skipping 145 matching lines @@
       this.networksChangedListener_ = this.onNetworksChangedEvent_.bind(this);
       this.networkingPrivate.onNetworksChanged.addListener(
           this.networksChangedListener_);
     }
     var queryParams = settings.getQueryParameters();
     this.guid = queryParams.get('guid') || '';
     if (!this.guid) {
       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_();
   },
 
   /** @private */
   close_: function() {
     // Delay navigating until the next render frame to allow other subpages to
     // load first.
     setTimeout(function() {
       settings.navigateTo(settings.Route.INTERNET);
     });
@@ skipping 161 matching lines @@
    * @return {boolean}
    * @private
    */
   isRememberedOrConnected_: function(networkProperties) {
     return this.isRemembered_(networkProperties) ||
         this.isConnectedState_(networkProperties);
   },
 
   /**
    * @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
    */
   connectNotAllowed_: function(networkProperties, globalPolicy) {
     return networkProperties.Type == CrOnc.Type.WI_FI &&
         !!globalPolicy.AllowOnlyPolicyNetworksToConnect &&
         !this.isPolicySource(networkProperties.Source);
   },
 
@@ skipping 33 matching lines @@
       return false;
     return this.isRemembered_(networkProperties);
   },
 
   /**
    * @param {!CrOnc.NetworkProperties} networkProperties
    * @return {boolean}
    * @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 ||
         activation == CrOnc.ActivationState.PARTIALLY_ACTIVATED;
   },
 
   /**
    * @param {!CrOnc.NetworkProperties} networkProperties
    * @param {!chrome.networkingPrivate.GlobalPolicy} globalPolicy
    * @return {boolean}
@@ skipping 12 matching lines @@
     }
     return this.isRemembered_(networkProperties);
   },
 
   /**
    * @param {!CrOnc.NetworkProperties} networkProperties
    * @return {boolean}
    * @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;
     }
 
     // Only show if online payment URL is provided or the carrier is Verizon.
     var carrier = CrOnc.getActiveValue(networkProperties.Cellular.Carrier);
     if (carrier != CARRIER_VERIZON) {
       var paymentPortal = networkProperties.Cellular.PaymentPortal;
       if (!paymentPortal || !paymentPortal.Url)
         return false;
     }
@@ skipping 266 matching lines @@
   hasInfoFields_: function() {
     return this.hasVisibleFields_(this.getInfoFields_());
   },
 
   /**
    * @return {!Array<string>} The fields to display in the info section.
    * @private
    */
   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');
       } else {
         fields.push('VPN.Host', 'VPN.Type');
         if (vpnType == 'OpenVPN')
           fields.push('VPN.OpenVPN.Username');
         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;
   },
 
   /**
    * @return {!Array<string>} The fields to display in the Advanced section.
    * @private
    */
   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');
+    } else if (type == CrOnc.Type.WI_MAX) {
+      fields.push('WiFi.SignalStrength');
     }
-    if (this.networkProperties.Type == CrOnc.Type.WI_MAX)
-      fields.push('WiFi.SignalStrength');
     return fields;
   },
 
   /**
    * @return {!Array<string>} The fields to display in the device section.
    * @private
    */
   getDeviceFields_: function() {
     /** @type {!Array<string>} */ var fields = [];
     if (this.networkProperties.Type == CrOnc.Type.CELLULAR) {
@@ skipping 87 matching lines @@
    */
   allPropertiesMatch_: function(curValue, newValue) {
     for (var key in newValue) {
       if (newValue[key] != curValue[key])
         return false;
     }
     return true;
   }
 });
 })();