[AndroidSmsService] Gate connection establishing with multidevice feature state.

This change adds logic to retrieve and listen for changes in multidevice
feature state and enables/disables establishing background connection from
service worker approriately.

Bug: 876163
Change-Id: I593a27f46a793678039ca40a1d0e49e97152c616
Reviewed-on: https://chromium-review.googlesource.com/1208143Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Azeem Arshad <azeemarshad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589411}

chrome/browser/chromeos/android_sms/BUILD.gn

@@ -35,6 +35,8 @@ static_library("android_sms") {
     "//chrome/browser/chromeos:chromeos",
     "//chromeos:chromeos",
     "//chromeos/components/proximity_auth/logging",
+    "//chromeos/services/multidevice_setup/public/cpp:cpp",
+    "//chromeos/services/multidevice_setup/public/cpp:prefs",
     "//components/keyed_service/content:content",
     "//components/keyed_service/core:core",
     "//components/session_manager/core:core",
@@ -68,6 +70,7 @@ source_set("unit_tests") {
     ":android_sms",
     ":android_sms_urls",
     ":test_support",
+    "//chromeos/services/multidevice_setup/public/cpp:test_support",
     "//content/public/browser",
     "//content/test:test_support",
     "//testing/gtest",

chrome/browser/chromeos/android_sms/android_sms_service.cc

@@ -5,9 +5,15 @@
 #include "chrome/browser/chromeos/android_sms/android_sms_service.h"
 #include "chrome/browser/chromeos/android_sms/android_sms_urls.h"
 #include "chrome/browser/chromeos/android_sms/connection_establisher_impl.h"
+#include "chrome/browser/chromeos/multidevice_setup/multidevice_setup_client_factory.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
 #include "components/session_manager/core/session_manager.h"
 #include "content/public/browser/storage_partition.h"
 
+using chromeos::multidevice_setup::MultiDeviceSetupClient;
+using chromeos::multidevice_setup::MultiDeviceSetupClientFactory;
+
 namespace chromeos {
 
 namespace android_sms {
@@ -37,8 +43,14 @@ void AndroidSmsService::OnSessionStateChanged() {
           browser_context_, GetAndroidMessagesURL());
   content::ServiceWorkerContext* service_worker_context =
       storage_partition->GetServiceWorkerContext();
+
+  MultiDeviceSetupClient* multidevice_setup_client =
+      MultiDeviceSetupClientFactory::GetForProfile(
+          Profile::FromBrowserContext(browser_context_));
+
   connection_manager_ = std::make_unique<ConnectionManager>(
-      service_worker_context, std::make_unique<ConnectionEstablisherImpl>());
+      service_worker_context, std::make_unique<ConnectionEstablisherImpl>(),
+      multidevice_setup_client);
 }
 
 }  // namespace android_sms

chrome/browser/chromeos/android_sms/android_sms_service_factory.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/android_sms/android_sms_service_factory.h"
+#include "chrome/browser/chromeos/multidevice_setup/multidevice_setup_client_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chromeos/chromeos_features.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -28,7 +29,10 @@ AndroidSmsService* AndroidSmsServiceFactory::GetForBrowserContext(
 AndroidSmsServiceFactory::AndroidSmsServiceFactory()
     : BrowserContextKeyedServiceFactory(
           "AndroidSmsService",
-          BrowserContextDependencyManager::GetInstance()) {}
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(chromeos::multidevice_setup::MultiDeviceSetupClientFactory::
+                GetInstance());
+}
 
 AndroidSmsServiceFactory::~AndroidSmsServiceFactory() = default;
 

chrome/browser/chromeos/android_sms/connection_manager.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/android_sms/connection_manager.h"
 #include "chrome/browser/chromeos/android_sms/android_sms_urls.h"
+#include "chromeos/components/proximity_auth/logging/logging.h"
 #include "components/session_manager/core/session_manager.h"
 
 namespace chromeos {
@@ -12,15 +13,20 @@ namespace android_sms {
 
 ConnectionManager::ConnectionManager(
     content::ServiceWorkerContext* service_worker_context,
-    std::unique_ptr<ConnectionEstablisher> connection_establisher)
+    std::unique_ptr<ConnectionEstablisher> connection_establisher,
+    MultiDeviceSetupClient* multidevice_setup_client)
     : service_worker_context_(service_worker_context),
-      connection_establisher_(std::move(connection_establisher)) {
+      connection_establisher_(std::move(connection_establisher)),
+      multidevice_setup_client_(multidevice_setup_client) {
   service_worker_context_->AddObserver(this);
-  connection_establisher_->EstablishConnection(service_worker_context_);
+  multidevice_setup_client_->AddObserver(this);
+  UpdateAndroidSmsFeatureState(multidevice_setup_client->GetFeatureState(
+      multidevice_setup::mojom::Feature::kMessages));
 }
 
 ConnectionManager::~ConnectionManager() {
   service_worker_context_->RemoveObserver(this);
+  multidevice_setup_client_->RemoveObserver(this);
 }
 
 void ConnectionManager::OnVersionActivated(int64_t version_id,
@@ -30,7 +36,8 @@ void ConnectionManager::OnVersionActivated(int64_t version_id,
 
   prev_active_version_id_ = active_version_id_;
   active_version_id_ = version_id;
-  connection_establisher_->EstablishConnection(service_worker_context_);
+  if (is_android_sms_enabled_)
+    connection_establisher_->EstablishConnection(service_worker_context_);
 }
 
 void ConnectionManager::OnVersionRedundant(int64_t version_id,
@@ -60,7 +67,36 @@ void ConnectionManager::OnNoControllees(int64_t version_id, const GURL& scope) {
   if (active_version_id_ != version_id)
     return;
 
-  connection_establisher_->EstablishConnection(service_worker_context_);
+  if (is_android_sms_enabled_)
+    connection_establisher_->EstablishConnection(service_worker_context_);
+}
+
+void ConnectionManager::OnFeatureStatesChanged(
+    const MultiDeviceSetupClient::FeatureStatesMap& feature_states_map) {
+  const auto it =
+      feature_states_map.find(multidevice_setup::mojom::Feature::kMessages);
+  if (it == feature_states_map.end())
+    return;
+
+  UpdateAndroidSmsFeatureState(it->second);
+}
+
+void ConnectionManager::UpdateAndroidSmsFeatureState(
+    multidevice_setup::mojom::FeatureState feature_state) {
+  bool is_enabled =
+      feature_state == multidevice_setup::mojom::FeatureState::kEnabledByUser;
+  if (is_android_sms_enabled_ == is_enabled)
+    return;
+
+  PA_LOG(INFO) << "ConnectionManager::UpdateAndroidSmsFeatureState enabled: "
+               << is_enabled;
+  if (is_enabled) {
+    connection_establisher_->EstablishConnection(service_worker_context_);
+  } else {
+    service_worker_context_->StopAllServiceWorkersForOrigin(
+        GetAndroidMessagesURL());
+  }
+  is_android_sms_enabled_ = is_enabled;
 }
 
 }  // namespace android_sms

chrome/browser/chromeos/android_sms/connection_manager.h

@@ -7,10 +7,13 @@
 
 #include "base/gtest_prod_util.h"
 #include "chrome/browser/chromeos/android_sms/connection_establisher.h"
+#include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/service_worker_context_observer.h"
 
+using chromeos::multidevice_setup::MultiDeviceSetupClient;
+
 namespace chromeos {
 
 namespace android_sms {
@@ -33,11 +36,13 @@ namespace android_sms {
 // connection as required. E.g., The service worker will not establish a
 // connection if it's is already connected to the Android Messages for Web page
 // or if a connection already exists.
-class ConnectionManager : public content::ServiceWorkerContextObserver {
+class ConnectionManager : public content::ServiceWorkerContextObserver,
+                          public MultiDeviceSetupClient::Observer {
  public:
   ConnectionManager(
       content::ServiceWorkerContext* service_worker_context,
-      std::unique_ptr<ConnectionEstablisher> connection_establisher);
+      std::unique_ptr<ConnectionEstablisher> connection_establisher,
+      MultiDeviceSetupClient* multidevice_setup_client);
   ~ConnectionManager() override;
 
  private:
@@ -46,8 +51,16 @@ class ConnectionManager : public content::ServiceWorkerContextObserver {
   void OnVersionRedundant(int64_t version_id, const GURL& scope) override;
   void OnNoControllees(int64_t version_id, const GURL& scope) override;
 
+  // MultideviceSetupClient::Observer:
+  void OnFeatureStatesChanged(const MultiDeviceSetupClient::FeatureStatesMap&
+                                  feature_state_map) override;
+
+  void UpdateAndroidSmsFeatureState(
+      multidevice_setup::mojom::FeatureState feature_state);
+
   content::ServiceWorkerContext* service_worker_context_;
   std::unique_ptr<ConnectionEstablisher> connection_establisher_;
+  MultiDeviceSetupClient* multidevice_setup_client_;
 
   // Version ID of the Android Messages for Web service worker that's currently
   // active i.e., capable of handling messages and controlling pages.
@@ -57,6 +70,8 @@ class ConnectionManager : public content::ServiceWorkerContextObserver {
   // service worker.
   base::Optional<int64_t> prev_active_version_id_;
 
+  bool is_android_sms_enabled_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(ConnectionManager);
 };
 

chrome/browser/chromeos/android_sms/connection_manager_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/android_sms/connection_manager.h"
 #include "chrome/browser/chromeos/android_sms/android_sms_urls.h"
 #include "chrome/browser/chromeos/android_sms/fake_connection_establisher.h"
+#include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
 #include "content/public/test/fake_service_worker_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -23,14 +24,21 @@ class ConnectionManagerTest : public testing::Test {
   ~ConnectionManagerTest() override = default;
 
   void SetUp() override {
+    fake_service_worker_context_ =
+        std::make_unique<content::FakeServiceWorkerContext>();
+    fake_multidevice_setup_client_ =
+        std::make_unique<multidevice_setup::FakeMultiDeviceSetupClient>();
+  }
+
+  void SetupConnectionManager(bool initial_feature_state) {
+    SetFeatureState(initial_feature_state);
     std::unique_ptr<FakeConnectionEstablisher> fake_connection_establisher =
         std::make_unique<FakeConnectionEstablisher>();
     fake_connection_establisher_ = fake_connection_establisher.get();
-    fake_service_worker_context_ =
-        std::make_unique<content::FakeServiceWorkerContext>();
     connection_manager_ = std::make_unique<ConnectionManager>(
         fake_service_worker_context_.get(),
-        std::move(fake_connection_establisher));
+        std::move(fake_connection_establisher),
+        fake_multidevice_setup_client_.get());
   }
 
   void VerifyNumEstablishConnectionCalls(size_t count) {
@@ -42,11 +50,20 @@ class ConnectionManagerTest : public testing::Test {
       EXPECT_EQ(fake_service_worker_context_.get(), service_worker_context);
   }
 
+  void SetFeatureState(bool enable) {
+    fake_multidevice_setup_client_->SetFeatureState(
+        multidevice_setup::mojom::Feature::kMessages,
+        enable ? multidevice_setup::mojom::FeatureState::kEnabledByUser
+               : multidevice_setup::mojom::FeatureState::kDisabledByUser);
+  }
+
   content::FakeServiceWorkerContext* fake_service_worker_context() const {
     return fake_service_worker_context_.get();
   }
 
  private:
+  std::unique_ptr<multidevice_setup::FakeMultiDeviceSetupClient>
+      fake_multidevice_setup_client_;
   FakeConnectionEstablisher* fake_connection_establisher_;
   std::unique_ptr<content::FakeServiceWorkerContext>
       fake_service_worker_context_;
@@ -55,6 +72,7 @@ class ConnectionManagerTest : public testing::Test {
 };
 
 TEST_F(ConnectionManagerTest, ConnectOnActivate) {
+  SetupConnectionManager(true /* initial_feature_state */);
   fake_service_worker_context()->NotifyObserversOnVersionActivated(
       kDummyVersionId, GetAndroidMessagesURL());
 
@@ -65,6 +83,7 @@ TEST_F(ConnectionManagerTest, ConnectOnActivate) {
 }
 
 TEST_F(ConnectionManagerTest, ConnectOnNoControllees) {
+  SetupConnectionManager(true /* initial_feature_state */);
   // Notify Activation so that Connection manager is tracking the version ID.
   fake_service_worker_context()->NotifyObserversOnVersionActivated(
       kDummyVersionId, GetAndroidMessagesURL());
@@ -78,6 +97,7 @@ TEST_F(ConnectionManagerTest, ConnectOnNoControllees) {
 }
 
 TEST_F(ConnectionManagerTest, IgnoreRedundantVersion) {
+  SetupConnectionManager(true /* initial_feature_state */);
   fake_service_worker_context()->NotifyObserversOnVersionActivated(
       kDummyVersionId, GetAndroidMessagesURL());
 
@@ -94,6 +114,7 @@ TEST_F(ConnectionManagerTest, IgnoreRedundantVersion) {
 }
 
 TEST_F(ConnectionManagerTest, ConnectOnNoControlleesWithNoActive) {
+  SetupConnectionManager(true /* initial_feature_state */);
   // Verify that connection establishing is attempted when there are no
   // controllees for a version ID even if the activate notification was missed.
   fake_service_worker_context()->NotifyObserversOnNoControllees(
@@ -103,6 +124,7 @@ TEST_F(ConnectionManagerTest, ConnectOnNoControlleesWithNoActive) {
 }
 
 TEST_F(ConnectionManagerTest, IgnoreOnNoControlleesInvalidId) {
+  SetupConnectionManager(true /* initial_feature_state */);
   fake_service_worker_context()->NotifyObserversOnVersionActivated(
       kDummyVersionId, GetAndroidMessagesURL());
 
@@ -114,6 +136,7 @@ TEST_F(ConnectionManagerTest, IgnoreOnNoControlleesInvalidId) {
 }
 
 TEST_F(ConnectionManagerTest, InvalidScope) {
+  SetupConnectionManager(true /* initial_feature_state */);
   GURL invalid_scope("https://example.com");
   // Verify that OnVersionActivated and OnNoControllees with invalid scope
   // are ignored
@@ -134,6 +157,39 @@ TEST_F(ConnectionManagerTest, InvalidScope) {
   VerifyNumEstablishConnectionCalls(3u);
 }
 
+TEST_F(ConnectionManagerTest, FeatureStateInitDisabled) {
+  // Verify that connection is not established on initialization
+  // if the feature is not enabled.
+  SetupConnectionManager(false /* initial_feature_state */);
+  VerifyNumEstablishConnectionCalls(0u);
+
+  SetFeatureState(true);
+  VerifyNumEstablishConnectionCalls(1u);
+}
+
+TEST_F(ConnectionManagerTest, FeatureStateChange) {
+  SetupConnectionManager(true /* initial_feature_state */);
+  fake_service_worker_context()->NotifyObserversOnVersionActivated(
+      kDummyVersionId, GetAndroidMessagesURL());
+
+  // Verify that disabling feature stops the service worker.
+  SetFeatureState(false);
+  VerifyNumEstablishConnectionCalls(2u);
+  const auto& stop_calls = fake_service_worker_context()
+                               ->stop_all_service_workers_for_origin_calls();
+  ASSERT_EQ(1u, stop_calls.size());
+  EXPECT_EQ(GetAndroidMessagesURL(), stop_calls[0]);
+
+  // Verify that subsequent service worker events do not trigger connection.
+  fake_service_worker_context()->NotifyObserversOnNoControllees(
+      kDummyVersionId, GetAndroidMessagesURL());
+  VerifyNumEstablishConnectionCalls(2u);
+
+  // Verify that enabling feature establishes connection again.
+  SetFeatureState(true);
+  VerifyNumEstablishConnectionCalls(3u);
+}
+
 }  // namespace android_sms
 
 }  // namespace chromeos

content/public/test/fake_service_worker_context.cc

@@ -89,8 +89,9 @@ void FakeServiceWorkerContext::StartServiceWorkerAndDispatchLongRunningMessage(
 
 void FakeServiceWorkerContext::StopAllServiceWorkersForOrigin(
     const GURL& origin) {
-  NOTREACHED();
+  stop_all_service_workers_for_origin_calls_.push_back(origin);
 }
+
 void FakeServiceWorkerContext::StopAllServiceWorkers(base::OnceClosure) {
   NOTREACHED();
 }

content/public/test/fake_service_worker_context.h

@@ -79,12 +79,18 @@ class FakeServiceWorkerContext : public ServiceWorkerContext {
     return start_service_worker_and_dispatch_long_running_message_calls_;
   };
 
+  const std::vector<GURL>& stop_all_service_workers_for_origin_calls() {
+    return stop_all_service_workers_for_origin_calls_;
+  }
+
  private:
   bool start_service_worker_for_navigation_hint_called_ = false;
 
   std::vector<StartServiceWorkerAndDispatchLongRunningMessageArgs>
       start_service_worker_and_dispatch_long_running_message_calls_;
 
+  std::vector<GURL> stop_all_service_workers_for_origin_calls_;
+
   base::ObserverList<ServiceWorkerContextObserver, true>::Unchecked observers_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeServiceWorkerContext);