Fix active profile handling in NearbySharingServiceImpl

- Moved check for active profile into InvalidateSurfaceState,
  so send/receive surface can still be registered without profile being
  active
- OnNearbyProcessStopped will InvalidateSurfaceState to potentially
  restart the flow if profile is still active

Bug: 1120087
Change-Id: I40cc43949a46808d2c090c1681d34e0450f62fd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2375289
Commit-Queue: Alex Chau <alexchau@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801474}

chrome/browser/nearby_sharing/mock_nearby_process_manager.h

@@ -29,6 +29,7 @@ class MockNearbyProcessManager : public NearbyProcessManager {
               (Profile * profile),
               (override));
   MOCK_METHOD(bool, IsActiveProfile, (Profile * profile), (const, override));
+  MOCK_METHOD(void, StopProcess, (Profile * profile), (override));
 };
 
 #endif  // CHROME_BROWSER_NEARBY_SHARING_MOCK_NEARBY_PROCESS_MANAGER_H_

chrome/browser/nearby_sharing/nearby_process_manager.h

@@ -101,7 +101,7 @@ class NearbyProcessManager : public ProfileManagerObserver {
   // Nearby Connections library if it should not be used right now. This will
   // not change the active profile and can be used to temporarily stop the
   // process (e.g. on screen lock) while keeping the active profile.
-  void StopProcess(Profile* profile);
+  virtual void StopProcess(Profile* profile);
 
   // ProfileManagerObserver:
   void OnProfileAdded(Profile* profile) override;

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -255,12 +255,6 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
 
   nearby_process_observer_.Add(process_manager_);
 
-  if (process_manager_->IsActiveProfile(profile_)) {
-    // TODO(crbug.com/1084576): Initialize NearbyConnectionsManager with
-    // NearbyConnectionsMojom from |process_manager|:
-    // process_manager_->GetOrStartNearbyConnections(profile_)
-  }
-
   settings_.AddSettingsObserver(settings_receiver_.BindNewPipeAndPassRemote());
 
   GetBluetoothAdapter();
@@ -285,12 +279,6 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::RegisterSendSurface(
   DCHECK(discovery_callback);
   DCHECK_NE(state, SendSurfaceState::kUnknown);
 
-  if (!process_manager_->IsActiveProfile(profile_)) {
-    NS_LOG(VERBOSE) << __func__
-                    << ": RegisterSendSurface failed, since profile not active";
-    return StatusCodes::kError;
-  }
-
   if (foreground_send_transfer_callbacks_.HasObserver(transfer_callback) ||
       background_send_transfer_callbacks_.HasObserver(transfer_callback)) {
     NS_LOG(VERBOSE) << __func__
@@ -398,13 +386,6 @@ NearbySharingServiceImpl::RegisterReceiveSurface(
   DCHECK(transfer_callback);
   DCHECK_NE(state, ReceiveSurfaceState::kUnknown);
 
-  if (!process_manager_->IsActiveProfile(profile_)) {
-    NS_LOG(VERBOSE)
-        << __func__
-        << ": registerReceiveSurface failed, since profile not active";
-    return StatusCodes::kError;
-  }
-
   if (is_sending_files_) {
     UnregisterReceiveSurface(transfer_callback);
     NS_LOG(VERBOSE)
@@ -586,6 +567,7 @@ void NearbySharingServiceImpl::OnNearbyProfileChanged(Profile* profile) {
   // TODO(crbug.com/1084576): Notify UI about the new active profile.
   NS_LOG(VERBOSE) << __func__ << ": Nearby profile changed to "
                   << process_manager_->IsActiveProfile(profile_);
+  InvalidateSurfaceState();
 }
 
 void NearbySharingServiceImpl::OnNearbyProcessStarted() {
@@ -594,10 +576,9 @@ void NearbySharingServiceImpl::OnNearbyProcessStarted() {
 }
 
 void NearbySharingServiceImpl::OnNearbyProcessStopped() {
-  if (process_manager_->IsActiveProfile(profile_)) {
-    // TODO(crbug.com/1084576): Check if process should be running and restart
-    // it after a delay.
-  }
+  InvalidateSurfaceState();
+  if (process_manager_->IsActiveProfile(profile_))
+    NS_LOG(VERBOSE) << __func__ << ": Nearby process stopped!";
 }
 
 void NearbySharingServiceImpl::OnIncomingConnection(
@@ -971,6 +952,35 @@ void NearbySharingServiceImpl::AdapterPoweredChanged(
 void NearbySharingServiceImpl::InvalidateSurfaceState() {
   InvalidateSendSurfaceState();
   InvalidateReceiveSurfaceState();
+  if (ShouldStopNearbyProcess()) {
+    NS_LOG(VERBOSE) << __func__ << ": Stopping process because it's not in use";
+    process_manager_->StopProcess(profile_);
+  }
+}
+
+bool NearbySharingServiceImpl::ShouldStopNearbyProcess() {
+  // Cannot stop process without being the active profile.
+  if (!process_manager_->IsActiveProfile(profile_))
+    return false;
+
+  // We're currently advertising.
+  if (advertising_power_level_ != PowerLevel::kUnknown)
+    return false;
+
+  // We're currently discovering.
+  if (is_scanning_)
+    return false;
+
+  // We're currently attempting to connect to a remote device.
+  if (is_connecting_)
+    return false;
+
+  // We're currently sending or receiving a file.
+  if (is_transferring_)
+    return false;
+
+  // We're not using NearbyConnections, should stop the process.
+  return true;
 }
 
 void NearbySharingServiceImpl::InvalidateReceiveSurfaceState() {
@@ -979,6 +989,13 @@ void NearbySharingServiceImpl::InvalidateReceiveSurfaceState() {
 }
 
 void NearbySharingServiceImpl::InvalidateAdvertisingState() {
+  if (!process_manager_->IsActiveProfile(profile_)) {
+    NS_LOG(VERBOSE) << __func__
+                    << ": Stopping advertising because profile not active";
+    StopAdvertising();
+    return;
+  }
+
   // Screen is off. Do no work.
   if (ui::CheckIdleStateIsLocked()) {
     StopAdvertising();
@@ -1123,6 +1140,13 @@ void NearbySharingServiceImpl::InvalidateSendSurfaceState() {
 }
 
 void NearbySharingServiceImpl::InvalidateScanningState() {
+  if (!process_manager_->IsActiveProfile(profile_)) {
+    NS_LOG(VERBOSE) << __func__
+                    << ": Stopping discovery because profile not active";
+    StopScanning();
+    return;
+  }
+
   // Screen is off. Do no work.
   if (ui::CheckIdleStateIsLocked()) {
     StopScanning();

chrome/browser/nearby_sharing/nearby_sharing_service_impl.h

@@ -151,11 +151,14 @@ class NearbySharingServiceImpl
   void AdapterPoweredChanged(device::BluetoothAdapter* adapter,
                              bool powered) override;
   void InvalidateSurfaceState();
+  bool ShouldStopNearbyProcess();
   void InvalidateSendSurfaceState();
   void InvalidateScanningState();
   void InvalidateReceiveSurfaceState();
   void InvalidateAdvertisingState();
   void StopAdvertising();
+  void StartScanning();
+  StatusCodes StopScanning();
 
   void OnTransferComplete();
   void OnTransferStarted(bool is_incoming);
@@ -184,10 +187,6 @@ class NearbySharingServiceImpl
       NearbyConnection& connection,
       sharing::nearby::ConnectionResponseFrame::Status reponse_status);
   void Fail(const ShareTarget& share_target, TransferMetadata::Status status);
-  void StartScanning(
-      base::Optional<ShareTargetDiscoveredCallback*> discovery_callback);
-  void StartScanning();
-  StatusCodes StopScanning();
   void OnIncomingAdvertisementDecoded(
       const std::string& endpoint_id,
       ShareTarget placeholder_share_target,

chrome/browser/nearby_sharing/nearby_sharing_service_impl_unittest.cc

@@ -59,6 +59,7 @@
 #include "ui/base/idle/scoped_set_idle_state.h"
 
 using ::testing::_;
+using testing::AtLeast;
 using testing::NiceMock;
 using testing::Return;
 
@@ -700,6 +701,7 @@ class NearbySharingServiceImplTest : public testing::Test {
   FakeNearbyShareCertificateManager::Factory certificate_manager_factory_;
   std::unique_ptr<NotificationDisplayServiceTester> notification_tester_;
   std::unique_ptr<base::ScopedDisallowBlocking> disallow_blocking_;
+  NiceMock<MockNearbyProcessManager> mock_nearby_process_manager_;
   std::unique_ptr<NearbySharingServiceImpl> service_;
   std::unique_ptr<FakeFastInitiationManagerFactory>
       fast_initiation_manager_factory_;
@@ -707,7 +709,6 @@ class NearbySharingServiceImplTest : public testing::Test {
   bool is_bluetooth_powered_ = true;
   device::BluetoothAdapter::Observer* adapter_observer_ = nullptr;
   scoped_refptr<NiceMock<device::MockBluetoothAdapter>> mock_bluetooth_adapter_;
-  NiceMock<MockNearbyProcessManager> mock_nearby_process_manager_;
   NiceMock<MockNearbySharingDecoder> mock_decoder_;
   FakeNearbyConnection connection_;
 };
@@ -921,9 +922,11 @@ TEST_F(NearbySharingServiceImplTest,
   MockTransferUpdateCallback transfer_callback;
   MockShareTargetDiscoveredCallback discovery_callback;
   EXPECT_EQ(
-      NearbySharingService::StatusCodes::kError,
+      NearbySharingService::StatusCodes::kOk,
       service_->RegisterSendSurface(&transfer_callback, &discovery_callback,
                                     SendSurfaceState::kForeground));
+  EXPECT_FALSE(fake_nearby_connections_manager_->IsDiscovering());
+  EXPECT_FALSE(fake_nearby_connections_manager_->is_shutdown());
 }
 
 TEST_F(NearbySharingServiceImplTest,
@@ -1202,6 +1205,8 @@ TEST_F(NearbySharingServiceImplTest, UnregisterSendSurfaceStopsDiscovering) {
                                     SendSurfaceState::kForeground));
   EXPECT_TRUE(fake_nearby_connections_manager_->IsDiscovering());
 
+  EXPECT_CALL(mock_nearby_process_manager(), StopProcess(profile_))
+      .Times(AtLeast(1));
   EXPECT_EQ(
       NearbySharingService::StatusCodes::kOk,
       service_->UnregisterSendSurface(&transfer_callback, &discovery_callback));
@@ -1232,6 +1237,9 @@ TEST_F(NearbySharingServiceImplTest,
 TEST_F(NearbySharingServiceImplTest, UnregisterSendSurfaceNeverRegistered) {
   ui::ScopedSetIdleState unlocked(ui::IDLE_STATE_IDLE);
   SetConnectionType(net::NetworkChangeNotifier::CONNECTION_WIFI);
+
+  EXPECT_CALL(mock_nearby_process_manager(), StopProcess(profile_))
+      .Times(AtLeast(1));
   MockTransferUpdateCallback transfer_callback;
   MockShareTargetDiscoveredCallback discovery_callback;
   EXPECT_EQ(
@@ -1589,6 +1597,8 @@ TEST_F(NearbySharingServiceImplTest, UnregisterReceiveSurfaceStopsAdvertising) {
   EXPECT_EQ(result, NearbySharingService::StatusCodes::kOk);
   EXPECT_TRUE(fake_nearby_connections_manager_->IsAdvertising());
 
+  EXPECT_CALL(mock_nearby_process_manager(), StopProcess(profile_))
+      .Times(AtLeast(1));
   NearbySharingService::StatusCodes result2 =
       service_->UnregisterReceiveSurface(&callback);
   EXPECT_EQ(result2, NearbySharingService::StatusCodes::kOk);
@@ -1616,6 +1626,9 @@ TEST_F(NearbySharingServiceImplTest,
 TEST_F(NearbySharingServiceImplTest, UnregisterReceiveSurfaceNeverRegistered) {
   ui::ScopedSetIdleState unlocked(ui::IDLE_STATE_IDLE);
   SetConnectionType(net::NetworkChangeNotifier::CONNECTION_WIFI);
+
+  EXPECT_CALL(mock_nearby_process_manager(), StopProcess(profile_))
+      .Times(AtLeast(1));
   MockTransferUpdateCallback callback;
   NearbySharingService::StatusCodes result =
       service_->UnregisterReceiveSurface(&callback);
@@ -2614,3 +2627,47 @@ TEST_F(NearbySharingServiceImplTest, SendFiles_Success) {
 
   service_->UnregisterSendSurface(&transfer_callback, &discovery_callback);
 }
+
+TEST_F(NearbySharingServiceImplTest, ProfileChangedControlsDiscovery) {
+  ui::ScopedSetIdleState unlocked(ui::IDLE_STATE_IDLE);
+  SetConnectionType(net::NetworkChangeNotifier::CONNECTION_WIFI);
+  MockTransferUpdateCallback transfer_callback;
+  MockShareTargetDiscoveredCallback discovery_callback;
+  EXPECT_EQ(
+      NearbySharingService::StatusCodes::kOk,
+      service_->RegisterSendSurface(&transfer_callback, &discovery_callback,
+                                    SendSurfaceState::kForeground));
+  EXPECT_TRUE(fake_nearby_connections_manager_->IsDiscovering());
+
+  ON_CALL(mock_nearby_process_manager_, IsActiveProfile(profile_))
+      .WillByDefault(Return(false));
+  service_->OnNearbyProfileChanged(/*profile=*/nullptr);
+  EXPECT_FALSE(fake_nearby_connections_manager_->IsDiscovering());
+  EXPECT_FALSE(fake_nearby_connections_manager_->is_shutdown());
+
+  ON_CALL(mock_nearby_process_manager_, IsActiveProfile(profile_))
+      .WillByDefault(Return(true));
+  service_->OnNearbyProfileChanged(/*profile=*/nullptr);
+  EXPECT_TRUE(fake_nearby_connections_manager_->IsDiscovering());
+}
+
+TEST_F(NearbySharingServiceImplTest, ProfileChangedControlsAdvertising) {
+  ui::ScopedSetIdleState unlocked(ui::IDLE_STATE_IDLE);
+  SetConnectionType(net::NetworkChangeNotifier::CONNECTION_WIFI);
+  MockTransferUpdateCallback callback;
+  NearbySharingService::StatusCodes result = service_->RegisterReceiveSurface(
+      &callback, NearbySharingService::ReceiveSurfaceState::kForeground);
+  EXPECT_EQ(result, NearbySharingService::StatusCodes::kOk);
+  EXPECT_TRUE(fake_nearby_connections_manager_->IsAdvertising());
+
+  ON_CALL(mock_nearby_process_manager_, IsActiveProfile(profile_))
+      .WillByDefault(Return(false));
+  service_->OnNearbyProfileChanged(/*profile=*/nullptr);
+  EXPECT_FALSE(fake_nearby_connections_manager_->IsAdvertising());
+  EXPECT_FALSE(fake_nearby_connections_manager_->is_shutdown());
+
+  ON_CALL(mock_nearby_process_manager_, IsActiveProfile(profile_))
+      .WillByDefault(Return(true));
+  service_->OnNearbyProfileChanged(/*profile=*/nullptr);
+  EXPECT_TRUE(fake_nearby_connections_manager_->IsAdvertising());
+}