[CrOS PhoneHub] CrosNetworkConfig observers notified on scan completed.

Previously, the enable hotspot button stays "connecting" indefinitely.
This is because CrosNetworkConfig::OnDeviceListChanged() is not called
immediately when a tether scan state completes. This CL corrects that.

Fixed: 1146611
Bug: 1106937
Change-Id: Ieafc1fdd0f03343861aaf6dca5233b863dd69cbe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531898
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826409}

chromeos/components/phonehub/tether_controller_impl_unittest.cc

@@ -516,20 +516,17 @@ TEST_F(TetherControllerImplTest, AttemptConnectFeatureOffNoNetwork) {
 
   // Tether is scanning, connection should be connecting still.
   SetTetherScanState(true);
-  EnableTetherDevice();
   EXPECT_EQ(GetStatus(), TetherController::Status::kConnecting);
   DisconnectTetherDevice();
 
   // Tether stops scanning, attempt ends and connection should become
   // unavailable.
   SetTetherScanState(false);
-  EnableTetherDevice();
   EXPECT_EQ(GetNumObserverScanFailed(), 1U);
   EXPECT_EQ(GetStatus(), TetherController::Status::kConnectionUnavailable);
 
   // Tether starts scanning after connection attempt ended.
   SetTetherScanState(true);
-  EnableTetherDevice();
   EXPECT_EQ(GetNumObserverScanFailed(), 1U);
   EXPECT_EQ(GetStatus(), TetherController::Status::kConnectionUnavailable);
 }

chromeos/network/network_state_handler.cc

@@ -339,6 +339,9 @@ void NetworkStateHandler::SetTetherScanState(bool is_scanning) {
   if (was_scanning && !is_scanning) {
     // If a scan was in progress but has completed, notify observers.
     NotifyScanCompleted(tether_device_state);
+  } else if (!was_scanning && is_scanning) {
+    // If a scan was started, notify observers.
+    NotifyScanStarted(tether_device_state);
   }
 }
 
@@ -2033,6 +2036,13 @@ void NetworkStateHandler::NotifyScanCompleted(const DeviceState* device) {
     observer.ScanCompleted(device);
 }
 
+void NetworkStateHandler::NotifyScanStarted(const DeviceState* device) {
+  SCOPED_NET_LOG_IF_SLOW();
+  NET_LOG(EVENT) << "NOTIFY: ScanStarted for: " << device->path();
+  for (auto& observer : observers_)
+    observer.ScanStarted(device);
+}
+
 void NetworkStateHandler::LogPropertyUpdated(const ManagedState* state,
                                              const std::string& key,
                                              const base::Value& value) {

chromeos/network/network_state_handler.h

@@ -571,6 +571,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandler
   // Called to ask observers to scan for networks.
   void NotifyScanRequested(const NetworkTypePattern& type);
 
+  // Called whenever Device.Scanning state transitions to true.
+  void NotifyScanStarted(const DeviceState* device);
+
   // Called whenever Device.Scanning state transitions to false.
   void NotifyScanCompleted(const DeviceState* device);
 

chromeos/network/network_state_handler_observer.cc

@@ -36,6 +36,8 @@ void NetworkStateHandlerObserver::DevicePropertiesUpdated(
 void NetworkStateHandlerObserver::ScanRequested(
     const NetworkTypePattern& type) {}
 
+void NetworkStateHandlerObserver::ScanStarted(const DeviceState* device) {}
+
 void NetworkStateHandlerObserver::ScanCompleted(const DeviceState* device) {}
 
 void NetworkStateHandlerObserver::HostnameChanged(const std::string& hostname) {

chromeos/network/network_state_handler_observer.h

@@ -66,6 +66,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandlerObserver {
   // A scan for a given network type has been requested.
   virtual void ScanRequested(const NetworkTypePattern& type);
 
+  // A scan for |device| started.
+  virtual void ScanStarted(const DeviceState* device);
+
   // A scan for |device| completed.
   virtual void ScanCompleted(const DeviceState* device);
 

chromeos/services/network_config/cros_network_config.cc

@@ -2628,6 +2628,14 @@ void CrosNetworkConfig::DevicePropertiesUpdated(const DeviceState* device) {
   DeviceListChanged();
 }
 
+void CrosNetworkConfig::ScanCompleted(const DeviceState* device) {
+  DeviceListChanged();
+}
+
+void CrosNetworkConfig::ScanStarted(const DeviceState* device) {
+  DeviceListChanged();
+}
+
 void CrosNetworkConfig::OnShuttingDown() {
   if (network_state_handler_->HasObserver(this))
     network_state_handler_->RemoveObserver(this, FROM_HERE);

chromeos/services/network_config/cros_network_config.h

@@ -150,6 +150,8 @@ class CrosNetworkConfig : public mojom::CrosNetworkConfig,
   void NetworkPropertiesUpdated(const NetworkState* network) override;
   void DevicePropertiesUpdated(const DeviceState* device) override;
   void OnShuttingDown() override;
+  void ScanStarted(const DeviceState* device) override;
+  void ScanCompleted(const DeviceState* device) override;
 
   // NetworkCertificateHandler::Observer
   void OnCertificatesChanged() override;

chromeos/services/network_config/cros_network_config_unittest.cc

@@ -1315,6 +1315,24 @@ TEST_F(CrosNetworkConfigTest, DeviceListChanged) {
   // disabling state, next when it's actually disabled, and lastly when
   // Device::available_managed_network_path_ changes.
   EXPECT_EQ(3, observer()->device_state_list_changed());
+
+  // Enable Tethering
+  helper().network_state_handler()->SetTetherTechnologyState(
+      NetworkStateHandler::TechnologyState::TECHNOLOGY_ENABLED);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(4, observer()->device_state_list_changed());
+
+  // Tests that observers are notified of device state list change
+  // when a tether scan begins for a device.
+  helper().network_state_handler()->SetTetherScanState(true);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(5, observer()->device_state_list_changed());
+
+  // Tests that observers are notified of device state list change
+  // when a tether scan completes.
+  helper().network_state_handler()->SetTetherScanState(false);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(6, observer()->device_state_list_changed());
 }
 
 TEST_F(CrosNetworkConfigTest, ActiveNetworksChanged) {