Settings: Network: Merge Tether networks into Mobile subpage

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

Index: chrome/browser/resources/settings/internet_page/network_summary.js
diff --git a/chrome/browser/resources/settings/internet_page/network_summary.js b/chrome/browser/resources/settings/internet_page/network_summary.js
index 3ef2ccb9a78bf25f947da6434f5e25bb35ffc856..9872db213ccc393417ab969bd0f28105ddfb0290 100644
--- a/chrome/browser/resources/settings/internet_page/network_summary.js
+++ b/chrome/browser/resources/settings/internet_page/network_summary.js
@@ -311,7 +311,19 @@ Polymer({
       var device = newDeviceStates[type];
       if (!device)

[Kyle Horimoto, 2017/06/01 20:15:21]

Is this if() necessary? It seems like we should always have a DeviceState, even
if it is unavailable.

As a corollary, would newDeviceStates[CELLULAR] below ever be falsey? Should we
be checking the state instead?

[stevenjb, 2017/06/01 21:55:03]

Incorrect. We have no "unavailable" device.State, instead we just do not provide
a device. (If there is no cellular or wimax modem, we do not burden the UI with
extra empty data).

(The reason we have this test at all is so that the UI can iterate over all
possible device types in the order it wants to present them, rather than relying
on whatever order the model provides or the arbitrary order of a map).

I'll add a small comment here.

         continue;
-      var state = activeNetworkStatesByType.get(type) || {GUID: '', Type: type};
+      // If both 'Tether' and 'Cellular' technoligies exist, merge the network
+      // lists and do not add an active network for 'Tether' so that there is
+      // only one 'Mobile data' section / subpage.
+      if (type == CrOnc.Type.TETHER && newDeviceStates[CrOnc.Type.CELLULAR]) {
+        newNetworkStateLists[CrOnc.Type.CELLULAR] =
+            newNetworkStateLists[CrOnc.Type.CELLULAR].concat(
+                newNetworkStateLists[CrOnc.Type.TETHER]);
+        continue;
+      }
+
+      // Note: The active state for 'Cellular' may be a Tether network if both
+      // types are enabled but no Cellular network exists (edge case).
+      var state = this.getActiveStateForType_(activeNetworkStatesByType, type);
       if (state.Source === undefined &&
           device.State == CrOnc.DeviceState.PROHIBITED) {
         // Prohibited technologies are enforced by the device policy.
@@ -326,4 +338,19 @@ Polymer({
     // Set activeNetworkStates last to rebuild the dom-repeat.
     this.activeNetworkStates_ = newActiveNetworkStates;
   },
+
+  /**
+   * Returns the active network state for |type| or a default network state.
+   * If there is no 'Celular' network, return the active 'Tether' network if

[Kyle Horimoto, 2017/06/01 20:15:21]

Cellular

[stevenjb, 2017/06/01 21:55:03]

Done.

+   * any since the two types are represented by the same section / subpage.
+   * @param {!Map<string, !CrOnc.NetworkStateProperties>} activeStatesByType
+   * @param {string} type
+   * @return {!CrOnc.NetworkStateProperties|undefined}
+   */
+  getActiveStateForType_: function(activeStatesByType, type) {
+    var activeState = activeStatesByType.get(type);
+    if (!activeState && type == CrOnc.Type.CELLULAR)
+      activeState = activeStatesByType.get(CrOnc.Type.TETHER);
+    return activeState || {GUID: '', Type: type};
+  },
 });