Refactor UserCloudPolicyManagerTest to use LoggedInUserMixin.

UserCloudPolicyManagerTest uses a lot of login functionality that's
centralized within LoggedInUserMixin. Avoid code duplication by
refactoring UserCloudPolicyManagerTest to use LoggedInUserMixin for
logging in.

Bug: 990909
Change-Id: I929c7182e8fc178191b8cc722ea6bbd6987b3411
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796562Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698550}

chrome/browser/chromeos/login/screens/supervision_transition_screen_browsertest.cc

@@ -47,7 +47,7 @@ class SupervisionTransitionScreenTest
   }
 
   void SetUpOnMainThread() override {
-    ASSERT_TRUE(logged_in_user_mixin_.RequestPolicyUpdate());
+    CHECK(logged_in_user_mixin_.GetUserPolicyMixin()->RequestPolicyUpdate());
 
     arc::ArcServiceLauncher::Get()->ResetForTesting();
     arc::ArcSessionManager::Get()->SetArcSessionRunnerForTesting(
@@ -108,7 +108,7 @@ IN_PROC_BROWSER_TEST_P(SupervisionTransitionScreenTest, SuccessfulTransition) {
   EXPECT_FALSE(ProfileManager::GetPrimaryUserProfile()->GetPrefs()->GetBoolean(
       arc::prefs::kArcDataRemoveRequested));
 
-  logged_in_user_mixin().WaitForActiveSession();
+  logged_in_user_mixin().GetLoginManagerMixin()->WaitForActiveSession();
 }
 
 IN_PROC_BROWSER_TEST_P(SupervisionTransitionScreenTest, PRE_TransitionTimeout) {
@@ -151,7 +151,7 @@ IN_PROC_BROWSER_TEST_P(SupervisionTransitionScreenTest, TransitionTimeout) {
 
   test::OobeJS().TapOnPath({"supervision-transition-md", "accept-button"});
 
-  logged_in_user_mixin().WaitForActiveSession();
+  logged_in_user_mixin().GetLoginManagerMixin()->WaitForActiveSession();
 }
 
 IN_PROC_BROWSER_TEST_P(SupervisionTransitionScreenTest,
@@ -161,7 +161,7 @@ IN_PROC_BROWSER_TEST_P(SupervisionTransitionScreenTest,
 IN_PROC_BROWSER_TEST_P(SupervisionTransitionScreenTest,
                        SkipTransitionIfArcNeverStarted) {
   // Login should go through without being interrupted.
-  logged_in_user_mixin().WaitForActiveSession();
+  logged_in_user_mixin().GetLoginManagerMixin()->WaitForActiveSession();
 }
 
 INSTANTIATE_TEST_SUITE_P(/* no prefix */,

chrome/browser/supervised_user/logged_in_user_mixin.cc

@@ -4,17 +4,17 @@
 
 #include "chrome/browser/supervised_user/logged_in_user_mixin.h"
 
+#include <vector>
+
 #include "chromeos/login/auth/stub_authenticator_builder.h"
 #include "chromeos/login/auth/user_context.h"
+#include "components/account_id/account_id.h"
 #include "net/dns/mock_host_resolver.h"
 
 namespace chromeos {
 
 namespace {
 
-constexpr char kTestUserName[] = "user@gmail.com";
-constexpr char kTestUserGaiaId[] = "user";
-
 user_manager::UserType ConvertUserType(LoggedInUserMixin::LogInType type) {
   switch (type) {
     case LoggedInUserMixin::LogInType::kChild:
@@ -24,16 +24,28 @@ user_manager::UserType ConvertUserType(LoggedInUserMixin::LogInType type) {
   }
 }
 
+std::vector<LoginManagerMixin::TestUserInfo> GetInitialUsers(
+    const LoginManagerMixin::TestUserInfo& user,
+    bool include_initial_user) {
+  if (include_initial_user)
+    return {user};
+  return {};
+}
+
 }  // namespace
 
 LoggedInUserMixin::LoggedInUserMixin(
     InProcessBrowserTestMixinHost* mixin_host,
     LogInType type,
     net::EmbeddedTestServer* embedded_test_server,
-    bool should_launch_browser)
-    : user_(AccountId::FromUserEmailGaiaId(kTestUserName, kTestUserGaiaId),
+    bool should_launch_browser,
+    base::Optional<AccountId> account_id,
+    bool include_initial_user)
+    : user_(account_id.value_or(
+                AccountId::FromUserEmailGaiaId(FakeGaiaMixin::kFakeUserEmail,
+                                               FakeGaiaMixin::kFakeUserGaiaId)),
             ConvertUserType(type)),
-      login_manager_(mixin_host, {user_}),
+      login_manager_(mixin_host, GetInitialUsers(user_, include_initial_user)),
       policy_server_(mixin_host),
       user_policy_(mixin_host, user_.account_id, &policy_server_),
       embedded_test_server_setup_(mixin_host, embedded_test_server),
@@ -67,6 +79,7 @@ void LoggedInUserMixin::SetUpOnMainThreadHelper(
 void LoggedInUserMixin::LogInUser(bool issue_any_scope_token,
                                   bool wait_for_active_session) {
   UserContext user_context = LoginManagerMixin::CreateDefaultUserContext(user_);
+  user_context.SetRefreshToken(FakeGaiaMixin::kFakeRefreshToken);
   if (user_.user_type == user_manager::USER_TYPE_CHILD) {
     fake_gaia_.SetupFakeGaiaForChildUser(
         user_.account_id.GetUserEmail(), user_.account_id.GetGaiaId(),
@@ -76,7 +89,6 @@ void LoggedInUserMixin::LogInUser(bool issue_any_scope_token,
                                      user_.account_id.GetGaiaId(),
                                      FakeGaiaMixin::kFakeRefreshToken);
   }
-  user_context.SetRefreshToken(FakeGaiaMixin::kFakeRefreshToken);
   if (wait_for_active_session) {
     login_manager_.LoginAndWaitForActiveSession(user_context);
   } else {

chrome/browser/supervised_user/logged_in_user_mixin.h

@@ -6,26 +6,46 @@
 #define CHROME_BROWSER_SUPERVISED_USER_LOGGED_IN_USER_MIXIN_H_
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "chrome/browser/chromeos/login/test/embedded_test_server_mixin.h"
 #include "chrome/browser/chromeos/login/test/fake_gaia_mixin.h"
 #include "chrome/browser/chromeos/login/test/local_policy_test_server_mixin.h"
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
 #include "chrome/browser/chromeos/login/test/user_policy_mixin.h"
 
+class AccountId;
+
 namespace chromeos {
 
-// Compound mixin class for child user browser tests.
-// Supports logging in as regular or child accounts.
-// Initiates other mixins required to log in users, sets up their user policies
-// and gaia auth.
+// Compound mixin class for easily logging in as regular or child accounts for
+// browser tests. Initiates other mixins required to log in users, sets up their
+// user policies and gaia auth.
 class LoggedInUserMixin {
  public:
   enum class LogInType { kRegular, kChild };
 
+  // |mixin_host| coordinates the other mixins and usually gets passed in from a
+  // descendant of MixinBasedInProcessBrowserTest.
+  // |type| specifies the desired user log in type, currently either regular or
+  // child.
+  // |embedded_test_server| usually gets passed in from a descendant of
+  // BrowserTestBase.
+  // |should_launch_browser| determines whether a browser instance is launched
+  // after successful login. Call SelectFirstBrowser() afterwards to ensure
+  // calls to browser() don't return nullptr. SetUpOnMainThreadHelper() already
+  // packages the calls to LoginUser() and SelectFirstBrowser() together for
+  // convenience.
+  // |account_id| is the desired test account id for logging in. The default
+  // test account already works for the majority of test cases, unless an
+  // enterprise account is needed for setting up policy.
+  // |include_initial_user| determines whether the TestUserInfo should be passed
+  // to the initial users list of the LoginManagerMixin.
   LoggedInUserMixin(InProcessBrowserTestMixinHost* mixin_host,
                     LogInType type,
                     net::EmbeddedTestServer* embedded_test_server,
-                    bool should_launch_browser = true);
+                    bool should_launch_browser = true,
+                    base::Optional<AccountId> account_id = base::nullopt,
+                    bool include_initial_user = true);
   ~LoggedInUserMixin();
 
   // Helper function for refactoring common setup code.
@@ -52,16 +72,16 @@ class LoggedInUserMixin {
   void LogInUser(bool issue_any_scope_token = false,
                  bool wait_for_active_session = true);
 
-  // Waits for the session state to change to ACTIVE. Returns immediately if the
-  // session is already active.
-  void WaitForActiveSession() { login_manager_.WaitForActiveSession(); }
+  LoginManagerMixin* GetLoginManagerMixin() { return &login_manager_; }
 
-  // Creates scoped user policy update from UserPolicyMixin.
-  // See UserPolicyMixin::RequestPolicyUpdate() for more info.
-  std::unique_ptr<ScopedUserPolicyUpdate> RequestPolicyUpdate() {
-    return user_policy_.RequestPolicyUpdate();
+  LocalPolicyTestServerMixin* GetLocalPolicyTestServerMixin() {
+    return &policy_server_;
   }
 
+  UserPolicyMixin* GetUserPolicyMixin() { return &user_policy_; }
+
+  const AccountId& GetAccountId() { return user_.account_id; }
+
  private:
   LoginManagerMixin::TestUserInfo user_;
   LoginManagerMixin login_manager_;