Use mojo API in NetworkSectionHeaderView

This also converts NetworkListView and VPNListView to use
TrayNetworkStateModel.

Bug: 862420
Change-Id: I54efc6823a59331c3d011f7af40d1fb05d27a20c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626256
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663094}

ash/system/network/network_list.cc

@@ -12,11 +12,13 @@
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
+#include "ash/system/model/system_tray_model.h"
 #include "ash/system/network/network_icon.h"
 #include "ash/system/network/network_icon_animation.h"
 #include "ash/system/network/network_info.h"
 #include "ash/system/network/network_section_header_view.h"
 #include "ash/system/network/network_state_list_detailed_view.h"
+#include "ash/system/network/tray_network_state_model.h"
 #include "ash/system/power/power_status.h"
 #include "ash/system/tray/hover_highlight_view.h"
 #include "ash/system/tray/tray_info_label.h"
@@ -73,9 +75,8 @@ bool IsSecondaryUser() {
 
 NetworkListView::NetworkListView(DetailedViewDelegate* delegate,
                                  LoginStatus login)
-    : NetworkStateListDetailedView(delegate, LIST_TYPE_NETWORK, login) {
-  BindCrosNetworkConfig();
-}
+    : NetworkStateListDetailedView(delegate, LIST_TYPE_NETWORK, login),
+      model_(Shell::Get()->system_tray_model()->network_state_model()) {}
 
 NetworkListView::~NetworkListView() {
   network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this);
@@ -83,9 +84,11 @@ NetworkListView::~NetworkListView() {
 
 void NetworkListView::UpdateNetworkList() {
   CHECK(scroll_content());
-  DCHECK(cros_network_config_ptr_);
-  cros_network_config_ptr_->GetDeviceStateList(base::BindOnce(
-      &NetworkListView::OnGetDeviceStateList, base::Unretained(this)));
+  model_->cros_network_config()->GetNetworkStateList(
+      NetworkFilter::New(FilterType::kVisible, NetworkType::kAll,
+                         chromeos::network_config::mojom::kNoLimit),
+      base::BindOnce(&NetworkListView::OnGetNetworkStateList,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 bool NetworkListView::IsNetworkEntry(views::View* view,
@@ -102,34 +105,6 @@ const char* NetworkListView::GetClassName() const {
   return "NetworkListView";
 }
 
-void NetworkListView::BindCrosNetworkConfig() {
-  // Ensure binding is reset in case this is called after a failure.
-  cros_network_config_ptr_.reset();
-
-  service_manager::Connector* connector = Shell::Get()->connector();
-  if (!connector)
-    return;
-  connector->BindInterface(chromeos::network_config::mojom::kServiceName,
-                           &cros_network_config_ptr_);
-
-  // If the connection is lost (e.g. due to a crash), attempt to rebind it.
-  cros_network_config_ptr_.set_connection_error_handler(base::BindOnce(
-      &NetworkListView::BindCrosNetworkConfig, base::Unretained(this)));
-}
-
-void NetworkListView::OnGetDeviceStateList(
-    std::vector<DeviceStatePropertiesPtr> devices) {
-  device_states_.clear();
-  for (auto& device : devices)
-    device_states_[device->type] = device->state;
-
-  cros_network_config_ptr_->GetNetworkStateList(
-      NetworkFilter::New(FilterType::kVisible, NetworkType::kAll,
-                         chromeos::network_config::mojom::kNoLimit),
-      base::BindOnce(&NetworkListView::OnGetNetworkStateList,
-                     base::Unretained(this)));
-}
-
 void NetworkListView::OnGetNetworkStateList(
     std::vector<NetworkStatePropertiesPtr> networks) {
   // |network_list_| contains all the info and is going to be cleared and
@@ -142,6 +117,8 @@ void NetworkListView::OnGetNetworkStateList(
   network_list_.clear();
   vpn_connected_ = false;
   wifi_has_networks_ = false;
+  mobile_has_networks_ = false;
+  tether_has_networks_ = false;
   for (auto& network : networks) {
     ConnectionStateType connection_state = network->connection_state;
     if (network->type == NetworkType::kVPN) {
@@ -154,12 +131,20 @@ void NetworkListView::OnGetNetworkStateList(
         network->cellular ? network->cellular->activation_state
                           : ActivationStateType::kUnknown;
     if (network->type == NetworkType::kCellular &&
-        GetDeviceState(NetworkType::kCellular) != DeviceStateType::kEnabled &&
+        model_->GetDeviceState(NetworkType::kCellular) !=
+            DeviceStateType::kEnabled &&
         activation_state == ActivationStateType::kNoService) {
       continue;
     }
     if (network->type == NetworkType::kWiFi)
       wifi_has_networks_ = true;
+    // Real (non 'default') Cellular networks are always connectable.
+    if (network->type == NetworkType::kCellular && network->connectable)
+      mobile_has_networks_ = true;
+    if (network->type == NetworkType::kTether) {
+      mobile_has_networks_ = true;
+      tether_has_networks_ = true;
+    }
 
     auto info = std::make_unique<NetworkInfo>(network->guid);
     info->label = network_icon::GetLabelForNetworkList(network.get());
@@ -271,7 +256,8 @@ NetworkListView::UpdateNetworkListEntries() {
     std::unique_ptr<std::set<std::string>> new_cellular_guids =
         UpdateNetworkChildren(NetworkType::kMobile, index);
     int mobile_status_message =
-        mobile_header_view_->UpdateToggleAndGetStatusMessage();
+        mobile_header_view_->UpdateToggleAndGetStatusMessage(
+            mobile_has_networks_, tether_has_networks_);
     // |mobile_status_message| may be zero. Passing zero to UpdateInfoLabel
     // clears the label.
     UpdateInfoLabel(mobile_status_message, index, &mobile_status_message_);
@@ -285,7 +271,7 @@ NetworkListView::UpdateNetworkListEntries() {
     wifi_header_view_ = new WifiSectionHeaderView();
 
   bool wifi_enabled =
-      GetDeviceState(NetworkType::kWiFi) == DeviceStateType::kEnabled;
+      model_->GetDeviceState(NetworkType::kWiFi) == DeviceStateType::kEnabled;
   index = UpdateNetworkSectionHeader(NetworkType::kWiFi, wifi_enabled, index,
                                      wifi_header_view_, &wifi_separator_view_);
 
@@ -323,10 +309,12 @@ NetworkListView::UpdateNetworkListEntries() {
 
 bool NetworkListView::ShouldMobileDataSectionBeShown() {
   // The section should always be shown if Cellular networks are available.
-  if (GetDeviceState(NetworkType::kCellular) != DeviceStateType::kUnavailable)
+  if (model_->GetDeviceState(NetworkType::kCellular) !=
+      DeviceStateType::kUnavailable) {
     return true;
+  }
 
-  DeviceStateType tether_state = GetDeviceState(NetworkType::kTether);
+  DeviceStateType tether_state = model_->GetDeviceState(NetworkType::kTether);
   // Hide the section if both Cellular and Tether are UNAVAILABLE.
   if (tether_state == DeviceStateType::kUnavailable)
     return false;
@@ -568,12 +556,5 @@ TriView* NetworkListView::CreateConnectionWarning() {
   return connection_warning;
 }
 
-DeviceStateType NetworkListView::GetDeviceState(NetworkType type) const {
-  auto iter = device_states_.find(type);
-  if (iter == device_states_.end())
-    return DeviceStateType::kUnavailable;
-  return iter->second;
-}
-
 }  // namespace tray
 }  // namespace ash

ash/system/network/network_list.h

@@ -16,7 +16,8 @@
 #include "ash/system/network/network_state_list_detailed_view.h"
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
-#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom-forward.h"
 
 namespace views {
 class Separator;
@@ -27,6 +28,7 @@ namespace ash {
 class HoverHighlightView;
 class TrayInfoLabel;
 class TriView;
+class TrayNetworkStateModel;
 
 namespace tray {
 class NetworkSectionHeaderView;
@@ -49,10 +51,6 @@ class NetworkListView : public NetworkStateListDetailedView,
   const char* GetClassName() const override;
 
  private:
-  void BindCrosNetworkConfig();
-  void OnGetDeviceStateList(
-      std::vector<chromeos::network_config::mojom::DeviceStatePropertiesPtr>
-          devices);
   void OnGetNetworkStateList(
       std::vector<chromeos::network_config::mojom::NetworkStatePropertiesPtr>
           networks);
@@ -131,8 +129,7 @@ class NetworkListView : public NetworkStateListDetailedView,
   // otherwise false.
   bool NeedUpdateViewForNetwork(const NetworkInfo& info) const;
 
-  chromeos::network_config::mojom::DeviceStateType GetDeviceState(
-      chromeos::network_config::mojom::NetworkType type) const;
+  TrayNetworkStateModel* model_;
 
   bool needs_relayout_ = false;
 
@@ -145,14 +142,10 @@ class NetworkListView : public NetworkStateListDetailedView,
   views::Separator* wifi_separator_view_ = nullptr;
   TriView* connection_warning_ = nullptr;
 
-  chromeos::network_config::mojom::CrosNetworkConfigPtr
-      cros_network_config_ptr_;
-
-  base::flat_map<chromeos::network_config::mojom::NetworkType,
-                 chromeos::network_config::mojom::DeviceStateType>
-      device_states_;
   bool vpn_connected_ = false;
   bool wifi_has_networks_ = false;
+  bool tether_has_networks_ = false;
+  bool mobile_has_networks_ = false;
 
   // An owned list of network info.
   std::vector<std::unique_ptr<NetworkInfo>> network_list_;
@@ -168,6 +161,8 @@ class NetworkListView : public NetworkStateListDetailedView,
   using NetworkInfoMap = std::map<std::string, std::unique_ptr<NetworkInfo>>;
   NetworkInfoMap last_network_info_map_;
 
+  base::WeakPtrFactory<NetworkListView> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(NetworkListView);
 };
 

ash/system/network/network_section_header_view.h

@@ -10,12 +10,14 @@
 #include "ash/system/tray/tri_view.h"
 #include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
-#include "chromeos/network/network_state_handler.h"
 #include "ui/views/controls/button/toggle_button.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/view.h"
 
 namespace ash {
+
+class TrayNetworkStateModel;
+
 namespace tray {
 
 // A header row for sections in network detailed view which contains a title and
@@ -48,6 +50,7 @@ class NetworkSectionHeaderView : public views::View,
   // enabled/disable their respective technology, for example.
   virtual void OnToggleToggled(bool is_on) = 0;
 
+  TrayNetworkStateModel* model() { return model_; }
   TriView* container() const { return container_; }
 
   // views::View:
@@ -64,6 +67,8 @@ class NetworkSectionHeaderView : public views::View,
   // accessible text on the section header toggle button.
   const int title_id_;
 
+  TrayNetworkStateModel* model_;
+
   // View containing header row views, including title, toggle, and extra
   // buttons.
   TriView* container_ = nullptr;
@@ -89,7 +94,8 @@ class MobileSectionHeaderView : public NetworkSectionHeaderView {
   // Updates mobile toggle state and returns the id of the status message
   // that should be shown while connecting to a network. Returns zero when no
   // message should be shown.
-  int UpdateToggleAndGetStatusMessage();
+  int UpdateToggleAndGetStatusMessage(bool mobile_has_networks,
+                                      bool tether_has_networks);
 
   // views::View:
   const char* GetClassName() const override;

ash/system/network/tray_network_state_model.h

@@ -55,6 +55,10 @@ class ASH_EXPORT TrayNetworkStateModel
       chromeos::network_config::mojom::NetworkType type,
       bool enabled);
 
+  chromeos::network_config::mojom::CrosNetworkConfig* cros_network_config() {
+    return cros_network_config_ptr_.get();
+  }
+
   const chromeos::network_config::mojom::NetworkStateProperties*
   default_network() const {
     return default_network_.get();

ash/system/network/vpn_list_view.cc

@@ -18,6 +18,7 @@
 #include "ash/system/network/network_icon.h"
 #include "ash/system/network/network_icon_animation.h"
 #include "ash/system/network/network_icon_animation_observer.h"
+#include "ash/system/network/tray_network_state_model.h"
 #include "ash/system/network/vpn_list.h"
 #include "ash/system/tray/hover_highlight_view.h"
 #include "ash/system/tray/system_menu_button.h"
@@ -191,6 +192,8 @@ class VPNListNetworkEntry : public HoverHighlightView,
 
   views::LabelButton* disconnect_button_ = nullptr;
 
+  base::WeakPtrFactory<VPNListNetworkEntry> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(VPNListNetworkEntry);
 };
 
@@ -205,13 +208,9 @@ VPNListNetworkEntry::~VPNListNetworkEntry() {
 }
 
 void VPNListNetworkEntry::NetworkIconChanged() {
-  chromeos::network_config::mojom::CrosNetworkConfig* cros_network_config =
-      owner_->cros_network_config();
-  if (!cros_network_config)
-    return;
-  cros_network_config->GetNetworkState(
+  owner_->model()->cros_network_config()->GetNetworkState(
       guid_, base::BindOnce(&VPNListNetworkEntry::OnGetNetworkState,
-                            base::Unretained(this)));
+                            weak_ptr_factory_.GetWeakPtr()));
 }
 
 void VPNListNetworkEntry::ButtonPressed(Button* sender,
@@ -270,9 +269,9 @@ void VPNListNetworkEntry::UpdateFromNetworkState(
 }  // namespace
 
 VPNListView::VPNListView(DetailedViewDelegate* delegate, LoginStatus login)
-    : NetworkStateListDetailedView(delegate, LIST_TYPE_VPN, login) {
+    : NetworkStateListDetailedView(delegate, LIST_TYPE_VPN, login),
+      model_(Shell::Get()->system_tray_model()->network_state_model()) {
   Shell::Get()->vpn_list()->AddObserver(this);
-  BindCrosNetworkConfig();
 }
 
 VPNListView::~VPNListView() {
@@ -280,12 +279,11 @@ VPNListView::~VPNListView() {
 }
 
 void VPNListView::UpdateNetworkList() {
-  DCHECK(cros_network_config_ptr_);
-  cros_network_config_ptr_->GetNetworkStateList(
+  model_->cros_network_config()->GetNetworkStateList(
       NetworkFilter::New(FilterType::kVisible, NetworkType::kVPN,
                          chromeos::network_config::mojom::kNoLimit),
       base::BindOnce(&VPNListView::OnGetNetworkStateList,
-                     base::Unretained(this)));
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void VPNListView::OnGetNetworkStateList(NetworkStateList networks) {
@@ -372,21 +370,6 @@ const char* VPNListView::GetClassName() const {
   return "VPNListView";
 }
 
-void VPNListView::BindCrosNetworkConfig() {
-  // Ensure binding is reset in case this is called after a failure.
-  cros_network_config_ptr_.reset();
-
-  service_manager::Connector* connector = Shell::Get()->connector();
-  if (!connector)
-    return;
-  connector->BindInterface(chromeos::network_config::mojom::kServiceName,
-                           &cros_network_config_ptr_);
-
-  // If the connection is lost (e.g. due to a crash), attempt to rebind it.
-  cros_network_config_ptr_.set_connection_error_handler(base::BindOnce(
-      &VPNListView::BindCrosNetworkConfig, base::Unretained(this)));
-}
-
 void VPNListView::AddNetwork(const NetworkStateProperties* network) {
   views::View* entry(new VPNListNetworkEntry(this, network));
   scroll_content()->AddChildView(entry);

ash/system/network/vpn_list_view.h

@@ -11,7 +11,8 @@
 #include "ash/system/network/network_state_list_detailed_view.h"
 #include "ash/system/network/vpn_list.h"
 #include "base/macros.h"
-#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom-forward.h"
 
 class PrefRegistrySimple;
 
@@ -20,6 +21,9 @@ class View;
 }
 
 namespace ash {
+
+class TrayNetworkStateModel;
+
 namespace tray {
 
 // A list of VPN providers and networks that shows VPN providers and networks in
@@ -52,9 +56,7 @@ class VPNListView : public NetworkStateListDetailedView,
   // VpnList::Observer:
   void OnVPNProvidersChanged() override;
 
-  chromeos::network_config::mojom::CrosNetworkConfig* cros_network_config() {
-    return cros_network_config_ptr_.get();
-  }
+  TrayNetworkStateModel* model() { return model_; }
 
   // See Shell::RegisterProfilePrefs().
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
@@ -65,7 +67,6 @@ class VPNListView : public NetworkStateListDetailedView,
  private:
   using NetworkStateList =
       std::vector<chromeos::network_config::mojom::NetworkStatePropertiesPtr>;
-  void BindCrosNetworkConfig();
   void OnGetNetworkStateList(NetworkStateList networks);
 
   // Adds a network to the list.
@@ -94,8 +95,7 @@ class VPNListView : public NetworkStateListDetailedView,
   // Adds all available VPN providers and networks to the list.
   void AddProvidersAndNetworks(const NetworkStateList& networks);
 
-  chromeos::network_config::mojom::CrosNetworkConfigPtr
-      cros_network_config_ptr_;
+  TrayNetworkStateModel* model_;
 
   // A mapping from each VPN provider's list entry to the provider.
   std::map<const views::View* const, VPNProvider> provider_view_map_;
@@ -107,6 +107,8 @@ class VPNListView : public NetworkStateListDetailedView,
   // the topmost entry).
   bool list_empty_ = true;
 
+  base::WeakPtrFactory<VPNListView> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(VPNListView);
 };
 

chromeos/services/network_config/cros_network_config.cc

@@ -118,8 +118,7 @@ mojom::DeviceStateType GetMojoDeviceStateType(
 }
 
 mojom::NetworkStatePropertiesPtr NetworkStateToMojo(const NetworkState* network,
-                                                    bool technology_enabled,
-                                                    const DeviceState* device) {
+                                                    bool technology_enabled) {
   mojom::NetworkType type = ShillTypeToMojo(network->type());
   if (type == mojom::NetworkType::kAll) {
     NET_LOG(ERROR) << "Unexpected network type: " << network->type()
@@ -161,10 +160,6 @@ mojom::NetworkStatePropertiesPtr NetworkStateToMojo(const NetworkState* network,
                                                 onc::kNetworkTechnologyTable);
       cellular->roaming = network->IndicateRoaming();
       cellular->signal_strength = network->signal_strength();
-      if (device) {
-        cellular->scanning = device->scanning();
-        cellular->sim_absent = device->IsSimAbsent();
-      }
       result->cellular = std::move(cellular);
       break;
     }
@@ -240,9 +235,8 @@ mojom::DeviceStatePropertiesPtr DeviceStateToMojo(
   result->state = technology_state;
   result->managed_network_available =
       !device->available_managed_network_path().empty();
-
+  result->sim_absent = device->IsSimAbsent();
   if (device->sim_present()) {
-    result->sim_present = true;
     auto sim_lock_status = mojom::SIMLockStatus::New();
     sim_lock_status->lock_type = device->sim_lock_type();
     sim_lock_status->lock_enabled = device->sim_lock_enabled();
@@ -422,6 +416,11 @@ void CrosNetworkConfig::ActiveNetworksChanged(
   });
 }
 
+void CrosNetworkConfig::ScanCompleted(const DeviceState* device) {
+  // Scanning state of device may have updated.
+  DeviceListChanged();
+}
+
 void CrosNetworkConfig::OnShuttingDown() {
   if (network_state_handler_->HasObserver(this))
     network_state_handler_->RemoveObserver(this, FROM_HERE);
@@ -433,9 +432,7 @@ mojom::NetworkStatePropertiesPtr CrosNetworkConfig::GetMojoNetworkState(
   bool technology_enabled = network->Matches(NetworkTypePattern::VPN()) ||
                             network_state_handler_->IsTechnologyEnabled(
                                 NetworkTypePattern::Primitive(network->type()));
-  return NetworkStateToMojo(
-      network, technology_enabled,
-      network_state_handler_->GetDeviceState(network->device_path()));
+  return NetworkStateToMojo(network, technology_enabled);
 }
 
 }  // namespace network_config

chromeos/services/network_config/cros_network_config.h

@@ -41,6 +41,7 @@ class CrosNetworkConfig : public mojom::CrosNetworkConfig,
   void DeviceListChanged() override;
   void ActiveNetworksChanged(
       const std::vector<const NetworkState*>& active_networks) override;
+  void ScanCompleted(const DeviceState* device) override;
   void OnShuttingDown() override;
 
  private:

chromeos/services/network_config/cros_network_config_unittest.cc

@@ -230,7 +230,7 @@ TEST_F(CrosNetworkConfigTest, GetDeviceStateList) {
         mojom::DeviceStateProperties* cellular = devices[2].get();
         EXPECT_EQ(mojom::NetworkType::kCellular, cellular->type);
         EXPECT_EQ(mojom::DeviceStateType::kEnabled, cellular->state);
-        EXPECT_TRUE(cellular->sim_present);
+        EXPECT_FALSE(cellular->sim_absent);
         ASSERT_TRUE(cellular->sim_lock_status);
         EXPECT_TRUE(cellular->sim_lock_status->lock_enabled);
         EXPECT_EQ(shill::kSIMLockPin, cellular->sim_lock_status->lock_type);

chromeos/services/network_config/public/mojom/cros_network_config.mojom

@@ -110,6 +110,8 @@ struct CaptivePortalProvider {
 
 // The SIM card lock status for Cellular networks.
 struct SIMLockStatus {
+  // True when a SIM is required and not present.
+  bool sim_absent = false;
   // The status of SIM lock. Possible values are 'sim-pin', 'sim-puk' or empty.
   string lock_type;
   // Whether SIM lock is enabled.
@@ -125,10 +127,6 @@ struct CellularStateProperties {
   string network_technology;
   // True when in a roaming state and the provider is not always roaming.
   bool roaming = false;
-  // True while performing a scan for cellular netowkrs.
-  bool scanning = false;
-  // True when a SIM is required and not present.
-  bool sim_absent = false;
   // The signal strength of the cellular connection.
   int32 signal_strength;
 };
@@ -202,12 +200,12 @@ struct NetworkStateProperties {
 };
 
 struct DeviceStateProperties {
-  // Set if the device is enabled. True if the device is currently scanning.
+  // Set if the device is currently scanning.
   bool scanning = false;
-  // The SIM lock status if Type = Cellular and SIMPresent = True.
+  // The SIM lock status if Type = Cellular and a SIM is present.
   SIMLockStatus? sim_lock_status;
-  // Set to the SIM present state if the device type is Cellular.
-  bool sim_present = false;
+  // True when a SIM is required and not present.
+  bool sim_absent = false;
   DeviceStateType state;
   NetworkType type;
   // Whether or not any managed networks are available.