Merged Tether and cellular network types in System Tray.

ash/system/network/network_list.cc

 // Copyright 2016 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.
 
 #include "ash/system/network/network_list.h"
 
 #include <memory>
 
 #include "ash/shell.h"
 #include "ash/shell_port.h"
@@ skipping 160 matching lines @@
   views::ToggleButton* toggle_;
 
   // TrayPopupItemStyle used to configure labels and buttons.
   std::unique_ptr<TrayPopupItemStyle> style_;
 
   DISALLOW_COPY_AND_ASSIGN(SectionHeaderRowView);
 };
 
 namespace {
 
 class CellularHeaderRowView : public NetworkListView::SectionHeaderRowView {

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

Isn't this called the Mobile section now? If so, you should update this class
name accordingly.

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

Done.

  public:
   CellularHeaderRowView()
       : SectionHeaderRowView(IDS_ASH_STATUS_TRAY_NETWORK_MOBILE) {}
 
   ~CellularHeaderRowView() override {}
 
   const char* GetClassName() const override { return "CellularHeaderRowView"; }
 
  protected:
   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:
   WifiHeaderRowView()
       : SectionHeaderRowView(IDS_ASH_STATUS_TRAY_NETWORK_WIFI),
         join_(nullptr) {}
 
   ~WifiHeaderRowView() override {}
 
   void SetIsOn(bool enabled) override {
@@ skipping 62 matching lines @@
 }  // namespace
 
 // NetworkListView:
 
 NetworkListView::NetworkListView(SystemTrayItem* owner, LoginStatus login)
     : NetworkStateListDetailedView(owner, LIST_TYPE_NETWORK, login),
       needs_relayout_(false),
       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) {}
 
 NetworkListView::~NetworkListView() {
   network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this);
 }
 
 void NetworkListView::UpdateNetworkList() {
   CHECK(scroll_content());
 
@@ skipping 109 matching lines @@
         network, network_icon::ICON_TYPE_MENU_LIST);
     info->image =
         network_icon::GetImageForNetwork(network, network_icon::ICON_TYPE_LIST);
     info->disable =
         (network->activation_state() == shill::kActivationStateActivating) ||
         prohibited_by_policy;
     info->connected = network->IsConnectedState();
     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);
     }
     if (!animating && network->IsConnectingState())
       animating = true;
   }
   if (animating)
     network_icon::NetworkIconAnimation::GetInstance()->AddObserver(this);
   else
@@ skipping 60 matching lines @@
     if (!connection_warning_)
       connection_warning_ = CreateConnectionWarning();
     PlaceViewAtIndex(connection_warning_, index++);
   }
 
   // First add high-priority networks (not Wi-Fi nor cellular).
   std::unique_ptr<std::set<std::string>> new_guids =
       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.
   int cellular_message_id = network_icon::GetCellularUninitializedMsg();
   if (!cellular_message_id &&
       handler->IsTechnologyEnabled(NetworkTypePattern::Mobile()) &&
       !handler->FirstNetworkByType(NetworkTypePattern::Mobile())) {
     cellular_message_id = IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS;
   }
   UpdateInfoLabel(cellular_message_id, index, &no_cellular_networks_view_);
   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,
       &wifi_header_view_, &wifi_separator_view_);
 
   // "Wifi Enabled / Disabled".
   int wifi_message_id = 0;
   if (network_list_.empty()) {
     wifi_message_id = handler->IsTechnologyEnabled(NetworkTypePattern::WiFi())
                           ? IDS_ASH_STATUS_TRAY_NETWORK_WIFI_ENABLED
@@ skipping 123 matching lines @@
   PlaceViewAtIndex(label, insertion_index);
   *label_ptr = label;
 }
 
 int NetworkListView::UpdateSectionHeaderRow(NetworkTypePattern pattern,
                                             bool enabled,
                                             int child_index,
                                             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
       NOTREACHED();
     (*view)->Init(enabled);
   }
   // Show or hide a separator above the header. The separator should only be
   // visible when the header row is not at the top of the list.
   if (child_index > 0) {
     if (!*separator_view)
@@ skipping 53 matching lines @@
       TriView::Container::CENTER, views::CreateEmptyBorder(gfx::Insets(
                                       0, 0, 0, kTrayPopupLabelRightPadding)));
 
   // Nothing to the right of the text.
   connection_warning->SetContainerVisible(TriView::Container::END, false);
   return connection_warning;
 }
 
 }  // namespace tray
 }  // namespace ash