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 98d8e1479b2739542722559283209265f5671b64..5e6fd23cb71bf63adbc9f70ea7da35cf0abed97e 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
+++ b/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
@@ -34,6 +34,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"
@@ -202,12 +203,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 (ShouldAlwaysStartArcInTest())
+      SetAlwaysStartArcForTesting();
 
     const AccountId account_id(AccountId::FromUserEmailGaiaId(
         profile()->GetProfileUserName(), "1234567890"));
@@ -215,11 +218,32 @@ class ArcSessionManagerTest : public ArcSessionManagerTestBase {
     GetFakeUserManager()->LoginUser(account_id);
   }
 
+ protected:
+  virtual bool ShouldAlwaysStartArcInTest() = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AbstractArcSessionManagerTest);
+};
+
+class ArcSessionManagerTest : public AbstractArcSessionManagerTest,
+                              public ::testing::WithParamInterface<bool> {
+ public:
+  ArcSessionManagerTest() = default;
+
+ protected:
+  bool ShouldAlwaysStartArcInTest() override { return GetParam(); }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ArcSessionManagerTest);
 };
 
-TEST_F(ArcSessionManagerTest, PrefChangeTriggersService) {
+INSTANTIATE_TEST_CASE_P(, ArcSessionManagerTest, ::testing::Bool());
+
+TEST_P(ArcSessionManagerTest, PrefChangeTriggersService) {
+  // TODO(victorhsieh): Implement opt-in and opt-out flow.
+  if (ShouldAlwaysStartArc())
+    return;
+
   ASSERT_EQ(ArcSessionManager::State::NOT_INITIALIZED,
             arc_session_manager()->state());
 
@@ -243,7 +267,7 @@ TEST_F(ArcSessionManagerTest, PrefChangeTriggersService) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, DisabledForEphemeralDataUsers) {
+TEST_P(ArcSessionManagerTest, DisabledForEphemeralDataUsers) {
   PrefService* const prefs = profile()->GetPrefs();
   EXPECT_FALSE(prefs->GetBoolean(prefs::kArcSignedIn));
   prefs->SetBoolean(prefs::kArcEnabled, true);
@@ -288,7 +312,11 @@ TEST_F(ArcSessionManagerTest, DisabledForEphemeralDataUsers) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, BaseWorkflow) {
+TEST_P(ArcSessionManagerTest, BaseWorkflow) {
+  // See BaseWorkflowOnPersistentArc for Persistent ARC flow.
+  if (ShouldAlwaysStartArc())
+    return;
+
   ASSERT_TRUE(arc_session_manager()->IsSessionStopped());
   ASSERT_EQ(ArcSessionManager::State::NOT_INITIALIZED,
             arc_session_manager()->state());
@@ -340,7 +368,11 @@ TEST_F(ArcSessionManagerTest, BaseWorkflow) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, CancelFetchingDisablesArc) {
+TEST_P(ArcSessionManagerTest, CancelFetchingDisablesArc) {
+  // TODO(victorhsieh): Implement opt-in flow on Persistent ARC.
+  if (ShouldAlwaysStartArc())
+    return;
+
   PrefService* const pref = profile()->GetPrefs();
 
   arc_session_manager()->OnPrimaryUserProfilePrepared(profile());
@@ -361,7 +393,7 @@ TEST_F(ArcSessionManagerTest, CancelFetchingDisablesArc) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, CloseUIKeepsArcEnabled) {
+TEST_P(ArcSessionManagerTest, CloseUIKeepsArcEnabled) {
   PrefService* const pref = profile()->GetPrefs();
 
   arc_session_manager()->OnPrimaryUserProfilePrepared(profile());
@@ -380,7 +412,7 @@ TEST_F(ArcSessionManagerTest, CloseUIKeepsArcEnabled) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, EnableDisablesArc) {
+TEST_P(ArcSessionManagerTest, EnableDisablesArc) {
   const PrefService* pref = profile()->GetPrefs();
   arc_session_manager()->OnPrimaryUserProfilePrepared(profile());
 
@@ -394,7 +426,7 @@ TEST_F(ArcSessionManagerTest, EnableDisablesArc) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, SignInStatus) {
+TEST_P(ArcSessionManagerTest, SignInStatus) {
   PrefService* const prefs = profile()->GetPrefs();
 
   EXPECT_TRUE(arc_session_manager()->sign_in_start_time().is_null());
@@ -404,12 +436,18 @@ TEST_F(ArcSessionManagerTest, SignInStatus) {
   prefs->SetBoolean(prefs::kArcEnabled, true);
 
   arc_session_manager()->OnPrimaryUserProfilePrepared(profile());
-  EXPECT_EQ(ArcSessionManager::State::SHOWING_TERMS_OF_SERVICE,
-            arc_session_manager()->state());
+  if (ShouldAlwaysStartArc()) {
+    EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
+  } else {
+    EXPECT_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()->StartArc();
+  if (!ShouldAlwaysStartArc()) {
+    arc_session_manager()->StartArc();
+  }
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
   EXPECT_TRUE(arc_session_manager()->IsSessionRunning());
   EXPECT_FALSE(prefs->GetBoolean(prefs::kArcSignedIn));
@@ -443,7 +481,7 @@ TEST_F(ArcSessionManagerTest, SignInStatus) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, DisabledForDeviceLocalAccount) {
+TEST_P(ArcSessionManagerTest, DisabledForDeviceLocalAccount) {
   PrefService* const prefs = profile()->GetPrefs();
   EXPECT_FALSE(prefs->GetBoolean(prefs::kArcSignedIn));
   prefs->SetBoolean(prefs::kArcEnabled, true);
@@ -475,7 +513,7 @@ TEST_F(ArcSessionManagerTest, DisabledForDeviceLocalAccount) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, DisabledForNonPrimaryProfile) {
+TEST_P(ArcSessionManagerTest, DisabledForNonPrimaryProfile) {
   profile()->GetPrefs()->SetBoolean(prefs::kArcEnabled, true);
   arc_session_manager()->OnPrimaryUserProfilePrepared(profile());
   arc_session_manager()->StartArc();
@@ -498,7 +536,11 @@ TEST_F(ArcSessionManagerTest, DisabledForNonPrimaryProfile) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, RemoveDataFolder) {
+TEST_P(ArcSessionManagerTest, RemoveDataFolder) {
+  // TODO(victorhsieh): Implement data removal on Persistent ARC.
+  if (ShouldAlwaysStartArc())
+    return;
+
   profile()->GetPrefs()->SetBoolean(prefs::kArcEnabled, false);
   // Starting session manager with prefs::kArcEnabled off automatically removes
   // Android's data folder.
@@ -551,7 +593,7 @@ TEST_F(ArcSessionManagerTest, RemoveDataFolder) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
+TEST_P(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
   profile()->GetPrefs()->SetBoolean(prefs::kArcEnabled, true);
   arc_session_manager()->OnPrimaryUserProfilePrepared(profile());
   arc_session_manager()->StartArc();
@@ -572,8 +614,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, bool>> {
  public:
   const base::Value& backup_restore_pref_value() const {
     return std::get<0>(GetParam());
@@ -582,9 +625,16 @@ class ArcSessionManagerPolicyTest
   const base::Value& location_service_pref_value() const {
     return std::get<1>(GetParam());
   }
+
+ protected:
+  bool ShouldAlwaysStartArcInTest() override { return std::get<2>(GetParam()); }
 };
 
 TEST_P(ArcSessionManagerPolicyTest, SkippingTerms) {
+  // TODO(victorhsieh): Implement opt-in flow.
+  if (ShouldAlwaysStartArc())
+    return;
+
   sync_preferences::TestingPrefServiceSyncable* const prefs =
       profile()->GetTestingPrefService();
 
@@ -634,11 +684,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::Bool()));
 
 class ArcSessionManagerKioskTest : public ArcSessionManagerTestBase {
  public:
@@ -674,11 +725,18 @@ TEST_F(ArcSessionManagerKioskTest, AuthFailure) {
   EXPECT_TRUE(terminated);
 }
 
-class ArcSessionOobeOptInTest : public ArcSessionManagerTest {
+// This class takes two test parameters because the 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<bool, bool>> {
  public:
   ArcSessionOobeOptInTest() = default;
 
  protected:
+  bool ShouldAlwaysStartArcInTest() override { return std::get<0>(GetParam()); }
+
   void CreateLoginDisplayHost() {
     fake_login_display_host_ = base::MakeUnique<FakeLoginDisplayHost>();
   }
@@ -696,7 +754,13 @@ class ArcSessionOobeOptInTest : public ArcSessionManagerTest {
   DISALLOW_COPY_AND_ASSIGN(ArcSessionOobeOptInTest);
 };
 
-TEST_F(ArcSessionOobeOptInTest, OobeOptInActive) {
+INSTANTIATE_TEST_CASE_P(
+    ,
+    ArcSessionOobeOptInTest,
+    testing::Combine(::testing::Bool() /* always start arc */,
+                     ::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());
@@ -717,8 +781,7 @@ TEST_F(ArcSessionOobeOptInTest, OobeOptInActive) {
 
 class ArcSessionOobeOptInNegotiatorTest
     : public ArcSessionOobeOptInTest,
-      public chromeos::ArcTermsOfServiceScreenActor,
-      public testing::WithParamInterface<bool> {
+      public chromeos::ArcTermsOfServiceScreenActor {
  public:
   ArcSessionOobeOptInNegotiatorTest() = default;
 
@@ -758,7 +821,8 @@ class ArcSessionOobeOptInNegotiatorTest
   }
 
  protected:
-  bool IsManagedUser() { return GetParam(); }
+  bool ShouldAlwaysStartArcInTest() override { return std::get<0>(GetParam()); }
+  bool IsManagedUser() { return std::get<1>(GetParam()); }
 
   void ReportResult(bool accepted) {
     for (auto& observer : observer_list_) {
@@ -816,9 +880,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(false) /* always start arc */,
+                     ::testing::Bool() /* managed user */));
 
 TEST_P(ArcSessionOobeOptInNegotiatorTest, OobeTermsAccepted) {
   actor()->Show();