Make Lacros lifetime closer to real use.

We started to enable Lacros's KeepAlive for AppService.
Following that, now AshBrowserTestStarter uses it to make it
closer to the real use cases.

To avoid name conflict, put AshBrowserTestStarter to crosapi::test
namespace.

BUG=1277898
TEST=Ran browser_tests.

Change-Id: I007e2cf5dc75cf581b3fdca595c18b2a9fbc8c5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3386527
Reviewed-by: Sven Zheng <svenzheng@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#960880}

chrome/browser/ash/crosapi/browser_manager.h

@@ -44,6 +44,10 @@ namespace mojom {
 class Crosapi;
 }  // namespace mojom
 
+namespace test {
+class AshBrowserTestStarter;
+}  // namespace test
+
 class BrowserLoader;
 class TestMojoConnectionManager;
 
@@ -273,6 +277,10 @@ class BrowserManager : public session_manager::SessionManagerObserver,
   // web apps in Lacros. Need to decouple the App Platform systems from
   // needing lacros-chrome running all the time.
   friend class apps::AppServiceProxyAsh;
+  // Currently in practice KeepAlive is enabled for AppService as described
+  // above. To make the testing environment closer to the case, set up
+  // KeepAlive for AshBrowserTestStarter, too.
+  friend class test::AshBrowserTestStarter;
 
   // Returns true if the binary is ready to launch or already launched.
   bool IsReady() const;

chrome/test/base/chromeos/ash_browser_test_starter.cc

@@ -20,6 +20,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace crosapi {
 namespace test {
 
 AshBrowserTestStarter::AshBrowserTestStarter() = default;
@@ -45,21 +46,21 @@ bool AshBrowserTestStarter::PrepareEnvironmentForLacros() {
   return true;
 }
 
-class LacrosStartedObserver : public crosapi::BrowserManagerObserver {
+class LacrosStartedObserver : public BrowserManagerObserver {
  public:
   LacrosStartedObserver() = default;
   LacrosStartedObserver(const LacrosStartedObserver&) = delete;
   LacrosStartedObserver& operator=(const LacrosStartedObserver&) = delete;
   ~LacrosStartedObserver() override = default;
 
   void OnStateChanged() override {
-    if (crosapi::BrowserManager::Get()->IsRunning()) {
+    if (BrowserManager::Get()->IsRunning()) {
       run_loop_.Quit();
     }
   }
 
   void Wait(base::TimeDelta timeout) {
-    if (crosapi::BrowserManager::Get()->IsRunning()) {
+    if (BrowserManager::Get()->IsRunning()) {
       return;
     }
     base::ThreadPool::PostDelayedTask(FROM_HERE, run_loop_.QuitClosure(),
@@ -91,16 +92,16 @@ void AshBrowserTestStarter::StartLacros(InProcessBrowserTest* test_class_obj) {
   DCHECK(HasLacrosArgument());
 
   WaitForExoStarted(scoped_temp_dir_xdg_.GetPath());
-
-  crosapi::BrowserManager::Get()->NewWindow(
-      /*incongnito=*/false, /*should_trigger_session_restore=*/false);
+  auto* browser_manager = BrowserManager::Get();
+  lacros_keep_alive_ =
+      browser_manager->KeepAlive(BrowserManager::Feature::kTestOnly);
 
   LacrosStartedObserver observer;
-  crosapi::BrowserManager::Get()->AddObserver(&observer);
+  browser_manager->AddObserver(&observer);
   observer.Wait(TestTimeouts::action_max_timeout());
-  crosapi::BrowserManager::Get()->RemoveObserver(&observer);
+  browser_manager->RemoveObserver(&observer);
 
-  CHECK(crosapi::BrowserManager::Get()->IsRunning());
+  CHECK(browser_manager->IsRunning());
 
   // Create a new ash browser window so browser() can work.
   Profile* profile = ProfileManager::GetActiveUserProfile();
@@ -109,3 +110,4 @@ void AshBrowserTestStarter::StartLacros(InProcessBrowserTest* test_class_obj) {
 }
 
 }  // namespace test
+}  // namespace crosapi

chrome/test/base/chromeos/ash_browser_test_starter.h

@@ -7,9 +7,11 @@
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/ash/crosapi/browser_manager.h"
 
 class InProcessBrowserTest;
 
+namespace crosapi {
 namespace test {
 
 class AshBrowserTestStarter {
@@ -37,8 +39,10 @@ class AshBrowserTestStarter {
   // This is XDG_RUNTIME_DIR.
   base::ScopedTempDir scoped_temp_dir_xdg_;
   base::test::ScopedFeatureList scoped_feature_list_;
+  std::unique_ptr<BrowserManager::ScopedKeepAlive> lacros_keep_alive_;
 };
 
 }  // namespace test
+}  // namespace crosapi
 
 #endif  // CHROME_TEST_BASE_CHROMEOS_ASH_BROWSER_TEST_STARTER_H_

chrome/test/base/chromeos/demo_ash_requires_lacros_browsertest.cc

@@ -34,7 +34,7 @@ class DemoAshRequiresLacrosTest : public InProcessBrowserTest {
   }
 
  protected:
-  test::AshBrowserTestStarter ash_starter_;
+  crosapi::test::AshBrowserTestStarter ash_starter_;
 };
 
 IN_PROC_BROWSER_TEST_F(DemoAshRequiresLacrosTest, NewTab) {

chrome/test/base/javascript_browser_test.cc

@@ -18,18 +18,13 @@ void JavaScriptBrowserTest::AddLibrary(const base::FilePath& library_path) {
   user_libraries_.push_back(library_path);
 }
 
-JavaScriptBrowserTest::JavaScriptBrowserTest() {
-#if BUILDFLAG(IS_CHROMEOS_ASH)
+JavaScriptBrowserTest::JavaScriptBrowserTest() = default;
 
-#endif
-}
-
-JavaScriptBrowserTest::~JavaScriptBrowserTest() {
-}
+JavaScriptBrowserTest::~JavaScriptBrowserTest() = default;
 
 void JavaScriptBrowserTest::SetUpInProcessBrowserTestFixture() {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
-  ash_starter_ = std::make_unique<test::AshBrowserTestStarter>();
+  ash_starter_ = std::make_unique<crosapi::test::AshBrowserTestStarter>();
   if (ash_starter_->HasLacrosArgument())
     ASSERT_TRUE(ash_starter_->PrepareEnvironmentForLacros());
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)

chrome/test/base/javascript_browser_test.h

@@ -49,7 +49,9 @@ class JavaScriptBrowserTest : public InProcessBrowserTest {
                                     std::vector<base::Value> args);
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
-  test::AshBrowserTestStarter* ash_starter() { return ash_starter_.get(); }
+  crosapi::test::AshBrowserTestStarter* ash_starter() {
+    return ash_starter_.get();
+  }
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
  private:
@@ -60,7 +62,7 @@ class JavaScriptBrowserTest : public InProcessBrowserTest {
   std::vector<base::FilePath> library_search_paths_;
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
-  std::unique_ptr<test::AshBrowserTestStarter> ash_starter_;
+  std::unique_ptr<crosapi::test::AshBrowserTestStarter> ash_starter_;
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 };