Stop using chromeos::FakeSessionManagerClient in ArcSessionImpl tests

It's no longer necessary as we've introduced our own abstraction
layer, arc::ArcClientAdapter. This CL injects a fake ArcClientAdapter
implementation to the test fixture to simplify the test. That allows
us to remove all usage of chromeos:: and login_manager:: from the
test, and the test no longer depends on the internals of the ARC
container.

BUG=b:143175953
TEST=try

Change-Id: Ia39f492a94b5698794df423b181ff935b944eec4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008284Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733888}

components/arc/session/arc_session_impl.cc

@@ -99,6 +99,7 @@ class ArcSessionDelegateImpl : public ArcSessionImpl::Delegate {
   void GetLcdDensity(GetLcdDensityCallback callback) override;
   void GetFreeDiskSpace(GetFreeDiskSpaceCallback callback) override;
   version_info::Channel GetChannel() override;
+  std::unique_ptr<ArcClientAdapter> CreateClient() override;
 
  private:
   // Synchronously create a UNIX domain socket. This is designed to run on a
@@ -192,6 +193,10 @@ version_info::Channel ArcSessionDelegateImpl::GetChannel() {
   return channel_;
 }
 
+std::unique_ptr<ArcClientAdapter> ArcSessionDelegateImpl::CreateClient() {
+  return ArcClientAdapter::Create(GetChannel());
+}
+
 // static
 base::ScopedFD ArcSessionDelegateImpl::CreateSocketInternal() {
   auto endpoint = mojo::NamedPlatformChannel({kArcBridgeSocketPath});
@@ -315,7 +320,7 @@ ArcSessionImpl::ArcSessionImpl(std::unique_ptr<Delegate> delegate,
                                chromeos::SchedulerConfigurationManagerBase*
                                    scheduler_configuration_manager)
     : delegate_(std::move(delegate)),
-      client_(ArcClientAdapter::Create(delegate_->GetChannel())),
+      client_(delegate_->CreateClient()),
       scheduler_configuration_manager_(scheduler_configuration_manager) {
   DCHECK(client_);
   client_->AddObserver(this);

components/arc/session/arc_session_impl.h

@@ -169,11 +169,14 @@ class ArcSessionImpl
 
     // Returns the channel for the installation.
     virtual version_info::Channel GetChannel() = 0;
+
+    // Creates and returns a client adapter.
+    virtual std::unique_ptr<ArcClientAdapter> CreateClient() = 0;
   };
 
   ArcSessionImpl(std::unique_ptr<Delegate> delegate,
                  chromeos::SchedulerConfigurationManagerBase*
-                     scheduler_configuration_manager_);
+                     scheduler_configuration_manager);
   ~ArcSessionImpl() override;
 
   // Returns default delegate implementation used for the production.
@@ -183,6 +186,7 @@ class ArcSessionImpl
       version_info::Channel channel);
 
   State GetStateForTesting() { return state_; }
+  ArcClientAdapter* GetClientForTesting() { return client_.get(); }
 
   // ArcSession overrides:
   void StartMiniInstance() override;

components/arc/session/arc_session_impl_unittest.cc

@@ -11,27 +11,25 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chromeos/constants/chromeos_switches.h"
-#include "chromeos/dbus/session_manager/fake_session_manager_client.h"
 #include "chromeos/system/scheduler_configuration_manager_base.h"
-#include "components/account_id/account_id.h"
+#include "components/arc/session/arc_client_adapter.h"
 #include "components/arc/session/arc_session_impl.h"
+#include "components/arc/session/arc_start_params.h"
+#include "components/arc/session/arc_upgrade_params.h"
 #include "components/arc/test/fake_arc_bridge_host.h"
-#include "components/user_manager/fake_user_manager.h"
-#include "components/user_manager/scoped_user_manager.h"
 #include "components/version_info/channel.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace arc {
 namespace {
 
-constexpr char kFakeGmail[] = "user@gmail.com";
-constexpr char kFakeGmailGaiaId[] = "1234567890";
 constexpr char kDefaultLocale[] = "en-US";
 
 UpgradeParams DefaultUpgradeParams() {
@@ -40,6 +38,78 @@ UpgradeParams DefaultUpgradeParams() {
   return params;
 }
 
+// An ArcClientAdapter implementation that does the same as the real ones but
+// without any D-Bus calls.
+class FakeArcClientAdapter : public ArcClientAdapter {
+ public:
+  FakeArcClientAdapter() = default;
+  ~FakeArcClientAdapter() override = default;
+
+  FakeArcClientAdapter(const FakeArcClientAdapter&) = delete;
+  FakeArcClientAdapter& operator=(const FakeArcClientAdapter&) = delete;
+
+  // ArcClientAdapter overrides:
+  void StartMiniArc(StartParams params,
+                    chromeos::VoidDBusMethodCallback callback) override {
+    last_start_params_ = std::move(params);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(&FakeArcClientAdapter::OnMiniArcStarted,
+                                  base::Unretained(this), std::move(callback),
+                                  arc_available_));
+  }
+
+  void UpgradeArc(UpgradeParams params,
+                  chromeos::VoidDBusMethodCallback callback) override {
+    last_upgrade_params_ = std::move(params);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(&FakeArcClientAdapter::OnArcUpgraded,
+                                  base::Unretained(this), std::move(callback),
+                                  !force_upgrade_failure_));
+  }
+
+  void StopArcInstance(bool on_shutdown) override {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&FakeArcClientAdapter::NotifyArcInstanceStopped,
+                       base::Unretained(this)));
+  }
+
+  void SetUserInfo(const std::string& hash,
+                   const std::string& serial_number) override {}
+
+  // Notifies ArcSessionImpl of the ARC instance stop event.
+  void NotifyArcInstanceStopped() {
+    for (auto& observer : observer_list_)
+      observer.ArcInstanceStopped();
+  }
+
+  void set_arc_available(bool arc_available) { arc_available_ = arc_available; }
+  void set_force_upgrade_failure(bool force_upgrade_failure) {
+    force_upgrade_failure_ = force_upgrade_failure;
+  }
+  const StartParams& last_start_params() const { return last_start_params_; }
+  const UpgradeParams& last_upgrade_params() const {
+    return last_upgrade_params_;
+  }
+
+ private:
+  void OnMiniArcStarted(chromeos::VoidDBusMethodCallback callback,
+                        bool result) {
+    std::move(callback).Run(result);
+  }
+
+  void OnArcUpgraded(chromeos::VoidDBusMethodCallback callback, bool result) {
+    std::move(callback).Run(result);
+    if (!result)
+      NotifyArcInstanceStopped();
+  }
+
+  bool arc_available_ = true;
+  bool force_upgrade_failure_ = false;
+  StartParams last_start_params_;
+  UpgradeParams last_upgrade_params_;
+};
+
 class FakeDelegate : public ArcSessionImpl::Delegate {
  public:
   explicit FakeDelegate(int32_t lcd_density = 160)
@@ -99,6 +169,10 @@ class FakeDelegate : public ArcSessionImpl::Delegate {
     return version_info::Channel::DEFAULT;
   }
 
+  std::unique_ptr<ArcClientAdapter> CreateClient() override {
+    return std::make_unique<FakeArcClientAdapter>();
+  }
+
   void SetLcdDensity(int32_t lcd_density) {
     lcd_density_ = lcd_density;
     ASSERT_TRUE(!lcd_density_callback_.is_null());
@@ -201,32 +275,8 @@ class FakeSchedulerConfigurationManager
 
 class ArcSessionImplTest : public testing::Test {
  public:
-  ArcSessionImplTest() {
-    // Create a user and set it as the primary user.
-    const AccountId account_id =
-        AccountId::FromUserEmailGaiaId(kFakeGmail, kFakeGmailGaiaId);
-    const user_manager::User* user = GetUserManager()->AddUser(account_id);
-    GetUserManager()->UserLoggedIn(account_id, user->username_hash(),
-                                   false /* browser_restart */,
-                                   false /* is_child */);
-  }
-
-  ~ArcSessionImplTest() override {
-    GetUserManager()->RemoveUserFromList(
-        AccountId::FromUserEmailGaiaId(kFakeGmail, kFakeGmailGaiaId));
-  }
-
-  void SetUp() override {
-    chromeos::SessionManagerClient::InitializeFakeInMemory();
-    chromeos::FakeSessionManagerClient::Get()->set_arc_available(true);
-  }
-
-  void TearDown() override { chromeos::SessionManagerClient::Shutdown(); }
-
-  user_manager::FakeUserManager* GetUserManager() {
-    return static_cast<user_manager::FakeUserManager*>(
-        user_manager::UserManager::Get());
-  }
+  ArcSessionImplTest() = default;
+  ~ArcSessionImplTest() override = default;
 
   std::unique_ptr<ArcSessionImpl, ArcSessionDeleter> CreateArcSession(
       std::unique_ptr<ArcSessionImpl::Delegate> delegate = nullptr,
@@ -254,6 +304,10 @@ class ArcSessionImplTest : public testing::Test {
   }
 
  protected:
+  FakeArcClientAdapter* GetClient(ArcSessionImpl* session) {
+    return static_cast<FakeArcClientAdapter*>(session->GetClientForTesting());
+  }
+
   FakeSchedulerConfigurationManager fake_schedule_configuration_manager_;
 
  private:
@@ -268,8 +322,6 @@ class ArcSessionImplTest : public testing::Test {
   }
 
   base::test::TaskEnvironment task_environment_;
-  user_manager::ScopedUserManager scoped_user_manager_{
-      std::make_unique<user_manager::FakeUserManager>()};
 
   DISALLOW_COPY_AND_ASSIGN(ArcSessionImplTest);
 };
@@ -286,13 +338,12 @@ TEST_F(ArcSessionImplTest, MiniInstance_Success) {
   EXPECT_FALSE(observer.on_session_stopped_args().has_value());
 }
 
-// SessionManagerClient::StartArcMiniContainer() reports an error, causing the
-// mini-container start to fail.
+// ArcClientAdapter::StartMiniArc() reports an error, causing the mini instance
+// start to fail.
 TEST_F(ArcSessionImplTest, MiniInstance_DBusFail) {
-  chromeos::FakeSessionManagerClient::Get()->set_arc_available(false);
-
   auto arc_session = CreateArcSession();
   TestArcSessionObserver observer(arc_session.get());
+  GetClient(arc_session.get())->set_arc_available(false);
   arc_session->StartMiniInstance();
   base::RunLoop().RunUntilIdle();
 
@@ -304,7 +355,7 @@ TEST_F(ArcSessionImplTest, MiniInstance_DBusFail) {
   EXPECT_FALSE(observer.on_session_stopped_args()->upgrade_requested);
 }
 
-// SessionManagerClient::UpgradeArcContainer() reports an error due to low disk,
+// ArcClientAdapter::UpgradeArc() reports an error due to low disk,
 // causing the container upgrade to fail to start container with reason
 // LOW_DISK_SPACE.
 TEST_F(ArcSessionImplTest, Upgrade_LowDisk) {
@@ -346,16 +397,15 @@ TEST_F(ArcSessionImplTest, Upgrade_Success) {
   EXPECT_FALSE(observer.on_session_stopped_args().has_value());
 }
 
-// SessionManagerClient::UpgradeArcContainer() reports an error, then the
-// upgrade fails.
+// ArcClientAdapter::UpgradeArc() reports an error, then the upgrade fails.
 TEST_F(ArcSessionImplTest, Upgrade_DBusFail) {
   // Set up. Start a mini instance.
   auto arc_session = CreateArcSession();
   TestArcSessionObserver observer(arc_session.get());
   ASSERT_NO_FATAL_FAILURE(SetupMiniContainer(arc_session.get(), &observer));
 
-  // Hereafter, let SessionManagerClient::UpgradeArcContainer() fail.
-  chromeos::FakeSessionManagerClient::Get()->set_force_upgrade_failure(true);
+  // Hereafter, let ArcClientAdapter::UpgradeArc() fail.
+  GetClient(arc_session.get())->set_force_upgrade_failure(true);
 
   // Then upgrade, which should fail.
   arc_session->RequestUpgrade(DefaultUpgradeParams());
@@ -573,8 +623,8 @@ TEST_F(ArcSessionImplTest, ArcStopInstance) {
   ASSERT_EQ(ArcSessionImpl::State::RUNNING_FULL_INSTANCE,
             arc_session->GetStateForTesting());
 
-  // Deliver the ArcInstanceStopped D-Bus signal.
-  chromeos::FakeSessionManagerClient::Get()->NotifyArcInstanceStopped();
+  // Notify ArcClientAdapter's observers of the crash event.
+  GetClient(arc_session.get())->NotifyArcInstanceStopped();
 
   EXPECT_EQ(ArcSessionImpl::State::STOPPED, arc_session->GetStateForTesting());
   ASSERT_TRUE(observer.on_session_stopped_args().has_value());
@@ -587,24 +637,18 @@ struct PackagesCacheModeState {
   // Possible values for chromeos::switches::kArcPackagesCacheMode
   const char* chrome_switch;
   bool full_container;
-  login_manager::UpgradeArcContainerRequest_PackageCacheMode
-      expected_packages_cache_mode;
+  UpgradeParams::PackageCacheMode expected_packages_cache_mode;
 };
 
 constexpr PackagesCacheModeState kPackagesCacheModeStates[] = {
-    {nullptr, true,
-     login_manager::UpgradeArcContainerRequest_PackageCacheMode_DEFAULT},
-    {nullptr, false,
-     login_manager::UpgradeArcContainerRequest_PackageCacheMode_DEFAULT},
+    {nullptr, true, UpgradeParams::PackageCacheMode::DEFAULT},
+    {nullptr, false, UpgradeParams::PackageCacheMode::DEFAULT},
     {kPackagesCacheModeCopy, true,
-     login_manager::UpgradeArcContainerRequest_PackageCacheMode_COPY_ON_INIT},
-    {kPackagesCacheModeCopy, false,
-     login_manager::UpgradeArcContainerRequest_PackageCacheMode_DEFAULT},
+     UpgradeParams::PackageCacheMode::COPY_ON_INIT},
+    {kPackagesCacheModeCopy, false, UpgradeParams::PackageCacheMode::DEFAULT},
     {kPackagesCacheModeSkipCopy, true,
-     login_manager::
-         UpgradeArcContainerRequest_PackageCacheMode_SKIP_SETUP_COPY_ON_INIT},
-    {kPackagesCacheModeCopy, false,
-     login_manager::UpgradeArcContainerRequest_PackageCacheMode_DEFAULT},
+     UpgradeParams::PackageCacheMode::SKIP_SETUP_COPY_ON_INIT},
+    {kPackagesCacheModeCopy, false, UpgradeParams::PackageCacheMode::DEFAULT},
 };
 
 class ArcSessionImplPackagesCacheModeTest
@@ -625,10 +669,9 @@ TEST_P(ArcSessionImplPackagesCacheModeTest, PackagesCacheModes) {
   if (state.full_container)
     arc_session->RequestUpgrade(DefaultUpgradeParams());
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(state.expected_packages_cache_mode,
-            chromeos::FakeSessionManagerClient::Get()
-                ->last_upgrade_arc_request()
-                .packages_cache_mode());
+  EXPECT_EQ(
+      state.expected_packages_cache_mode,
+      GetClient(arc_session.get())->last_upgrade_params().packages_cache_mode);
 }
 
 INSTANTIATE_TEST_SUITE_P(All,
@@ -650,9 +693,9 @@ TEST_P(ArcSessionImplGmsCoreCacheTest, GmsCoreCaches) {
   arc_session->StartMiniInstance();
   arc_session->RequestUpgrade(DefaultUpgradeParams());
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(GetParam(), chromeos::FakeSessionManagerClient::Get()
-                            ->last_upgrade_arc_request()
-                            .skip_gms_core_cache());
+  EXPECT_EQ(
+      GetParam(),
+      GetClient(arc_session.get())->last_upgrade_params().skip_gms_core_cache);
 }
 
 INSTANTIATE_TEST_SUITE_P(All,
@@ -663,8 +706,8 @@ TEST_F(ArcSessionImplTest, DemoSession) {
   auto arc_session = CreateArcSession();
   arc_session->StartMiniInstance();
 
-  const std::string demo_apps_path =
-      "/run/imageloader/demo_mode_resources/android_apps.squash";
+  const base::FilePath demo_apps_path(
+      "/run/imageloader/demo_mode_resources/android_apps.squash");
   UpgradeParams params;
   params.is_demo_session = true;
   params.demo_session_apps_path = base::FilePath(demo_apps_path);
@@ -672,12 +715,11 @@ TEST_F(ArcSessionImplTest, DemoSession) {
   arc_session->RequestUpgrade(std::move(params));
 
   base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(chromeos::FakeSessionManagerClient::Get()
-                  ->last_upgrade_arc_request()
-                  .is_demo_session());
-  EXPECT_EQ(demo_apps_path, chromeos::FakeSessionManagerClient::Get()
-                                ->last_upgrade_arc_request()
-                                .demo_session_apps_path());
+  EXPECT_TRUE(
+      GetClient(arc_session.get())->last_upgrade_params().is_demo_session);
+  EXPECT_EQ(demo_apps_path, GetClient(arc_session.get())
+                                ->last_upgrade_params()
+                                .demo_session_apps_path);
 }
 
 TEST_F(ArcSessionImplTest, DemoSessionWithoutOfflineDemoApps) {
@@ -690,12 +732,11 @@ TEST_F(ArcSessionImplTest, DemoSessionWithoutOfflineDemoApps) {
   arc_session->RequestUpgrade(std::move(params));
 
   base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(chromeos::FakeSessionManagerClient::Get()
-                  ->last_upgrade_arc_request()
-                  .is_demo_session());
-  EXPECT_EQ(std::string(), chromeos::FakeSessionManagerClient::Get()
-                               ->last_upgrade_arc_request()
-                               .demo_session_apps_path());
+  EXPECT_TRUE(
+      GetClient(arc_session.get())->last_upgrade_params().is_demo_session);
+  EXPECT_EQ(base::FilePath(), GetClient(arc_session.get())
+                                  ->last_upgrade_params()
+                                  .demo_session_apps_path);
 }
 
 TEST_F(ArcSessionImplTest, SupervisionTransitionShouldGraduate) {
@@ -708,15 +749,11 @@ TEST_F(ArcSessionImplTest, SupervisionTransitionShouldGraduate) {
   arc_session->RequestUpgrade(std::move(params));
 
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(
-      login_manager::
-          UpgradeArcContainerRequest_SupervisionTransition_CHILD_TO_REGULAR,
-      chromeos::FakeSessionManagerClient::Get()
-          ->last_upgrade_arc_request()
-          .supervision_transition());
-  EXPECT_EQ(160, chromeos::FakeSessionManagerClient::Get()
-                     ->last_start_arc_mini_container_request()
-                     .lcd_density());
+  EXPECT_EQ(ArcSupervisionTransition::CHILD_TO_REGULAR,
+            GetClient(arc_session.get())
+                ->last_upgrade_params()
+                .supervision_transition);
+  EXPECT_EQ(160, GetClient(arc_session.get())->last_start_params().lcd_density);
 }
 
 TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensity) {
@@ -729,9 +766,7 @@ TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensity) {
 
   EXPECT_EQ(ArcSessionImpl::State::RUNNING_MINI_INSTANCE,
             arc_session->GetStateForTesting());
-  EXPECT_EQ(240, chromeos::FakeSessionManagerClient::Get()
-                     ->last_start_arc_mini_container_request()
-                     .lcd_density());
+  EXPECT_EQ(240, GetClient(arc_session.get())->last_start_params().lcd_density);
 }
 
 TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensityAsync) {
@@ -749,9 +784,7 @@ TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensityAsync) {
             arc_session->GetStateForTesting());
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(240, chromeos::FakeSessionManagerClient::Get()
-                     ->last_start_arc_mini_container_request()
-                     .lcd_density());
+  EXPECT_EQ(240, GetClient(arc_session.get())->last_start_params().lcd_density);
 }
 
 TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensityAsyncReversedOrder) {
@@ -768,9 +801,7 @@ TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensityAsyncReversedOrder) {
             arc_session->GetStateForTesting());
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(240, chromeos::FakeSessionManagerClient::Get()
-                     ->last_start_arc_mini_container_request()
-                     .lcd_density());
+  EXPECT_EQ(240, GetClient(arc_session.get())->last_start_params().lcd_density);
 }
 
 TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensityAsyncCpuInfoEarly) {
@@ -787,9 +818,7 @@ TEST_F(ArcSessionImplTest, StartArcMiniContainerWithDensityAsyncCpuInfoEarly) {
             arc_session->GetStateForTesting());
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(240, chromeos::FakeSessionManagerClient::Get()
-                     ->last_start_arc_mini_container_request()
-                     .lcd_density());
+  EXPECT_EQ(240, GetClient(arc_session.get())->last_start_params().lcd_density);
 }
 
 TEST_F(ArcSessionImplTest, StopWhileWaitingForLcdDensity) {