Install Messages PWA when host is set or verified.

This change moves the logic for installing the Android Messages PWA into
a helper class which is a HostStatusProvider::Observer. This allows it
to install the PWA when a host is set and also when signing into a new
Chromebook if the account already has a valid host.

Bug: 887775
Change-Id: I1a1433b03010d391f9b8256754fc578dfb8a140b
Reviewed-on: https://chromium-review.googlesource.com/1248130Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Jeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594943}

chromeos/services/multidevice_setup/BUILD.gn

@@ -14,6 +14,8 @@ static_library("multidevice_setup") {
     "account_status_change_delegate_notifier.h",
     "account_status_change_delegate_notifier_impl.cc",
     "account_status_change_delegate_notifier_impl.h",
+    "android_sms_app_installing_status_observer.cc",
+    "android_sms_app_installing_status_observer.h",
     "device_reenroller.cc",
     "device_reenroller.h",
     "eligible_host_devices_provider.h",
@@ -118,6 +120,7 @@ source_set("unit_tests") {
 
   sources = [
     "account_status_change_delegate_notifier_impl_unittest.cc",
+    "android_sms_app_installing_status_observer_unittest.cc",
     "device_reenroller_unittest.cc",
     "eligible_host_devices_provider_impl_unittest.cc",
     "feature_state_manager_impl_unittest.cc",

chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.h"
+
+#include "base/memory/ptr_util.h"
+#include "base/no_destructor.h"
+#include "chromeos/services/multidevice_setup/host_status_provider.h"
+#include "chromeos/services/multidevice_setup/public/cpp/android_sms_app_helper_delegate.h"
+#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+// static
+AndroidSmsAppInstallingStatusObserver::Factory*
+    AndroidSmsAppInstallingStatusObserver::Factory::test_factory_ = nullptr;
+
+// static
+AndroidSmsAppInstallingStatusObserver::Factory*
+AndroidSmsAppInstallingStatusObserver::Factory::Get() {
+  if (test_factory_)
+    return test_factory_;
+
+  static base::NoDestructor<Factory> factory;
+  return factory.get();
+}
+
+// static
+void AndroidSmsAppInstallingStatusObserver::Factory::SetFactoryForTesting(
+    Factory* test_factory) {
+  test_factory_ = test_factory;
+}
+
+AndroidSmsAppInstallingStatusObserver::Factory::~Factory() = default;
+
+std::unique_ptr<AndroidSmsAppInstallingStatusObserver>
+AndroidSmsAppInstallingStatusObserver::Factory::BuildInstance(
+    HostStatusProvider* host_status_provider,
+    std::unique_ptr<AndroidSmsAppHelperDelegate>
+        android_sms_app_helper_delegate) {
+  return base::WrapUnique(new AndroidSmsAppInstallingStatusObserver(
+      host_status_provider, std::move(android_sms_app_helper_delegate)));
+}
+
+AndroidSmsAppInstallingStatusObserver::
+    ~AndroidSmsAppInstallingStatusObserver() {
+  host_status_provider_->RemoveObserver(this);
+}
+
+AndroidSmsAppInstallingStatusObserver::AndroidSmsAppInstallingStatusObserver(
+    HostStatusProvider* host_status_provider,
+    std::unique_ptr<AndroidSmsAppHelperDelegate>
+        android_sms_app_helper_delegate)
+    : host_status_provider_(host_status_provider),
+      android_sms_app_helper_delegate_(
+          std::move(android_sms_app_helper_delegate)) {
+  host_status_provider_->AddObserver(this);
+}
+
+void AndroidSmsAppInstallingStatusObserver::OnHostStatusChange(
+    const HostStatusProvider::HostStatusWithDevice& host_status_with_device) {
+  mojom::HostStatus status(host_status_with_device.host_status());
+  if (status ==
+          mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation ||
+      status == mojom::HostStatus::kHostVerified) {
+    // This call is re-entrant. If the app is already installed, it will just
+    // fail silently, which is fine.
+    android_sms_app_helper_delegate_->InstallAndroidSmsApp();
+  }
+}
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos

chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_SERVICES_MULTIDEVICE_SETUP_ANDROID_SMS_APP_INSTALLING_STATUS_OBSERVER_H_
+#define CHROMEOS_SERVICES_MULTIDEVICE_SETUP_ANDROID_SMS_APP_INSTALLING_STATUS_OBSERVER_H_
+
+#include <memory>
+
+#include "chromeos/services/multidevice_setup/host_status_provider.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+class AndroidSmsAppHelperDelegate;
+
+// Listens for status changes in multidevice state and installs the Android
+// Messages PWA if needed.
+//
+// TODO(crbug.com/884290): Also observe FeatureStateManager to make sure
+// Messages is supported.
+class AndroidSmsAppInstallingStatusObserver
+    : public HostStatusProvider::Observer {
+ public:
+  class Factory {
+   public:
+    static Factory* Get();
+    static void SetFactoryForTesting(Factory* test_factory);
+    virtual ~Factory();
+    virtual std::unique_ptr<AndroidSmsAppInstallingStatusObserver>
+    BuildInstance(HostStatusProvider* host_status_provider,
+                  std::unique_ptr<AndroidSmsAppHelperDelegate>
+                      android_sms_app_helper_delegate);
+
+   private:
+    static Factory* test_factory_;
+  };
+
+  ~AndroidSmsAppInstallingStatusObserver() override;
+
+ private:
+  AndroidSmsAppInstallingStatusObserver(
+      HostStatusProvider* host_status_provider,
+      std::unique_ptr<AndroidSmsAppHelperDelegate>
+          android_sms_app_helper_delegate);
+
+  // HostStatusProvider::Observer:
+  void OnHostStatusChange(const HostStatusProvider::HostStatusWithDevice&
+                              host_status_with_device) override;
+
+  HostStatusProvider* host_status_provider_;
+  std::unique_ptr<AndroidSmsAppHelperDelegate> android_sms_app_helper_delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(AndroidSmsAppInstallingStatusObserver);
+};
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_MULTIDEVICE_SETUP_ANDROID_SMS_APP_INSTALLING_STATUS_OBSERVER_H_

chromeos/services/multidevice_setup/android_sms_app_installing_status_observer_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.h"
+
+#include <string>
+
+#include "chromeos/services/multidevice_setup/fake_host_status_provider.h"
+#include "chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h"
+#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
+#include "components/cryptauth/remote_device_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+namespace {
+
+const char kFakePhoneKey[] = "fake-phone-key";
+const char kFakePhoneName[] = "Phony Phone";
+
+}  // namespace
+
+class MultiDeviceSetupAndroidSmsAppInstallingStatusObserverTest
+    : public testing::Test {
+ protected:
+  MultiDeviceSetupAndroidSmsAppInstallingStatusObserverTest() = default;
+
+  ~MultiDeviceSetupAndroidSmsAppInstallingStatusObserverTest() override =
+      default;
+
+  void SetUp() override {
+    auto fake_android_sms_app_helper_delegate =
+        std::make_unique<FakeAndroidSmsAppHelperDelegate>();
+    fake_android_sms_app_helper_delegate_ =
+        fake_android_sms_app_helper_delegate.get();
+    fake_host_status_provider_ = std::make_unique<FakeHostStatusProvider>();
+    android_sms_app_installing_status_observer_ =
+        AndroidSmsAppInstallingStatusObserver::Factory::Get()->BuildInstance(
+            fake_host_status_provider_.get(),
+            std::move(fake_android_sms_app_helper_delegate));
+
+    EXPECT_FALSE(fake_app_helper_delegate()->HasInstalledApp());
+  }
+
+  void SetHostWithStatus(
+      mojom::HostStatus host_status,
+      const base::Optional<cryptauth::RemoteDeviceRef>& host_device) {
+    fake_host_status_provider_->SetHostWithStatus(host_status, host_device);
+  }
+
+  FakeAndroidSmsAppHelperDelegate* fake_app_helper_delegate() {
+    return fake_android_sms_app_helper_delegate_;
+  }
+
+  cryptauth::RemoteDeviceRef GetFakePhone() {
+    return cryptauth::RemoteDeviceRefBuilder()
+        .SetPublicKey(kFakePhoneKey)
+        .SetName(kFakePhoneName)
+        .Build();
+  }
+
+ private:
+  std::unique_ptr<FakeHostStatusProvider> fake_host_status_provider_;
+  FakeAndroidSmsAppHelperDelegate* fake_android_sms_app_helper_delegate_;
+
+  std::unique_ptr<AndroidSmsAppInstallingStatusObserver>
+      android_sms_app_installing_status_observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(
+      MultiDeviceSetupAndroidSmsAppInstallingStatusObserverTest);
+};
+
+TEST_F(MultiDeviceSetupAndroidSmsAppInstallingStatusObserverTest,
+       InstallsAfterHostPending) {
+  SetHostWithStatus(mojom::HostStatus::kEligibleHostExistsButNoHostSet,
+                    base::nullopt /* host_device */);
+  EXPECT_FALSE(fake_app_helper_delegate()->HasInstalledApp());
+
+  SetHostWithStatus(
+      mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
+      GetFakePhone());
+  EXPECT_TRUE(fake_app_helper_delegate()->HasInstalledApp());
+}
+
+TEST_F(MultiDeviceSetupAndroidSmsAppInstallingStatusObserverTest,
+       InstallsAfterHostVerified) {
+  SetHostWithStatus(mojom::HostStatus::kNoEligibleHosts,
+                    base::nullopt /* host_device */);
+  EXPECT_FALSE(fake_app_helper_delegate()->HasInstalledApp());
+
+  SetHostWithStatus(mojom::HostStatus::kHostVerified, GetFakePhone());
+  EXPECT_TRUE(fake_app_helper_delegate()->HasInstalledApp());
+}
+}  // namespace multidevice_setup
+
+}  // namespace chromeos

chromeos/services/multidevice_setup/multidevice_setup_impl.cc

@@ -9,6 +9,7 @@
 #include "base/time/default_clock.h"
 #include "chromeos/components/proximity_auth/logging/logging.h"
 #include "chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.h"
+#include "chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.h"
 #include "chromeos/services/multidevice_setup/device_reenroller.h"
 #include "chromeos/services/multidevice_setup/eligible_host_devices_provider_impl.h"
 #include "chromeos/services/multidevice_setup/feature_state_manager_impl.h"
@@ -74,9 +75,7 @@ MultiDeviceSetupImpl::MultiDeviceSetupImpl(
     std::unique_ptr<AndroidSmsPairingStateTracker>
         android_sms_pairing_state_tracker,
     const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider)
-    : android_sms_app_helper_delegate_(
-          std::move(android_sms_app_helper_delegate)),
-      eligible_host_devices_provider_(
+    : eligible_host_devices_provider_(
           EligibleHostDevicesProviderImpl::Factory::Get()->BuildInstance(
               device_sync_client)),
       host_backend_delegate_(
@@ -113,6 +112,10 @@ MultiDeviceSetupImpl::MultiDeviceSetupImpl(
       device_reenroller_(DeviceReenroller::Factory::Get()->BuildInstance(
           device_sync_client,
           gcm_device_info_provider)),
+      android_sms_app_installing_host_observer_(
+          AndroidSmsAppInstallingStatusObserver::Factory::Get()->BuildInstance(
+              host_status_provider_.get(),
+              std::move(android_sms_app_helper_delegate))),
       auth_token_validator_(auth_token_validator) {
   host_status_provider_->AddObserver(this);
   feature_state_manager_->AddObserver(this);
@@ -302,7 +305,6 @@ bool MultiDeviceSetupImpl::AttemptSetHost(const std::string& host_device_id) {
     return false;
 
   host_backend_delegate_->AttemptToSetMultiDeviceHostOnBackend(*it);
-  android_sms_app_helper_delegate_->InstallAndroidSmsApp();
 
   return true;
 }

chromeos/services/multidevice_setup/multidevice_setup_impl.h

@@ -32,6 +32,7 @@ namespace multidevice_setup {
 
 class AccountStatusChangeDelegateNotifier;
 class AndroidSmsAppHelperDelegate;
+class AndroidSmsAppInstallingStatusObserver;
 class AndroidSmsPairingStateTracker;
 class AuthTokenValidator;
 class DeviceReenroller;
@@ -124,7 +125,6 @@ class MultiDeviceSetupImpl : public MultiDeviceSetupBase,
 
   void FlushForTesting();
 
-  std::unique_ptr<AndroidSmsAppHelperDelegate> android_sms_app_helper_delegate_;
   std::unique_ptr<EligibleHostDevicesProvider> eligible_host_devices_provider_;
   std::unique_ptr<HostBackendDelegate> host_backend_delegate_;
   std::unique_ptr<HostVerifier> host_verifier_;
@@ -133,6 +133,8 @@ class MultiDeviceSetupImpl : public MultiDeviceSetupBase,
   std::unique_ptr<SetupFlowCompletionRecorder> setup_flow_completion_recorder_;
   std::unique_ptr<AccountStatusChangeDelegateNotifier> delegate_notifier_;
   std::unique_ptr<DeviceReenroller> device_reenroller_;
+  std::unique_ptr<AndroidSmsAppInstallingStatusObserver>
+      android_sms_app_installing_host_observer_;
   AuthTokenValidator* auth_token_validator_;
 
   mojo::InterfacePtrSet<mojom::HostStatusObserver> host_status_observers_;

chromeos/services/multidevice_setup/multidevice_setup_impl_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/test/scoped_task_environment.h"
 #include "chromeos/services/device_sync/public/cpp/fake_device_sync_client.h"
 #include "chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.h"
+#include "chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.h"
 #include "chromeos/services/multidevice_setup/device_reenroller.h"
 #include "chromeos/services/multidevice_setup/eligible_host_devices_provider_impl.h"
 #include "chromeos/services/multidevice_setup/fake_account_status_change_delegate.h"
@@ -392,6 +393,39 @@ class FakeDeviceReenrollerFactory : public DeviceReenroller::Factory {
   DISALLOW_COPY_AND_ASSIGN(FakeDeviceReenrollerFactory);
 };
 
+class FakeAndroidSmsAppInstallingStatusObserverFactory
+    : public AndroidSmsAppInstallingStatusObserver::Factory {
+ public:
+  FakeAndroidSmsAppInstallingStatusObserverFactory(
+      FakeHostStatusProviderFactory* fake_host_status_provider_factory,
+      AndroidSmsAppHelperDelegate* expected_android_sms_app_helper_delegate)
+      : fake_host_status_provider_factory_(fake_host_status_provider_factory),
+        expected_android_sms_app_helper_delegate_(
+            expected_android_sms_app_helper_delegate) {}
+
+  ~FakeAndroidSmsAppInstallingStatusObserverFactory() override = default;
+
+ private:
+  // AndroidSmsAppInstallingStatusObserver::Factory:
+  std::unique_ptr<AndroidSmsAppInstallingStatusObserver> BuildInstance(
+      HostStatusProvider* host_status_provider,
+      std::unique_ptr<AndroidSmsAppHelperDelegate>
+          android_sms_app_helper_delegate) override {
+    EXPECT_EQ(fake_host_status_provider_factory_->instance(),
+              host_status_provider);
+    EXPECT_EQ(expected_android_sms_app_helper_delegate_,
+              android_sms_app_helper_delegate.get());
+    // Only check inputs and return nullptr. We do not want to trigger the
+    // AndroidSmsAppInstallingStatusObserver logic in these unit tests.
+    return nullptr;
+  }
+
+  FakeHostStatusProviderFactory* fake_host_status_provider_factory_;
+  AndroidSmsAppHelperDelegate* expected_android_sms_app_helper_delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeAndroidSmsAppInstallingStatusObserverFactory);
+};
+
 }  // namespace
 
 class MultiDeviceSetupImplTest : public testing::Test {
@@ -479,6 +513,13 @@ class MultiDeviceSetupImplTest : public testing::Test {
     DeviceReenroller::Factory::SetFactoryForTesting(
         fake_device_reenroller_factory_.get());
 
+    fake_android_sms_app_installing_status_observer_factory_ =
+        std::make_unique<FakeAndroidSmsAppInstallingStatusObserverFactory>(
+            fake_host_status_provider_factory_.get(),
+            fake_android_sms_app_helper_delegate_);
+    AndroidSmsAppInstallingStatusObserver::Factory::SetFactoryForTesting(
+        fake_android_sms_app_installing_status_observer_factory_.get());
+
     multidevice_setup_ = MultiDeviceSetupImpl::Factory::Get()->BuildInstance(
         test_pref_service_.get(), fake_device_sync_client_.get(),
         fake_auth_token_validator_.get(),
@@ -497,6 +538,8 @@ class MultiDeviceSetupImplTest : public testing::Test {
     AccountStatusChangeDelegateNotifierImpl::Factory::SetFactoryForTesting(
         nullptr);
     DeviceReenroller::Factory::SetFactoryForTesting(nullptr);
+    AndroidSmsAppInstallingStatusObserver::Factory::SetFactoryForTesting(
+        nullptr);
   }
 
   void CallSetAccountStatusChangeDelegate() {
@@ -780,6 +823,8 @@ class MultiDeviceSetupImplTest : public testing::Test {
   std::unique_ptr<FakeAccountStatusChangeDelegateNotifierFactory>
       fake_account_status_change_delegate_notifier_factory_;
   std::unique_ptr<FakeDeviceReenrollerFactory> fake_device_reenroller_factory_;
+  std::unique_ptr<FakeAndroidSmsAppInstallingStatusObserverFactory>
+      fake_android_sms_app_installing_status_observer_factory_;
   FakeAndroidSmsAppHelperDelegate* fake_android_sms_app_helper_delegate_;
   FakeAndroidSmsPairingStateTracker* fake_android_sms_pairing_state_tracker_;
 

chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.cc

@@ -32,6 +32,11 @@ bool FakeAndroidSmsAppHelperDelegate::HasLaunchedApp() {
   return has_launched_;
 }
 
+void FakeAndroidSmsAppHelperDelegate::Reset() {
+  has_installed_ = false;
+  has_launched_ = false;
+}
+
 }  // namespace multidevice_setup
 
 }  // namespace chromeos

chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h

@@ -17,6 +17,7 @@ class FakeAndroidSmsAppHelperDelegate : public AndroidSmsAppHelperDelegate {
   ~FakeAndroidSmsAppHelperDelegate() override;
   bool HasInstalledApp();
   bool HasLaunchedApp();
+  void Reset();
 
   // AndroidSmsAppHelperDelegate:
   void InstallAndroidSmsApp() override;