Convert ExtensionWebstorePrivateApiTestChild to use child accounts

ExtensionWebstorePrivateApiTestChild in webstore_private_apitest.cc
has been using legacy switches to log in as supervised user for the
InstallBlocked test. This CL converts the test class to use child
accounts instead.

Bug: 990542
Change-Id: Ib704b601e23a9f815328c8c5cfbe621f7bf62ba5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752528Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690773}

chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc

@@ -42,6 +42,7 @@
 #include "ui/gl/gl_switches.h"
 
 #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
+#include "chrome/browser/supervised_user/logged_in_user_mixin.h"
 #include "chrome/browser/supervised_user/supervised_user_constants.h"
 
 #if defined(OS_CHROMEOS)
@@ -375,23 +376,97 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, EmptyCrx) {
 class ExtensionWebstorePrivateApiTestChild
     : public ExtensionWebstorePrivateApiTest {
  public:
+  ExtensionWebstorePrivateApiTestChild()
+      : embedded_test_server_(std::make_unique<net::EmbeddedTestServer>()),
+        logged_in_user_mixin_(&mixin_host_,
+                              chromeos::LoggedInUserMixin::LogInType::kChild,
+                              embedded_test_server_.get()) {
+    // Suppress regular user login to enable child user login.
+    set_chromeos_user_ = false;
+    // Launch a browser instance after logging in.
+    logged_in_user_mixin_.set_should_launch_browser(true);
+  }
+
+  void SetUp() override {
+    mixin_host_.SetUp();
+    ExtensionWebstorePrivateApiTest::SetUp();
+  }
+
   void SetUpCommandLine(base::CommandLine* command_line) override {
+    mixin_host_.SetUpCommandLine(command_line);
     ExtensionWebstorePrivateApiTest::SetUpCommandLine(command_line);
-    command_line->AppendSwitchASCII(switches::kSupervisedUserId,
-                                    supervised_users::kChildAccountSUID);
-#if defined(OS_CHROMEOS)
-    command_line->AppendSwitchASCII(
-        chromeos::switches::kLoginUser,
-        "supervised_user@locally-managed.localhost");
-    command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile, "hash");
-#endif
+    // Shortens the merge session timeout from 20 to 1 seconds to speed up the
+    // test by about 19 seconds.
+    // TODO (crbug.com/995575): figure out why this switch speeds up the test,
+    // and fix the test setup so this is not required.
+    command_line->AppendSwitch(switches::kShortMergeSessionTimeoutForTest);
+  }
+
+  void SetUpDefaultCommandLine(base::CommandLine* command_line) override {
+    mixin_host_.SetUpDefaultCommandLine(command_line);
+    ExtensionWebstorePrivateApiTest::SetUpDefaultCommandLine(command_line);
+  }
+
+  bool SetUpUserDataDirectory() override {
+    return mixin_host_.SetUpUserDataDirectory() &&
+           ExtensionWebstorePrivateApiTest::SetUpUserDataDirectory();
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {
+    mixin_host_.SetUpInProcessBrowserTestFixture();
+    ExtensionWebstorePrivateApiTest::SetUpInProcessBrowserTestFixture();
+  }
+
+  void CreatedBrowserMainParts(
+      content::BrowserMainParts* browser_main_parts) override {
+    mixin_host_.CreatedBrowserMainParts(browser_main_parts);
+    ExtensionWebstorePrivateApiTest::CreatedBrowserMainParts(
+        browser_main_parts);
+  }
+
+  void SetUpOnMainThread() override {
+    mixin_host_.SetUpOnMainThread();
+    ExtensionWebstorePrivateApiTest::SetUpOnMainThread();
+    // Needed for resolving FakeGaiaMixin token requests.
+    // Otherwise the test times out.
+    host_resolver()->AddRule("*", "127.0.0.1");
+    logged_in_user_mixin_.LogInUser(true /* issue_any_scope_token */);
+    // Set the private |browser_| member in InProcessBrowserTest.
+    // Otherwise calls to InProcessBrowserTest::browser() returns null and leads
+    // to segmentation faults.
+    SelectFirstBrowser();
+  }
+
+  void TearDownOnMainThread() override {
+    mixin_host_.TearDownOnMainThread();
+    ExtensionWebstorePrivateApiTest::TearDownOnMainThread();
   }
+
+  void TearDownInProcessBrowserTestFixture() override {
+    mixin_host_.TearDownInProcessBrowserTestFixture();
+    ExtensionWebstorePrivateApiTest::TearDownInProcessBrowserTestFixture();
+  }
+
+  void TearDown() override {
+    mixin_host_.TearDown();
+    ExtensionWebstorePrivateApiTest::TearDown();
+  }
+
+ private:
+  // Replicate what MixinBasedInProcessBrowserTest does since inheriting from
+  // that class is inconvenient here.
+  InProcessBrowserTestMixinHost mixin_host_;
+  // Create another embedded test server to avoid starting the same one twice.
+  std::unique_ptr<net::EmbeddedTestServer> embedded_test_server_;
+
+  chromeos::LoggedInUserMixin logged_in_user_mixin_;
 };
 
 // Tests that extension installation is blocked for child accounts, and
 // attempting to do so produces a special error code.
 // Note: This will have to be updated when we enable child-initiated installs.
 IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChild, InstallBlocked) {
+  ASSERT_TRUE(browser());
   ASSERT_TRUE(RunInstallTest("begin_install_fail_child.html", "extension.crx"));
 }
 

chrome/browser/supervised_user/logged_in_user_mixin.cc

@@ -38,12 +38,12 @@ LoggedInUserMixin::LoggedInUserMixin(
 
 LoggedInUserMixin::~LoggedInUserMixin() = default;
 
-void LoggedInUserMixin::LogInUser() {
+void LoggedInUserMixin::LogInUser(bool issue_any_scope_token) {
   UserContext user_context = LoginManagerMixin::CreateDefaultUserContext(user_);
   if (user_.user_type == user_manager::USER_TYPE_CHILD) {
     fake_gaia_.SetupFakeGaiaForChildUser(
         user_.account_id.GetUserEmail(), user_.account_id.GetGaiaId(),
-        FakeGaiaMixin::kFakeRefreshToken, false /*issue_any_scope_token*/);
+        FakeGaiaMixin::kFakeRefreshToken, issue_any_scope_token);
   } else {
     fake_gaia_.SetupFakeGaiaForLogin(user_.account_id.GetUserEmail(),
                                      user_.account_id.GetGaiaId(),
@@ -53,4 +53,8 @@ void LoggedInUserMixin::LogInUser() {
   login_manager_.LoginAndWaitForActiveSession(user_context);
 }
 
+void LoggedInUserMixin::set_should_launch_browser(bool value) {
+  login_manager_.set_should_launch_browser(value);
+}
+
 }  // namespace chromeos

chrome/browser/supervised_user/logged_in_user_mixin.h

@@ -27,7 +27,16 @@ class LoggedInUserMixin {
                     net::EmbeddedTestServer* embedded_test_server);
   ~LoggedInUserMixin();
 
-  void LogInUser();
+  // Log in as regular or child account depending on the |type| argument passed
+  // to the constructor.
+  // * If |issue_any_scope_token|, FakeGaiaMixin will issue a special all-access
+  // token associated with the test refresh token. Only matters for child login.
+  void LogInUser(bool issue_any_scope_token = false);
+
+  // By default, LoginManagerMixin will set up user session manager not to
+  // launch browser as part of user session setup - use this to override that
+  // behavior.
+  void set_should_launch_browser(bool value);
 
  private:
   LoginManagerMixin::TestUserInfo user_;

chrome/test/base/in_process_browser_test.cc

@@ -450,6 +450,12 @@ Browser* InProcessBrowserTest::CreateBrowserForApp(const std::string& app_name,
 }
 #endif  // !defined(OS_MACOSX)
 
+void InProcessBrowserTest::SelectFirstBrowser() {
+  const BrowserList* browser_list = BrowserList::GetInstance();
+  if (!browser_list->empty())
+    browser_ = browser_list->get(0);
+}
+
 void InProcessBrowserTest::AddBlankTabAndShow(Browser* browser) {
   content::WindowedNotificationObserver observer(
       content::NOTIFICATION_LOAD_STOP,
@@ -498,9 +504,8 @@ void InProcessBrowserTest::PreRunTestOnMainThread() {
   // Pump startup related events.
   content::RunAllPendingInMessageLoop();
 
-  const BrowserList* active_browser_list = BrowserList::GetInstance();
-  if (!active_browser_list->empty()) {
-    browser_ = active_browser_list->get(0);
+  SelectFirstBrowser();
+  if (browser_) {
 #if defined(OS_CHROMEOS)
     // There are cases where windows get created maximized by default.
     if (browser_->window()->IsMaximized())

chrome/test/base/in_process_browser_test.h

@@ -147,6 +147,9 @@ class InProcessBrowserTest : public content::BrowserTestBase {
   }
 
   // Returns the browser created by BrowserMain().
+  // If no browser is created in BrowserMain(), this will return nullptr unless
+  // another browser instance is created at a later time and
+  // SelectFirstBrowser() is called.
   Browser* browser() const { return browser_; }
 
  protected:
@@ -223,6 +226,12 @@ class InProcessBrowserTest : public content::BrowserTestBase {
   // the browser.
   Browser* CreateBrowserForApp(const std::string& app_name, Profile* profile);
 
+  // Set |browser_| to the first browser on the browser list.
+  // Call this when your test subclass wants to access a non-null browser
+  // instance through browser() but browser creation is delayed until after
+  // PreRunTestOnMainThread().
+  void SelectFirstBrowser();
+
   // Called from the various CreateBrowser methods to add a blank tab, wait for
   // the navigation to complete, and show the browser's window.
   void AddBlankTabAndShow(Browser* browser);
@@ -263,7 +272,10 @@ class InProcessBrowserTest : public content::BrowserTestBase {
 
   static SetUpBrowserFunction* global_browser_set_up_function_;
 
-  // Browser created in BrowserMain().
+  // Usually references the browser created in BrowserMain().
+  // If no browser is created in BrowserMain(), then |browser_| will remain
+  // nullptr unless SelectFirstBrowser() is called after the creation of the
+  // first browser instance at a later time.
   Browser* browser_ = nullptr;
 
   // Used to run the process until the BrowserProcess signals the test to quit.