[Nearby] Implement StopAllEndpoints NearbyConnection interface

chrome/services/sharing/*
- Introduced StopAllEndpoints to clean up all connections to remote
  endpoints

chrome/browser/nearby_sharing/nearby_connections_manager*
- Browser side usage of StopAllEndpoints. NearbyConnectionsManager
  uses StopAllEndpoints to clean up when shutdown is requested, or
  when the nearby process is stopped.

Bug: 1076008
Change-Id: Ie5cc7dedf2accad91774217be87d9e5218116594
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346673
Auto-Submit: Naomi Musgrave <nmusgrave@chromium.org>
Commit-Queue: Alex Chau <alexchau@chromium.org>
Reviewed-by: Alex Chau <alexchau@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798642}

chrome/browser/nearby_sharing/mock_nearby_connections.h

@@ -78,6 +78,10 @@ class MockNearbyConnections : public NearbyConnectionsMojom {
               CancelPayload,
               (int64_t payload_id, CancelPayloadCallback callback),
               (override));
+  MOCK_METHOD(void,
+              StopAllEndpoints,
+              (DisconnectFromEndpointCallback callback),
+              (override));
 };
 
 #endif  // CHROME_BROWSER_NEARBY_SHARING_MOCK_NEARBY_CONNECTIONS_H_

chrome/browser/nearby_sharing/nearby_connections_manager.h

@@ -66,6 +66,8 @@ class NearbyConnectionsManager {
   virtual ~NearbyConnectionsManager() = default;
 
   // Disconnects from all endpoints and shut down Nearby Connections.
+  // As a side effect of this call, both StopAdvertising and StopDiscovery may
+  // be invoked if Nearby Connections is advertising or discovering.
   virtual void Shutdown() = 0;
 
   // Starts advertising through Nearby Connections. Caller is expected to ensure

chrome/browser/nearby_sharing/nearby_connections_manager_impl.cc

@@ -59,6 +59,8 @@ NearbyConnectionsManagerImpl::~NearbyConnectionsManagerImpl() = default;
 
 void NearbyConnectionsManagerImpl::Shutdown() {
   // TOOD(crbug/1076008): Implement.
+  // Disconnects from all endpoints and shut down Nearby Connections.
+  Reset();
 }
 
 void NearbyConnectionsManagerImpl::StartAdvertising(
@@ -95,8 +97,15 @@ void NearbyConnectionsManagerImpl::StartAdvertising(
 }
 
 void NearbyConnectionsManagerImpl::StopAdvertising() {
-  if (nearby_connections_)
-    nearby_connections_->StopAdvertising(base::DoNothing());
+  if (nearby_connections_) {
+    nearby_connections_->StopAdvertising(base::BindOnce([](ConnectionsStatus
+                                                               status) {
+      NS_LOG(VERBOSE)
+          << __func__
+          << ": Stop advertising attempted over Nearby Connections with result "
+          << status;
+    }));
+  }
 
   incoming_connection_listener_ = nullptr;
 }
@@ -120,8 +129,15 @@ void NearbyConnectionsManagerImpl::StartDiscovery(
 }
 
 void NearbyConnectionsManagerImpl::StopDiscovery() {
-  if (nearby_connections_)
-    nearby_connections_->StopDiscovery(base::DoNothing());
+  if (nearby_connections_) {
+    nearby_connections_->StopDiscovery(
+        base::BindOnce([](ConnectionsStatus status) {
+          NS_LOG(VERBOSE) << __func__
+                          << ": Stop discovery attempted over Nearby "
+                             "Connections with result "
+                          << status;
+        }));
+  }
 
   discovered_endpoints_.clear();
   discovery_listener_ = nullptr;
@@ -158,7 +174,16 @@ void NearbyConnectionsManagerImpl::OnConnectionRequested(
     ConnectionsStatus status) {
   if (status != ConnectionsStatus::kSuccess) {
     NS_LOG(ERROR) << "Failed to connect to the remote shareTarget: " << status;
-    nearby_connections_->DisconnectFromEndpoint(endpoint_id, base::DoNothing());
+    nearby_connections_->DisconnectFromEndpoint(
+        endpoint_id,
+        base::BindOnce(
+            [](const std::string& endpoint_id, ConnectionsStatus status) {
+              NS_LOG(VERBOSE)
+                  << __func__ << ": Disconnecting from endpoint " << endpoint_id
+                  << " attempted over Nearby Connections with result "
+                  << status;
+            },
+            endpoint_id));
     std::move(callback).Run(nullptr);
     return;
   }
@@ -174,7 +199,16 @@ void NearbyConnectionsManagerImpl::Disconnect(const std::string& endpoint_id) {
   if (!nearby_connections_)
     return;
 
-  nearby_connections_->DisconnectFromEndpoint(endpoint_id, base::DoNothing());
+  nearby_connections_->DisconnectFromEndpoint(
+      endpoint_id,
+      base::BindOnce(
+          [](const std::string& endpoint_id, ConnectionsStatus status) {
+            NS_LOG(VERBOSE)
+                << __func__ << ": Disconnecting from endpoint " << endpoint_id
+                << " attempted over Nearby Connections with result " << status;
+          },
+          endpoint_id));
+
   OnDisconnected(endpoint_id);
   NS_LOG(INFO) << "Disconnected from " << endpoint_id;
 }
@@ -188,8 +222,15 @@ void NearbyConnectionsManagerImpl::Send(const std::string& endpoint_id,
   if (listener)
     RegisterPayloadStatusListener(payload->id, listener);
 
-  nearby_connections_->SendPayload({endpoint_id}, std::move(payload),
-                                   base::DoNothing());
+  nearby_connections_->SendPayload(
+      {endpoint_id}, std::move(payload),
+      base::BindOnce(
+          [](const std::string& endpoint_id, ConnectionsStatus status) {
+            NS_LOG(VERBOSE)
+                << __func__ << ": Sending payload to endpoint " << endpoint_id
+                << " attempted over Nearby Connections with result " << status;
+          },
+          endpoint_id));
 }
 
 void NearbyConnectionsManagerImpl::RegisterPayloadStatusListener(
@@ -216,7 +257,16 @@ void NearbyConnectionsManagerImpl::Cancel(int64_t payload_id) {
                                    /*bytes_transferred=*/0));
     payload_status_listeners_.erase(it);
   }
-  nearby_connections_->CancelPayload(payload_id, base::DoNothing());
+  nearby_connections_->CancelPayload(
+      payload_id, base::BindOnce(
+                      [](int64_t payload_id, ConnectionsStatus status) {
+                        NS_LOG(VERBOSE)
+                            << __func__ << ": Cancelling payload to id "
+                            << payload_id
+                            << " attempted over Nearby Connections with result "
+                            << status;
+                      },
+                      payload_id));
   NS_LOG(INFO) << "Cancelling payload: " << payload_id;
 }
 
@@ -249,6 +299,9 @@ void NearbyConnectionsManagerImpl::OnNearbyProcessStarted() {
 
 void NearbyConnectionsManagerImpl::OnNearbyProcessStopped() {
   NS_LOG(VERBOSE) << __func__;
+  // Not safe to use nearby_connections after we are notified the process has
+  // been stopped.
+  nearby_connections_ = nullptr;
   Reset();
 }
 
@@ -310,7 +363,15 @@ void NearbyConnectionsManagerImpl::OnConnectionInitiated(
                          payload_listener.InitWithNewPipeAndPassReceiver());
 
   nearby_connections_->AcceptConnection(
-      endpoint_id, std::move(payload_listener), base::DoNothing());
+      endpoint_id, std::move(payload_listener),
+      base::BindOnce(
+          [](const std::string& endpoint_id, ConnectionsStatus status) {
+            NS_LOG(VERBOSE)
+                << __func__ << ": Accept connection attempted to endpoint "
+                << endpoint_id << "over Nearby Connections with result "
+                << status;
+          },
+          endpoint_id));
 }
 
 void NearbyConnectionsManagerImpl::OnConnectionAccepted(
@@ -414,6 +475,15 @@ bool NearbyConnectionsManagerImpl::BindNearbyConnections() {
 }
 
 void NearbyConnectionsManagerImpl::Reset() {
+  if (nearby_connections_) {
+    nearby_connections_->StopAllEndpoints(
+        base::BindOnce([](ConnectionsStatus status) {
+          NS_LOG(VERBOSE) << __func__
+                          << ": Stop all endpoints attempted over Nearby "
+                             "Connections with result "
+                          << status;
+        }));
+  }
   nearby_connections_ = nullptr;
   discovered_endpoints_.clear();
   discovery_listener_ = nullptr;

chrome/browser/nearby_sharing/nearby_connections_manager_impl.h

@@ -63,6 +63,9 @@ class NearbyConnectionsManagerImpl
       const std::string& endpoint_id) override;
   void UpgradeBandwidth(const std::string& endpoint_id) override;
 
+  // Converts the status to a logging-friendly string.
+  static std::string ConnectionsStatusToString(ConnectionsStatus status);
+
  private:
   using AdvertisingOptions =
       location::nearby::connections::mojom::AdvertisingOptions;

chrome/browser/nearby_sharing/nearby_connections_manager_impl_unittest.cc

@@ -344,6 +344,7 @@ TEST_F(NearbyConnectionsManagerImplTest, DiscoveryProcessStopped) {
   testing::NiceMock<MockDiscoveryListener> discovery_listener;
   StartDiscovery(listener_remote, discovery_listener);
 
+  EXPECT_CALL(nearby_connections_, StopAllEndpoints).Times(0);
   nearby_connections_manager_.OnNearbyProcessStopped();
 
   // Invoking OnEndpointFound will do nothing.
@@ -868,3 +869,43 @@ TEST_F(NearbyConnectionsManagerImplTest, StopAdvertising) {
   EXPECT_CALL(nearby_connections_, StopAdvertising);
   nearby_connections_manager_.StopAdvertising();
 }
+
+TEST_F(NearbyConnectionsManagerImplTest, ShutdownAdvertising) {
+  mojo::Remote<ConnectionLifecycleListener> listener_remote;
+  testing::NiceMock<MockIncomingConnectionListener>
+      incoming_connection_listener;
+  StartAdvertising(listener_remote, incoming_connection_listener);
+
+  EXPECT_CALL(nearby_connections_, StopAllEndpoints);
+  nearby_connections_manager_.Shutdown();
+}
+
+TEST_F(NearbyConnectionsManagerImplTest, ShutdownDiscoveryConnectionFails) {
+  mojo::Remote<EndpointDiscoveryListener> discovery_listener_remote;
+  testing::NiceMock<MockDiscoveryListener> discovery_listener;
+  StartDiscovery(discovery_listener_remote, discovery_listener);
+
+  EXPECT_CALL(nearby_connections_, StopAllEndpoints);
+  nearby_connections_manager_.Shutdown();
+
+  // RequestConnection will fail.
+  const std::vector<uint8_t> local_endpoint_info(std::begin(kEndpointInfo),
+                                                 std::end(kEndpointInfo));
+  base::RunLoop run_loop;
+  NearbyConnection* nearby_connection;
+  nearby_connections_manager_.Connect(
+      local_endpoint_info, kRemoteEndpointId,
+      /*bluetooth_mac_address=*/base::nullopt, DataUsage::kOffline,
+      base::BindLambdaForTesting([&](NearbyConnection* connection) {
+        nearby_connection = connection;
+        run_loop.Quit();
+      }));
+
+  EXPECT_FALSE(nearby_connection);
+}
+
+TEST_F(NearbyConnectionsManagerImplTest,
+       ShutdownBeforeStartDiscoveryOrAdvertising) {
+  EXPECT_CALL(nearby_connections_, StopAllEndpoints).Times(0);
+  nearby_connections_manager_.Shutdown();
+}

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -54,19 +54,6 @@ std::string ReceiveSurfaceStateToString(
   }
 }
 
-std::string DataUsageToString(DataUsage usage) {
-  switch (usage) {
-    case DataUsage::kOffline:
-      return "OFFLINE";
-    case DataUsage::kOnline:
-      return "ONLINE";
-    case DataUsage::kWifiOnly:
-      return "WIFI_ONLY";
-    case DataUsage::kUnknown:
-      return "UNKNOWN";
-  }
-}
-
 std::string PowerLevelToString(PowerLevel level) {
   switch (level) {
     case PowerLevel::kLowPower:
@@ -80,55 +67,6 @@ std::string PowerLevelToString(PowerLevel level) {
   }
 }
 
-std::string VisibilityToString(Visibility visibility) {
-  switch (visibility) {
-    case Visibility::kNoOne:
-      return "NO_ONE";
-    case Visibility::kAllContacts:
-      return "ALL_CONTACTS";
-    case Visibility::kSelectedContacts:
-      return "SELECTED_CONTACTS";
-    case Visibility::kUnknown:
-      return "UNKNOWN";
-  }
-}
-
-std::string ConnectionsStatusToString(
-    NearbyConnectionsManager::ConnectionsStatus status) {
-  switch (status) {
-    case NearbyConnectionsManager::ConnectionsStatus::kSuccess:
-      return "SUCCESS";
-    case NearbyConnectionsManager::ConnectionsStatus::kError:
-      return "ERROR";
-    case NearbyConnectionsManager::ConnectionsStatus::kOutOfOrderApiCall:
-      return "OUT_OF_ORDER_API_CALL";
-    case NearbyConnectionsManager::ConnectionsStatus::
-        kAlreadyHaveActiveStrategy:
-      return "ALREADY_HAVE_ACTIVE_STRATEGY";
-    case NearbyConnectionsManager::ConnectionsStatus::kAlreadyAdvertising:
-      return "ALREADY_ADVERTISING";
-    case NearbyConnectionsManager::ConnectionsStatus::kAlreadyDiscovering:
-      return "ALREADY_DISCOVERING";
-    case NearbyConnectionsManager::ConnectionsStatus::kEndpointIOError:
-      return "ENDPOINT_IO_ERROR";
-    case NearbyConnectionsManager::ConnectionsStatus::kEndpointUnknown:
-      return "ENDPOINT_UNKNOWN";
-    case NearbyConnectionsManager::ConnectionsStatus::kConnectionRejected:
-      return "CONNECTION_REJECTED";
-    case NearbyConnectionsManager::ConnectionsStatus::
-        kAlreadyConnectedToEndpoint:
-      return "ALREADY_CONNECTED_TO_ENDPOINT";
-    case NearbyConnectionsManager::ConnectionsStatus::kNotConnectedToEndpoint:
-      return "NOT_CONNECTED_TO_ENDPOINT";
-    case NearbyConnectionsManager::ConnectionsStatus::kBluetoothError:
-      return "BLUETOOTH_ERROR";
-    case NearbyConnectionsManager::ConnectionsStatus::kWifiLanError:
-      return "WIFI_LAN_ERROR";
-    case NearbyConnectionsManager::ConnectionsStatus::kPayloadUnknown:
-      return "PAYLOAD_UNKNOWN";
-  }
-}
-
 base::Optional<std::string> GetDeviceName(
     const sharing::mojom::AdvertisementPtr& advertisement,
     const base::Optional<NearbyShareDecryptedPublicCertificate>& certificate) {
@@ -700,7 +638,7 @@ bool NearbySharingServiceImpl::IsVisibleInBackground(Visibility visibility) {
 void NearbySharingServiceImpl::OnVisibilityChanged(Visibility new_visibility) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   NS_LOG(VERBOSE) << __func__ << ": Nearby sharing visibility changed to "
-                  << VisibilityToString(new_visibility);
+                  << new_visibility;
   if (advertising_power_level_ != PowerLevel::kUnknown)
     StopAdvertising();
 
@@ -710,7 +648,7 @@ void NearbySharingServiceImpl::OnVisibilityChanged(Visibility new_visibility) {
 void NearbySharingServiceImpl::OnDataUsageChanged(DataUsage data_usage) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   NS_LOG(VERBOSE) << __func__ << ": Nearby sharing data usage changed to "
-                  << DataUsageToString(data_usage);
+                  << data_usage;
 
   if (advertising_power_level_ != PowerLevel::kUnknown)
     StopAdvertising();
@@ -955,15 +893,14 @@ void NearbySharingServiceImpl::InvalidateAdvertisingState() {
           << ": Failed to advertise because we're already advertising with "
              "power level "
           << PowerLevelToString(advertising_power_level_)
-          << " and data usage preference " << DataUsageToString(data_usage);
+          << " and data usage preference " << data_usage;
       return;
     }
 
     StopAdvertising();
     NS_LOG(VERBOSE) << __func__ << ": Restart advertising with power level "
                     << PowerLevelToString(power_level)
-                    << " and data usage preference "
-                    << DataUsageToString(data_usage);
+                    << " and data usage preference " << data_usage;
   }
 
   base::Optional<std::string> device_name;
@@ -988,16 +925,15 @@ void NearbySharingServiceImpl::InvalidateAdvertisingState() {
         NS_LOG(VERBOSE)
             << __func__
             << ": Advertising attempted over Nearby Connections with result "
-            << ConnectionsStatusToString(status);
+            << status;
       }));
 
   advertising_power_level_ = power_level;
   NS_LOG(VERBOSE) << __func__
                   << ": Advertising has started over Nearby Connections: "
                   << " power level " << PowerLevelToString(power_level)
-                  << " visibility "
-                  << VisibilityToString(settings_.GetVisibility())
-                  << " data usage " << DataUsageToString(data_usage);
+                  << " visibility " << settings_.GetVisibility()
+                  << " data usage " << data_usage;
   return;
 }
 
@@ -1113,7 +1049,7 @@ void NearbySharingServiceImpl::StartScanning() {
         NS_LOG(VERBOSE) << __func__
                         << ": Scanning start attempted over Nearby Connections "
                            "with result "
-                        << ConnectionsStatusToString(status);
+                        << status;
       }));
 
   InvalidateSendSurfaceState();

chrome/services/sharing/nearby/nearby_connections.cc

@@ -337,6 +337,10 @@ void NearbyConnections::CancelPayload(int64_t payload_id,
                        ResultCallbackFromMojom(std::move(callback)));
 }
 
+void NearbyConnections::StopAllEndpoints(StopAllEndpointsCallback callback) {
+  core_->StopAllEndpoints(ResultCallbackFromMojom(std::move(callback)));
+}
+
 base::File NearbyConnections::ExtractFileForPayload(int64_t payload_id) {
   auto file_it = outgoing_file_map_.find(payload_id);
   if (file_it == outgoing_file_map_.end())

chrome/services/sharing/nearby/nearby_connections.h

@@ -93,6 +93,7 @@ class NearbyConnections : public mojom::NearbyConnections {
                    SendPayloadCallback callback) override;
   void CancelPayload(int64_t payload_id,
                      CancelPayloadCallback callback) override;
+  void StopAllEndpoints(StopAllEndpointsCallback callback) override;
 
   // Return the file associated with |payload_id|.
   base::File ExtractFileForPayload(int64_t payload_id);

chrome/services/sharing/public/mojom/nearby_connections.mojom

@@ -258,6 +258,16 @@ interface NearbyConnections {
   // Possible return values include:
   //   Status::kSuccess if the payload got canceled.
   CancelPayload(int64 payload_id) => (Status status);
+
+   // Disconnects from, and removes all traces of, all connected and/or
+   // discovered endpoints. As a side effect of this call, both
+   // StopAdvertising and StopDiscovery are invoked. After calling
+   // StopAllEndpoints, no further operations with remote endpoints will
+   // be possible until a new call to one of StartAdvertising or
+   // StartDiscovery.
+   // Possible return values include:
+   //   Status::kSuccess disconnected from all endpoints successfully.
+  StopAllEndpoints() => (Status status);
 };
 
 // Provide all the dependencies that NearbyConnections library requires.