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 130662b020410af63b397da7dc52649df7ab1a7e..a93e938b8ba693ec7a2f52f1b5f2f896a5101701 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,38 @@ 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, PrefChangeTriggersService) {
+INSTANTIATE_TEST_CASE_P(,
+                        ArcSessionManagerTest,
+                        testing::Values(ArcAlwaysStartOption::OFF,
+                                        ArcAlwaysStartOption::ON));
+
+TEST_P(ArcSessionManagerTest, PrefChangeTriggersService) {
+  // TODO(victorhsieh): Implement opt-in and opt-out flow.
+  if (ShouldArcAlwaysStart())
+    return;
+
   ASSERT_FALSE(IsArcPlayStoreEnabledForProfile(profile()));
   arc_session_manager()->SetProfile(profile());
   arc_session_manager()->StartPreferenceHandler();
@@ -246,7 +281,11 @@ TEST_F(ArcSessionManagerTest, PrefChangeTriggersService) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, PrefChangeTriggersService_Restart) {
+TEST_P(ArcSessionManagerTest, PrefChangeTriggersService_Restart) {
+  // TODO(victorhsieh): Implement opt-in and opt-out flow.
+  if (ShouldArcAlwaysStart())
+    return;
+
   // Sets the Google Play Store preference at beginning.
   SetArcPlayStoreEnabledForProfile(profile(), true);
 
@@ -268,7 +307,10 @@ TEST_F(ArcSessionManagerTest, PrefChangeTriggersService_Restart) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, BaseWorkflow) {
+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());
 
@@ -296,7 +338,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 (ShouldArcAlwaysStart())
+    return;
+
   SetArcPlayStoreEnabledForProfile(profile(), true);
 
   // Starts ARC.
@@ -322,7 +368,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());
@@ -342,7 +392,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());
@@ -353,12 +403,18 @@ 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()->StartArc();
+  if (!ShouldArcAlwaysStart()) {
+    arc_session_manager()->StartArc();
+  }
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
   EXPECT_TRUE(arc_session_manager()->IsSessionRunning());
 
@@ -376,7 +432,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();
@@ -403,12 +459,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,
@@ -442,7 +503,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);
@@ -458,7 +523,11 @@ TEST_F(ArcSessionManagerTest, RemoveDataDir_Restart) {
   arc_session_manager()->Shutdown();
 }
 
-TEST_F(ArcSessionManagerTest, RemoveDataDir_Managed) {
+TEST_P(ArcSessionManagerTest, RemoveDataDir_Managed) {
+  // TODO(victorhsieh): Implement data removal on Persistent ARC.
+  if (ShouldArcAlwaysStart())
+    return;
+
   // Set ARC to be managed and disabled.
   profile()->GetTestingPrefService()->SetManagedPref(prefs::kArcEnabled,
                                                      new base::Value(false));
@@ -471,7 +540,7 @@ TEST_F(ArcSessionManagerTest, RemoveDataDir_Managed) {
       profile()->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested));
 }
 
-TEST_F(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
+TEST_P(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
   arc_session_manager()->SetProfile(profile());
   arc_session_manager()->RequestEnable();
   arc_session_manager()->StartArc();
@@ -492,8 +561,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());
@@ -502,9 +572,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();
 
@@ -567,11 +646,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:
@@ -607,11 +687,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>> {

[Luis Héctor Chávez, 2017/03/02 21:07:10]

Much better :D

  public:
   ArcSessionOobeOptInTest() = default;
 
  protected:
+  ArcAlwaysStartOption ShouldArcAlwaysStartInTest() override {
+    return std::get<0>(GetParam());
+  }
+
   void CreateLoginDisplayHost() {
     fake_login_display_host_ = base::MakeUnique<FakeLoginDisplayHost>();
   }
@@ -629,7 +718,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());
@@ -650,8 +746,7 @@ TEST_F(ArcSessionOobeOptInTest, OobeOptInActive) {
 
 class ArcSessionOobeOptInNegotiatorTest
     : public ArcSessionOobeOptInTest,
-      public chromeos::ArcTermsOfServiceScreenView,
-      public testing::WithParamInterface<bool> {
+      public chromeos::ArcTermsOfServiceScreenView {
  public:
   ArcSessionOobeOptInNegotiatorTest() = default;
 
@@ -692,7 +787,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_) {
@@ -738,9 +837,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();