Merged Tether and cellular network types in System Tray.

ash/system/network/network_list.cc

Index: ash/system/network/network_list.cc
diff --git a/ash/system/network/network_list.cc b/ash/system/network/network_list.cc
index b462bbdfad167379057cdc55cb69b81658cda050..0b0f24a4d9cedfc4cefcafee433c38f2d483da89 100644
--- a/ash/system/network/network_list.cc
+++ b/ash/system/network/network_list.cc
@@ -191,31 +191,22 @@ class CellularHeaderRowView : public NetworkListView::SectionHeaderRowView {
   void OnToggleToggled(bool is_on) override {
     NetworkStateHandler* handler =
         NetworkHandler::Get()->network_state_handler();
-    handler->SetTechnologyEnabled(NetworkTypePattern::Cellular(), is_on,
-                                  chromeos::network_handler::ErrorCallback());
+    // When cellular network technology is available on a device, Tether is

[khorimoto, 2017/05/30 22:35:15]

This comment is a bit vague. Can you explain it more thoroughly? Something like:

The mobile section contains both Cellular and Tether networks, though one or
both of these subtypes may be unavailable. If the toggle has been changed and
Cellular networks are available, set the enabled value accordingly. However, if
Cellular networks are unavailable when the toggle's value is changed, the toggle
should change the enabled value of Tether networks instead.

[lesliewatkins, 2017/05/31 00:43:27]

Done.

[Kyle Horimoto, 2017/05/31 20:13:59]

Not actually done.

[lesliewatkins, 2017/05/31 22:06:28]

Done.

+    // treated as a subset of Cellular technology. Otherwise, Tether has to
+    // be explicity enabled and disabled.
+    if (handler->IsTechnologyAvailable(NetworkTypePattern::Cellular())) {
+      handler->SetTechnologyEnabled(NetworkTypePattern::Cellular(), is_on,
+                                    chromeos::network_handler::ErrorCallback());
+    } else {
+      handler->SetTechnologyEnabled(NetworkTypePattern::Tether(), is_on,

[khorimoto, 2017/05/30 22:35:16]

Above this line, you should
DCHECK(handler->IsTechnologyAvailable(NetworkTypePattern::Tether())).

[lesliewatkins, 2017/05/31 00:43:27]

Done.

[Kyle Horimoto, 2017/05/31 20:13:58]

Not actually done.

[lesliewatkins, 2017/05/31 22:06:28]

Done.

+                                    chromeos::network_handler::ErrorCallback());
+    }
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(CellularHeaderRowView);
 };
 
-class TetherHeaderRowView : public NetworkListView::SectionHeaderRowView {
- public:
-  TetherHeaderRowView()
-      : SectionHeaderRowView(IDS_ASH_STATUS_TRAY_NETWORK_TETHER) {}
-
-  ~TetherHeaderRowView() override {}
-
-  const char* GetClassName() const override { return "TetherHeaderRowView"; }
-
- protected:
-  void OnToggleToggled(bool is_on) override {
-    // TODO (hansberry): Persist toggle to settings/preferences.
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TetherHeaderRowView);
-};
 
 class WifiHeaderRowView : public NetworkListView::SectionHeaderRowView {
  public:
@@ -298,10 +289,8 @@ NetworkListView::NetworkListView(SystemTrayItem* owner, LoginStatus login)
       no_wifi_networks_view_(nullptr),
       no_cellular_networks_view_(nullptr),
       cellular_header_view_(nullptr),
-      tether_header_view_(nullptr),
       wifi_header_view_(nullptr),
       cellular_separator_view_(nullptr),
-      tether_separator_view_(nullptr),
       wifi_separator_view_(nullptr),
       connection_warning_(nullptr) {}
 
@@ -431,10 +420,8 @@ void NetworkListView::UpdateNetworkIcons() {
     info->connecting = network->IsConnectingState();
     if (network->Matches(NetworkTypePattern::WiFi()))
       info->type = NetworkInfo::Type::WIFI;
-    else if (network->Matches(NetworkTypePattern::Cellular()))
-      info->type = NetworkInfo::Type::CELLULAR;
-    else if (network->Matches(NetworkTypePattern::Tether()))
-      info->type = NetworkInfo::Type::TETHER;
+    else if (network->Matches(NetworkTypePattern::Mobile()))
+      info->type = NetworkInfo::Type::MOBILE;
     if (prohibited_by_policy) {
       info->tooltip =
           l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_PROHIBITED);
@@ -515,11 +502,15 @@ NetworkListView::UpdateNetworkListEntries() {
       UpdateNetworkChildren(NetworkInfo::Type::UNKNOWN, index);
   index += new_guids->size();
 
-  if (handler->IsTechnologyAvailable(NetworkTypePattern::Cellular())) {
-    index = UpdateSectionHeaderRow(
-        NetworkTypePattern::Cellular(),
-        handler->IsTechnologyEnabled(NetworkTypePattern::Cellular()), index,
-        &cellular_header_view_, &cellular_separator_view_);
+  if (handler->IsTechnologyAvailable(NetworkTypePattern::Cellular()) ||
+      handler->IsTechnologyAvailable(NetworkTypePattern::Tether())) {
+    bool is_enabled =
+        handler->IsTechnologyEnabled(NetworkTypePattern::Cellular()) ||
+        handler->IsTechnologyEnabled(NetworkTypePattern::Tether());
+
+    index = UpdateSectionHeaderRow(NetworkTypePattern::Cellular(), is_enabled,
+                                   index, &cellular_header_view_,
+                                   &cellular_separator_view_);
   }
 
   // Cellular initializing.
@@ -533,31 +524,14 @@ NetworkListView::UpdateNetworkListEntries() {
   if (cellular_message_id)
     ++index;
 
-  // Add cellular networks.
+  // Add cellular and Tether networks.
+  // TODO (hansberry): Audit existing usage of NonVirtual and consider changing

[khorimoto, 2017/05/30 22:35:16]

We are using Mobile instead of NonVirtual now, so you can remove this TODO.

[lesliewatkins, 2017/05/31 16:41:05]

Done! Also closed the associated bug.

+  // it to include Tether. See crbug.com/693647.
   std::unique_ptr<std::set<std::string>> new_cellular_guids =
-      UpdateNetworkChildren(NetworkInfo::Type::CELLULAR, index);
+      UpdateNetworkChildren(NetworkInfo::Type::MOBILE, index);
   index += new_cellular_guids->size();
   new_guids->insert(new_cellular_guids->begin(), new_cellular_guids->end());
 
-  // TODO (hansberry): Audit existing usage of NonVirtual and consider changing
-  // it to include Tether. See crbug.com/693647.
-  if (handler->IsTechnologyAvailable(NetworkTypePattern::Tether())) {
-    index = UpdateSectionHeaderRow(
-        NetworkTypePattern::Tether(),
-        handler->IsTechnologyEnabled(NetworkTypePattern::Tether()), index,
-        &tether_header_view_, &tether_separator_view_);
-
-    // TODO (hansberry): Should a message similar to
-    // IDS_ASH_STATUS_TRAY_NO_CELLULAR_NETWORKS be shown if Tether technology is
-    // enabled but no networks are around?
-
-    // Add Tether networks.
-    std::unique_ptr<std::set<std::string>> new_tether_guids =
-        UpdateNetworkChildren(NetworkInfo::Type::TETHER, index);
-    index += new_tether_guids->size();
-    new_guids->insert(new_tether_guids->begin(), new_tether_guids->end());
-  }
-
   index = UpdateSectionHeaderRow(
       NetworkTypePattern::WiFi(),
       handler->IsTechnologyEnabled(NetworkTypePattern::WiFi()), index,
@@ -701,10 +675,8 @@ int NetworkListView::UpdateSectionHeaderRow(NetworkTypePattern pattern,
                                             SectionHeaderRowView** view,
                                             views::Separator** separator_view) {
   if (!*view) {
-    if (pattern.Equals(NetworkTypePattern::Cellular()))
+    if (pattern.MatchesPattern(NetworkTypePattern::Mobile()))
       *view = new CellularHeaderRowView();
-    else if (pattern.Equals(NetworkTypePattern::Tether()))
-      *view = new TetherHeaderRowView();
     else if (pattern.Equals(NetworkTypePattern::WiFi()))
       *view = new WifiHeaderRowView();
     else