Start ARC and sign in after Chrome OS login

chrome/browser/chromeos/arc/arc_session_manager_unittest.cc

Index: chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
diff --git a/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc b/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
index efdfe629e5ffb024eff224ebad20fb3453129379..42d18a230ddd0d4b2d8d95eab674a4ceb929ca41 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
+++ b/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
@@ -35,6 +35,7 @@
 #include "chrome/browser/prefs/pref_service_syncable_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_test.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
@@ -64,6 +65,11 @@ namespace arc {
 
 namespace {
 
+enum ArcAlwaysStartOption : bool {
+  ON = true,
+  OFF = false,
+};
+
 class FakeLoginDisplayHost : public chromeos::LoginDisplayHost {
  public:
   FakeLoginDisplayHost() {
@@ -203,12 +209,14 @@ class ArcSessionManagerTestBase : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(ArcSessionManagerTestBase);
 };
 
-class ArcSessionManagerTest : public ArcSessionManagerTestBase {
+// Intermediate class so that the children can inject test parameter freely.
+class AbstractArcSessionManagerTest : public ArcSessionManagerTestBase {
  public:
-  ArcSessionManagerTest() = default;
-
+  AbstractArcSessionManagerTest() = default;
   void SetUp() override {
     ArcSessionManagerTestBase::SetUp();
+    if (ShouldArcAlwaysStartInTest() == ArcAlwaysStartOption::ON)
+      SetArcAlwaysStartForTesting();
 
     const AccountId account_id(AccountId::FromUserEmailGaiaId(
         profile()->GetProfileUserName(), "1234567890"));
@@ -220,11 +228,37 @@ class ArcSessionManagerTest : public ArcSessionManagerTestBase {
     ASSERT_TRUE(arc_session_manager()->IsSessionStopped());
   }
 
+ protected:
+  virtual ArcAlwaysStartOption ShouldArcAlwaysStartInTest() = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AbstractArcSessionManagerTest);
+};
+
+class ArcSessionManagerTest
+    : public AbstractArcSessionManagerTest,
+      public testing::WithParamInterface<ArcAlwaysStartOption> {
+ public:
+  ArcSessionManagerTest() = default;
+
+ protected:
+  ArcAlwaysStartOption ShouldArcAlwaysStartInTest() override {
+    return GetParam();
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ArcSessionManagerTest);
 };
 
-TEST_F(ArcSessionManagerTest, BaseWorkflow) {
+INSTANTIATE_TEST_CASE_P(,
+                        ArcSessionManagerTest,
+                        testing::Values(ArcAlwaysStartOption::OFF,
+                                        ArcAlwaysStartOption::ON));
+
+TEST_P(ArcSessionManagerTest, BaseWorkflow) {
+  if (ShouldArcAlwaysStart())
+    return;
+
   EXPECT_TRUE(arc_session_manager()->sign_in_start_time().is_null());
   EXPECT_TRUE(arc_session_manager()->arc_start_time().is_null());
 
@@ -252,7 +286,40 @@ TEST_F(ArcSessionManagerTest, BaseWorkflow) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, CancelFetchingDisablesArc) {
+TEST_P(ArcSessionManagerTest, BaseWorkflowWithArcAlwaysStart) {
+  if (!ShouldArcAlwaysStart())
+    return;
+
+  // TODO(victorhsieh): Consider also tracking sign-in activity, which is
+  // initiated from the Android side.
+  EXPECT_TRUE(arc_session_manager()->arc_start_time().is_null());
+
+  arc_session_manager()->SetProfile(profile());
+
+  // By default ARC is not enabled.
+  EXPECT_EQ(ArcSessionManager::State::STOPPED, arc_session_manager()->state());
+
+  // When ARC is always started, ArcSessionManager should always be in ACTIVE
+  // state.
+  arc_session_manager()->RequestEnable();
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
+
+  arc_session_manager()->StartArcForTesting();
+
+  EXPECT_FALSE(arc_session_manager()->arc_start_time().is_null());
+
+  ASSERT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
+  ASSERT_TRUE(arc_session_manager()->IsSessionRunning());
+
+  arc_session_manager()->Shutdown();
+}
+
+TEST_P(ArcSessionManagerTest, CancelFetchingDisablesArc) {
+  // TODO(victorhsieh): Implement opt-in flow on Persistent ARC.
+  if (ShouldArcAlwaysStart())
+    return;
+
   SetArcPlayStoreEnabledForProfile(profile(), true);
 
   // Starts ARC.
@@ -278,7 +345,11 @@ TEST_F(ArcSessionManagerTest, CancelFetchingDisablesArc) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, CloseUIKeepsArcEnabled) {
+TEST_P(ArcSessionManagerTest, CloseUIKeepsArcEnabled) {
+  // TODO(victorhsieh): Implement opt-in flow.
+  if (ShouldArcAlwaysStart())
+    return;
+
   // Starts ARC.
   SetArcPlayStoreEnabledForProfile(profile(), true);
   arc_session_manager()->SetProfile(profile());
@@ -298,7 +369,7 @@ TEST_F(ArcSessionManagerTest, CloseUIKeepsArcEnabled) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, Provisioning_Success) {
+TEST_P(ArcSessionManagerTest, Provisioning_Success) {
   PrefService* const prefs = profile()->GetPrefs();
 
   EXPECT_TRUE(arc_session_manager()->sign_in_start_time().is_null());
@@ -309,12 +380,17 @@ TEST_F(ArcSessionManagerTest, Provisioning_Success) {
 
   arc_session_manager()->SetProfile(profile());
   arc_session_manager()->RequestEnable();
-  ASSERT_EQ(ArcSessionManager::State::SHOWING_TERMS_OF_SERVICE,
-            arc_session_manager()->state());
+  if (ShouldArcAlwaysStart()) {
+    ASSERT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
+  } else {
+    ASSERT_EQ(ArcSessionManager::State::SHOWING_TERMS_OF_SERVICE,
+              arc_session_manager()->state());
+  }
 
   // Emulate to accept the terms of service.
   prefs->SetBoolean(prefs::kArcTermsAccepted, true);
-  arc_session_manager()->StartArcForTesting();
+  if (!ShouldArcAlwaysStart())
+    arc_session_manager()->StartArcForTesting();
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
   EXPECT_TRUE(arc_session_manager()->IsSessionRunning());
 
@@ -332,7 +408,7 @@ TEST_F(ArcSessionManagerTest, Provisioning_Success) {
   EXPECT_TRUE(arc_session_manager()->IsPlaystoreLaunchRequestedForTesting());
 }
 
-TEST_F(ArcSessionManagerTest, Provisioning_Restart) {
+TEST_P(ArcSessionManagerTest, Provisioning_Restart) {
   // Set up the situation that provisioning is successfully done in the
   // previous session.
   PrefService* const prefs = profile()->GetPrefs();
@@ -359,12 +435,17 @@ TEST_F(ArcSessionManagerTest, Provisioning_Restart) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, RemoveDataDir) {
+TEST_P(ArcSessionManagerTest, RemoveDataDir) {
+  // TODO(victorhsieh): Implement data removal on Persistent ARC.
+  if (ShouldArcAlwaysStart())
+    return;
+
   // Emulate the situation where the initial Google Play Store enabled
   // preference is false for managed user, i.e., data dir is being removed at
   // beginning.
   arc_session_manager()->SetProfile(profile());
   arc_session_manager()->RemoveArcData();
+
   EXPECT_TRUE(
       profile()->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested));
   EXPECT_EQ(ArcSessionManager::State::REMOVING_DATA_DIR,
@@ -398,7 +479,11 @@ TEST_F(ArcSessionManagerTest, RemoveDataDir) {
       profile()->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested));
 }
 
-TEST_F(ArcSessionManagerTest, RemoveDataDir_Restart) {
+TEST_P(ArcSessionManagerTest, RemoveDataDir_Restart) {
+  // TODO(victorhsieh): Implement data removal on Persistent ARC.
+  if (ShouldArcAlwaysStart())
+    return;
+
   // Emulate second sign-in. Data should be removed first and ARC started after.
   PrefService* const prefs = profile()->GetPrefs();
   prefs->SetBoolean(prefs::kArcDataRemoveRequested, true);
@@ -414,7 +499,7 @@ TEST_F(ArcSessionManagerTest, RemoveDataDir_Restart) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
+TEST_P(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
   arc_session_manager()->SetProfile(profile());
   arc_session_manager()->RequestEnable();
   arc_session_manager()->StartArcForTesting();
@@ -435,8 +520,9 @@ TEST_F(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
 }
 
 class ArcSessionManagerPolicyTest
-    : public ArcSessionManagerTest,
-      public testing::WithParamInterface<std::tuple<base::Value, base::Value>> {
+    : public AbstractArcSessionManagerTest,
+      public testing::WithParamInterface<
+          std::tuple<base::Value, base::Value, ArcAlwaysStartOption>> {
  public:
   const base::Value& backup_restore_pref_value() const {
     return std::get<0>(GetParam());
@@ -445,9 +531,18 @@ class ArcSessionManagerPolicyTest
   const base::Value& location_service_pref_value() const {
     return std::get<1>(GetParam());
   }
+
+ protected:
+  ArcAlwaysStartOption ShouldArcAlwaysStartInTest() override {
+    return std::get<2>(GetParam());
+  }
 };
 
 TEST_P(ArcSessionManagerPolicyTest, SkippingTerms) {
+  // TODO(victorhsieh): Implement opt-in flow.
+  if (ShouldArcAlwaysStart())
+    return;
+
   sync_preferences::TestingPrefServiceSyncable* const prefs =
       profile()->GetTestingPrefService();
 
@@ -510,11 +605,12 @@ TEST_P(ArcSessionManagerPolicyTest, SkippingTerms) {
 }
 
 INSTANTIATE_TEST_CASE_P(
-    ArcSessionManagerPolicyTest,
+    ,
     ArcSessionManagerPolicyTest,
     testing::Combine(
         testing::Values(base::Value(), base::Value(false), base::Value(true)),
-        testing::Values(base::Value(), base::Value(false), base::Value(true))));
+        testing::Values(base::Value(), base::Value(false), base::Value(true)),
+        testing::Values(ArcAlwaysStartOption::OFF, ArcAlwaysStartOption::ON)));
 
 class ArcSessionManagerKioskTest : public ArcSessionManagerTestBase {
  public:
@@ -550,11 +646,20 @@ TEST_F(ArcSessionManagerKioskTest, AuthFailure) {
   EXPECT_TRUE(terminated);
 }
 
-class ArcSessionOobeOptInTest : public ArcSessionManagerTest {
+// This class takes two test parameters because both itself and its child need
+// to be parameterized.  Having redundant parameter here avoid the trouble to
+// deal with multiple inheritance from WithParamInterface instances.
+class ArcSessionOobeOptInTest : public AbstractArcSessionManagerTest,
+                                public testing::WithParamInterface<
+                                    std::tuple<ArcAlwaysStartOption, bool>> {
  public:
   ArcSessionOobeOptInTest() = default;
 
  protected:
+  ArcAlwaysStartOption ShouldArcAlwaysStartInTest() override {
+    return std::get<0>(GetParam());
+  }
+
   void CreateLoginDisplayHost() {
     fake_login_display_host_ = base::MakeUnique<FakeLoginDisplayHost>();
   }
@@ -572,7 +677,14 @@ class ArcSessionOobeOptInTest : public ArcSessionManagerTest {
   DISALLOW_COPY_AND_ASSIGN(ArcSessionOobeOptInTest);
 };
 
-TEST_F(ArcSessionOobeOptInTest, OobeOptInActive) {
+INSTANTIATE_TEST_CASE_P(
+    ,
+    ArcSessionOobeOptInTest,
+    testing::Combine(testing::Values(ArcAlwaysStartOption::OFF,
+                                     ArcAlwaysStartOption::ON),
+                     testing::Values(false) /* dummy */));
+
+TEST_P(ArcSessionOobeOptInTest, OobeOptInActive) {
   // OOBE OptIn is active in case of OOBE is started for new user and ARC OOBE
   // is enabled by switch.
   EXPECT_FALSE(ArcSessionManager::IsOobeOptInActive());
@@ -593,8 +705,7 @@ TEST_F(ArcSessionOobeOptInTest, OobeOptInActive) {
 
 class ArcSessionOobeOptInNegotiatorTest
     : public ArcSessionOobeOptInTest,
-      public chromeos::ArcTermsOfServiceScreenView,
-      public testing::WithParamInterface<bool> {
+      public chromeos::ArcTermsOfServiceScreenView {
  public:
   ArcSessionOobeOptInNegotiatorTest() = default;
 
@@ -636,7 +747,11 @@ class ArcSessionOobeOptInNegotiatorTest
   }
 
  protected:
-  bool IsManagedUser() { return GetParam(); }
+  ArcAlwaysStartOption ShouldArcAlwaysStartInTest() override {
+    return std::get<0>(GetParam());
+  }
+
+  bool IsManagedUser() { return std::get<1>(GetParam()); }
 
   void ReportResult(bool accepted) {
     for (auto& observer : observer_list_) {
@@ -683,9 +798,12 @@ class ArcSessionOobeOptInNegotiatorTest
   DISALLOW_COPY_AND_ASSIGN(ArcSessionOobeOptInNegotiatorTest);
 };
 
-INSTANTIATE_TEST_CASE_P(ArcSessionOobeOptInNegotiatorTestImpl,
-                        ArcSessionOobeOptInNegotiatorTest,
-                        ::testing::Values(true, false));
+// TODO(victorhsieh): Add test to cover when ARC always start
+INSTANTIATE_TEST_CASE_P(
+    ,
+    ArcSessionOobeOptInNegotiatorTest,
+    testing::Combine(testing::Values(ArcAlwaysStartOption::OFF),
+                     testing::Bool() /* managed user */));
 
 TEST_P(ArcSessionOobeOptInNegotiatorTest, OobeTermsAccepted) {
   view()->Show();