From 9333d21822b12e5240f66bf132d4d67fc5acecce Mon Sep 17 00:00:00 2001 From: James Vecore Date: Sat, 27 Feb 2021 00:38:32 +0000 Subject: [PATCH] [Nearby] Stop forwarding Stop() to the ServiceController When NearbyShare is toggled off, StopAllEndpoints() is called which has the unfortunate side effect of calling Stop() on the shared OfflineServiceController. This ends up causing two problems: 1) It breaks other clients like PhoneHub sharing the service controller. 2) The OfflineServiceController is not designed to be restarted so this is not recoverable. If the feature is re-enabled in the same session the user ends up having a broken nearby share experience (see crbug/1180463 and crbug/1182425). The NC change that introduced this is here: cl/356658736 and the uprev is here: https://crrev.com/c/2697908. To fix this issue we simply don't forward the Stop() call to the shared OfflineServiceController from the proxy. This is necessary to allow both PhoneHub and Nearby Share to be able to toggle their feature states without affecting each other. We rely on the destructor to do the necessary final cleanup. Long term, the API design of NC needs to be updated to properly support multiple clients with a share OfflineServiceController see: https://crbug.com/1176249. (cherry picked from commit 6c0823dc0ad5b5b82fac47a7d8a4bc7ffc6c9ffe) Fixed: 1180463, 1182425 Change-Id: I8c1f6e4e63a4d5b01a7e6da7e1c57b4a77814d1d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2716476 Commit-Queue: James Vecore Reviewed-by: Josh Nohle Cr-Original-Commit-Position: refs/heads/master@{#858258} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2725354 Commit-Queue: Josh Nohle Auto-Submit: James Vecore Cr-Commit-Position: refs/branch-heads/4430@{#13} Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950} --- chrome/services/sharing/nearby/nearby_connections.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/chrome/services/sharing/nearby/nearby_connections.cc b/chrome/services/sharing/nearby/nearby_connections.cc index 2bc9bc9104d91..2504919c53c83 100644 --- a/chrome/services/sharing/nearby/nearby_connections.cc +++ b/chrome/services/sharing/nearby/nearby_connections.cc @@ -130,11 +130,8 @@ class ServiceControllerProxy : public ServiceController { } void Stop() override { - if (!inner_service_controller_) - return; - // TODO(crbug/1176249): This stops the service controller for all Cores that - // share this inner_service_controller_. - inner_service_controller_->Stop(); + // TODO(https://crbug.com/1182428): we purposefully do nothing here so we + // don't shutdown the share OfflineServiceController. } private: