Support unique ash chrome in tests

Currently for lacros browser tests, shared ash chrome was started and
used by tests. But there are user cases that a feature needs to be
enabled in ash, then we can test some lacros behavior. This change
makes writing such test cases possible.

The ideal usage would be:
1 Developer developing an ash feature behind a flag.
2 Write a browser test using StartUniqueAshChrome with the feature on.
3 After the feature is on by default, remove the feature flag and
  switch to the shared ash chrome.

Bug: 1368284
Change-Id: I472458ca7f894822965fbb4bc2013417f4dcf55f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4292072
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Jenny Zhang <jennyz@chromium.org>
Commit-Queue: Sven Zheng <svenzheng@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1111887}

build/lacros/test_runner.py

@@ -608,6 +608,7 @@ def _RunTestWithAshChrome(args, forward_args):
     if enable_mojo_crosapi:
       forward_args.append(lacros_mojo_socket_arg)
 
+    forward_args.append('--ash-chrome-path=' + ash_chrome_file)
     test_env = os.environ.copy()
     test_env['WAYLAND_DISPLAY'] = ash_wayland_socket_name
     test_env['EGL_PLATFORM'] = 'surfaceless'

build/lacros/test_runner_test.py

@@ -125,6 +125,7 @@ def test_require_ash_chrome(self, command, mock_popen, mock_download, *_):
             command,
             '--test-launcher-filter-file=/a/b/filter',
             '--lacros-mojo-socket-for-testing=/tmp/ash-data/lacros.sock',
+            '--ash-chrome-path=' + ash_chrome_args[0],
         ], test_args)
       else:
         self.assertListEqual(test_args[:len(command_parts)], command_parts)

chrome/test/BUILD.gn

@@ -4854,6 +4854,7 @@ if (is_chromeos_lacros) {
       "../browser/sync/test/lacros/sync_apps_toggle_sharing_lacros_browsertest.cc",
       "../browser/sync/test/lacros/sync_custom_passphrase_sharing_lacros_browsertest.cc",
       "../browser/upgrade_detector/get_installed_version_lacros_browsertest.cc",
+      "base/in_process_browser_test_browsertest_lacros.cc",
 
       # ScreenCapturer should work for lacros with crosapi.
       "base/save_desktop_snapshot_browsertest.cc",

chrome/test/base/in_process_browser_test.cc

@@ -139,10 +139,16 @@
 #endif
 
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
+#include "base/base_switches.h"
+#include "base/environment.h"
+#include "base/files/file_path_watcher.h"
+#include "base/guid.h"
+#include "base/process/launch.h"
 #include "chrome/browser/lacros/cert/cert_db_initializer_factory.h"
 #include "components/account_manager_core/chromeos/account_manager.h"
 #include "components/account_manager_core/chromeos/account_manager_facade_factory.h"  // nogncheck
 #include "components/account_manager_core/chromeos/fake_account_manager_ui.h"  // nogncheck
+#include "components/variations/variations_switches.h"
 #include "content/public/test/network_connection_change_simulator.h"
 #include "ui/aura/test/ui_controls_factory_aura.h"
 #include "ui/base/test/ui_controls.h"
@@ -513,6 +519,14 @@ void InProcessBrowserTest::TearDown() {
   ash::device_sync::DeviceSyncImpl::Factory::SetCustomFactory(nullptr);
   launch_browser_for_testing_ = nullptr;
 #endif
+
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+  if (ash_process_.IsValid()) {
+    // Need to wait for the termination so the temporary user data dir
+    // can be cleaned up.
+    ash_process_.Terminate(0, /*wait=*/true);
+  }
+#endif
 }
 
 // static
@@ -855,3 +869,90 @@ void InProcessBrowserTest::QuitBrowsers() {
   autorelease_pool_ = nullptr;
 #endif
 }
+
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+void InProcessBrowserTest::StartUniqueAshChrome(
+    const std::vector<std::string>& enabled_features,
+    const std::vector<std::string>& disabled_features,
+    const std::vector<std::string>& additional_cmdline_switches,
+    const std::string& bug_number_and_reason) {
+  DCHECK(!bug_number_and_reason.empty());
+  CHECK(!base::CommandLine::ForCurrentProcess()
+             ->GetSwitchValuePath("lacros-mojo-socket-for-testing")
+             .empty())
+      << "You can only start unique ash chrome when crosapi is enabled. "
+      << "It should not be necessary otherwise.";
+  CHECK(unique_ash_user_data_dir_.CreateUniqueTempDir());
+  base::FilePath socket_file =
+      unique_ash_user_data_dir_.GetPath().Append("lacros.sock");
+
+  // Reset the current test runner connecting to the unique ash chrome.
+  base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
+  cmdline->RemoveSwitch("lacros-mojo-socket-for-testing");
+  cmdline->AppendSwitchPath("lacros-mojo-socket-for-testing", socket_file);
+  // Need unique socket name for wayland globally. So for each ash and lacros
+  // pair, they have a unique socket to communicate.
+  base::Environment::Create()->SetVar(
+      "WAYLAND_DISPLAY",
+      base::JoinString({"unique_wayland",
+                        base::GUID::GenerateRandomV4().AsLowercaseString()},
+                       "_"));
+
+  base::FilePath ash_chrome_path =
+      cmdline->GetSwitchValuePath("ash-chrome-path");
+  CHECK(!ash_chrome_path.empty());
+  base::CommandLine ash_cmdline(ash_chrome_path);
+  ash_cmdline.AppendSwitchPath(switches::kUserDataDir,
+                               unique_ash_user_data_dir_.GetPath());
+  ash_cmdline.AppendSwitch("enable-wayland-server");
+  ash_cmdline.AppendSwitch(switches::kNoStartupWindow);
+  ash_cmdline.AppendSwitch("disable-lacros-keep-alive");
+  ash_cmdline.AppendSwitch("disable-login-lacros-opening");
+  ash_cmdline.AppendSwitch(
+      variations::switches::kEnableFieldTrialTestingConfig);
+  for (const std::string& cmdline_switch : additional_cmdline_switches) {
+    ash_cmdline.AppendSwitch(cmdline_switch);
+  }
+
+  std::vector<std::string> all_enabled_features = {
+      "LacrosSupport", "LacrosPrimary", "LacrosOnly"};
+  all_enabled_features.insert(enabled_features.end(), enabled_features.begin(),
+                              enabled_features.end());
+  ash_cmdline.AppendSwitchASCII(switches::kEnableFeatures,
+                                base::JoinString(all_enabled_features, ","));
+  ash_cmdline.AppendSwitchASCII(switches::kDisableFeatures,
+                                base::JoinString(disabled_features, ","));
+
+  ash_cmdline.AppendSwitchPath("lacros-mojo-socket-for-testing", socket_file);
+  std::string wayland_socket;
+  CHECK(
+      base::Environment::Create()->GetVar("WAYLAND_DISPLAY", &wayland_socket));
+  DCHECK(!wayland_socket.empty());
+  ash_cmdline.AppendSwitchASCII("wayland-server-socket", wayland_socket);
+  const base::FilePath ash_ready_file =
+      unique_ash_user_data_dir_.GetPath().AppendASCII("ash_ready.txt");
+  ash_cmdline.AppendSwitchPath("ash-ready-file-path", ash_ready_file);
+
+  // Need this for RunLoop. See
+  // //docs/threading_and_tasks_testing.md#basetestsinglethreadtaskenvironment
+  base::test::SingleThreadTaskEnvironment task_environment;
+  base::FilePathWatcher watcher;
+  base::RunLoop run_loop;
+  CHECK(watcher.Watch(base::FilePath(ash_ready_file),
+                      base::FilePathWatcher::Type::kNonRecursive,
+                      base::BindLambdaForTesting(
+                          [&](const base::FilePath& filepath, bool error) {
+                            CHECK(!error);
+                            run_loop.Quit();
+                          })));
+  base::LaunchOptions option;
+  ash_process_ = base::LaunchProcess(ash_cmdline, option);
+  CHECK(ash_process_.IsValid());
+  run_loop.Run();
+  // When ash is ready and crosapi was enabled, we expect mojo socket is
+  // also ready.
+  CHECK(base::PathExists(socket_file));
+  LOG(INFO) << "Successfully started a unique ash chrome.";
+}
+
+#endif  // BUILDFLAG(IS_CHROMEOS_LACROS)

chrome/test/base/in_process_browser_test.h

@@ -45,6 +45,10 @@ namespace win {
 class ScopedCOMInitializer;
 }
 #endif  // BUILDFLAG(IS_WIN)
+
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+class Process;
+#endif  // BUILDFLAG(IS_CHROMEOS_LACROS)
 }  // namespace base
 
 #if defined(TOOLKIT_VIEWS)
@@ -187,6 +191,45 @@ class InProcessBrowserTest : public content::BrowserTestBase {
   // The final value of the result is the format of key1=value1;key2=value2.
   void RecordPropertyFromMap(const std::map<std::string, std::string>& tags);
 
+  // Start ash-chrome with specific flags.
+  // In general, there is a shared ash chrome started and a lacros chrome
+  // started before a test case. But for some tests, you may need a special
+  // ash chrome. 2 common use cases:
+  //   1. you need to enable a feature in ash chrome then your test can verify
+  //      some behavior. In this case you need to call this function to start
+  //      a unique ash chrome with the feature enabled.
+  //   2. your test case will pollute ash and cause following test cases fail or
+  //      flaky. Instead of implementing cleanup in TearDown(), using a
+  //      unique ash just for the test is better.
+  // Call this function in the test SetUp() function before invoking
+  // InProcessBrowserTest::SetUp().
+  // This function has negative performance impact:
+  //   1. Start additional ash chrome uses more time.
+  //   2. Additional ash chrome uses more resources.
+  //      The shared ash chrome is still running. By calling this function,
+  //      you start another ash chrome.
+  // Args:
+  //   enabled_features: Additional features to be enabled in ash chrome.
+  //   disabled_features: Additional features to be disabled in ash chrome.
+  //   additional_cmdline_switches: Additional cmdline switches.
+  //       e.g. {"enable-pixel-outputs-in-tests"}
+  //   bug_number_and_reason: Not used in code. But please provide information
+  //       about why you need unique ash chrome. Hopefully this can help reduce
+  //       the usage of unique ash.
+  //       e.g. "crbug.com/11. Switch to shared ash when feature XX is default."
+  //
+  // After you call this function in SetUp(), before the test case test body,
+  // a unique ash chrome will be started and a lacros chrome will be connected
+  // to it. After the test case finishes, the unique ash chrome will be
+  // terminated and the next test case will use the default shared ash chrome.
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+  void StartUniqueAshChrome(
+      const std::vector<std::string>& enabled_features,
+      const std::vector<std::string>& disabled_features,
+      const std::vector<std::string>& additional_cmdline_switches,
+      const std::string& bug_number_and_reason);
+#endif
+
  protected:
   // Closes the given browser and waits for it to release all its resources.
   void CloseBrowserSynchronously(Browser* browser);
@@ -331,6 +374,8 @@ class InProcessBrowserTest : public content::BrowserTestBase {
 #endif
 
  private:
+  friend class StartUniqueAshBrowserTest;
+
   void Initialize();
 
   // Quits all open browsers and waits until there are no more browsers.
@@ -403,6 +448,11 @@ class InProcessBrowserTest : public content::BrowserTestBase {
   std::unique_ptr<ash::full_restore::ScopedLaunchBrowserForTesting>
       launch_browser_for_testing_;
 #endif
+
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+  base::ScopedTempDir unique_ash_user_data_dir_;
+  base::Process ash_process_;
+#endif
 };
 
 // When including either in_process_browser_test.h or android_browser_test.h

chrome/test/base/in_process_browser_test_browsertest_lacros.cc

@@ -0,0 +1,26 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/test/base/in_process_browser_test.h"
+
+#include "content/public/test/browser_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// TODO(crbug.com/1368284): Remove this test when there are more use cases
+// to verify the start ash chrome logic.
+class StartUniqueAshBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUp() override {
+    // Need to put this before starting lacros.
+    StartUniqueAshChrome({}, {}, {"random-unused-example-cmdline"},
+                         "Reason:Test");
+    InProcessBrowserTest::SetUp();
+  }
+
+  void CheckExpectations() { EXPECT_TRUE(ash_process_.IsValid()); }
+};
+
+IN_PROC_BROWSER_TEST_F(StartUniqueAshBrowserTest, StartAshChrome) {
+  CheckExpectations();
+}