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..f4cfc754dbff06336599ecb9ac2540dde34c7d63 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,13 @@ 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();
+    ArcAppTest::Init(IsPersistentArcEnabled());
 
     const AccountId account_id(AccountId::FromUserEmailGaiaId(
         profile()->GetProfileUserName(), "1234567890"));
@@ -215,11 +217,34 @@ class ArcSessionManagerTest : public ArcSessionManagerTestBase {
     GetFakeUserManager()->LoginUser(account_id);
   }
 
+ protected:
+  virtual bool IsPersistentArcEnabled() = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AbstractArcSessionManagerTest);
+};
+
+class ArcSessionManagerTest : public AbstractArcSessionManagerTest,
+                              public ::testing::WithParamInterface<bool> {
+ public:
+  ArcSessionManagerTest() = default;
+
+ protected:
+  bool IsPersistentArcEnabled() override { return GetParam(); }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ArcSessionManagerTest);
 };
 
-TEST_F(ArcSessionManagerTest, PrefChangeTriggersService) {
+INSTANTIATE_TEST_CASE_P(,

[hidehiko, 2017/02/23 10:29:16]

Any name?

[victorhsieh, 2017/02/24 00:44:59]

I prefer not to.  It will change the test name from FooTest to FooTest/FooTest
without benefit in this case.  Many existing code does the same thing. 
Actually, perhaps let me remove the name from the other case in this file.

+                        ArcSessionManagerTest,
+                        ::testing::Values(false, true));

[hidehiko, 2017/02/23 10:29:16]

::testing::Bool() can be used?

[victorhsieh, 2017/02/24 00:44:59]

Done.

+
+TEST_P(ArcSessionManagerTest, PrefChangeTriggersService) {
+  // TODO(victorhsieh): Implement opt-in and opt-out flow.
+  if (arc_session_manager()->IsPersistentArc())
+    return;
+
   ASSERT_EQ(ArcSessionManager::State::NOT_INITIALIZED,
             arc_session_manager()->state());
 
@@ -243,7 +268,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 +313,11 @@ TEST_F(ArcSessionManagerTest, DisabledForEphemeralDataUsers) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, BaseWorkflow) {
+TEST_P(ArcSessionManagerTest, BaseWorkflow) {
+  // See BaseWorkflowOnPersistentArc for Persistent ARC flow.
+  if (arc_session_manager()->IsPersistentArc())
+    return;
+
   ASSERT_TRUE(arc_session_manager()->IsSessionStopped());
   ASSERT_EQ(ArcSessionManager::State::NOT_INITIALIZED,
             arc_session_manager()->state());
@@ -340,7 +369,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 (arc_session_manager()->IsPersistentArc())
+    return;
+
   PrefService* const pref = profile()->GetPrefs();
 
   arc_session_manager()->OnPrimaryUserProfilePrepared(profile());
@@ -361,7 +394,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 +413,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 +427,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 +437,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 (arc_session_manager()->IsPersistentArc()) {
+    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 (!arc_session_manager()->IsPersistentArc()) {
+    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 +482,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 +514,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 +537,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 (arc_session_manager()->IsPersistentArc())
+    return;
+
   profile()->GetPrefs()->SetBoolean(prefs::kArcEnabled, false);
   // Starting session manager with prefs::kArcEnabled off automatically removes
   // Android's data folder.
@@ -551,7 +594,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 +615,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,6 +626,9 @@ class ArcSessionManagerPolicyTest
   const base::Value& location_service_pref_value() const {
     return std::get<1>(GetParam());
   }
+
+ protected:
+  bool IsPersistentArcEnabled() override { return std::get<2>(GetParam()); }
 };
 
 TEST_P(ArcSessionManagerPolicyTest, SkippingTerms) {
@@ -638,7 +685,8 @@ INSTANTIATE_TEST_CASE_P(
     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(true, false)));
 
 class ArcSessionManagerKioskTest : public ArcSessionManagerTestBase {
  public:
@@ -674,11 +722,13 @@ TEST_F(ArcSessionManagerKioskTest, AuthFailure) {
   EXPECT_TRUE(terminated);
 }
 
-class ArcSessionOobeOptInTest : public ArcSessionManagerTest {
+class ArcSessionOobeOptInTest : public AbstractArcSessionManagerTest {
  public:
   ArcSessionOobeOptInTest() = default;
 
  protected:
+  bool IsPersistentArcEnabled() override { return false; }

[hidehiko, 2017/02/23 10:29:16]

This also should be tested with and without PARC?

[victorhsieh, 2017/02/24 00:44:59]

Done.

+
   void CreateLoginDisplayHost() {
     fake_login_display_host_ = base::MakeUnique<FakeLoginDisplayHost>();
   }
@@ -718,7 +768,7 @@ TEST_F(ArcSessionOobeOptInTest, OobeOptInActive) {
 class ArcSessionOobeOptInNegotiatorTest
     : public ArcSessionOobeOptInTest,
       public chromeos::ArcTermsOfServiceScreenActor,
-      public testing::WithParamInterface<bool> {
+      public testing::WithParamInterface<std::tuple<bool, bool>> {
  public:
   ArcSessionOobeOptInNegotiatorTest() = default;
 
@@ -758,7 +808,9 @@ class ArcSessionOobeOptInNegotiatorTest
   }
 
  protected:
-  bool IsManagedUser() { return GetParam(); }
+  bool IsManagedUser() { return std::get<0>(GetParam()); }
+
+  bool IsPersistentArcEnabled() override { return std::get<1>(GetParam()); }
 
   void ReportResult(bool accepted) {
     for (auto& observer : observer_list_) {
@@ -818,7 +870,8 @@ class ArcSessionOobeOptInNegotiatorTest
 
 INSTANTIATE_TEST_CASE_P(ArcSessionOobeOptInNegotiatorTestImpl,
                         ArcSessionOobeOptInNegotiatorTest,
-                        ::testing::Values(true, false));
+                        testing::Combine(::testing::Values(true, false),
+                                         ::testing::Values(true, false)));
 
 TEST_P(ArcSessionOobeOptInNegotiatorTest, OobeTermsAccepted) {
   actor()->Show();