Reland "Add Headless Shell discovery to ChromeDriver"

This is a reland of commit d4de6a5 Original change's description: > Add Headless Shell discovery to ChromeDriver > > With this commit ChromeDriver is able to search for > chrome-headless-shell binary. The search is invoked if user sets > "chrome-headless-shell" to "browserName" capability and specifies > no explicit path to the browser binary. > > Bug: chromedriver:4358 > Change-Id: I6b2356518e1959f88bdadd9f444532d5f7bd657b > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4978598 > Commit-Queue: Vladimir Nechaev <nechaev@chromium.org> > Reviewed-by: Mathias Bynens <mathias@chromium.org> > Reviewed-by: Thiago Perrotta <tperrotta@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1216072} Bug: chromedriver:4358 Change-Id: Iad97c4df22610c57e5d6a68880bc6d4e710cadff Validate-Test-Flakiness: skip Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4987262 Reviewed-by: Thiago Perrotta <tperrotta@chromium.org> Commit-Queue: Vladimir Nechaev <nechaev@chromium.org> Cr-Commit-Position: refs/heads/main@{#1216897}
chromium · Oct 30, 2023 · 754e352 · 754e352
1 parent fb1f493
commit 754e352
Show file tree

Hide file tree

Showing 12 changed files with 210 additions and 62 deletions.
diff --git a/chrome/test/chromedriver/chrome/browser_info.cc b/chrome/test/chromedriver/chrome/browser_info.cc
@@ -119,8 +119,7 @@ Status BrowserInfo::ParseBrowserString(bool has_android_package,
 
     if (build_no != 0) {
       if (headless_shell) {
-        browser_info->browser_name =
-            base::StringPrintf("headless %s", kBrowserCapabilityName);
+        browser_info->browser_name = kHeadlessShellCapabilityName;
         browser_info->is_headless_shell = true;
       } else {
         browser_info->browser_name = kBrowserCapabilityName;

diff --git a/chrome/test/chromedriver/chrome/browser_info_unittest.cc b/chrome/test/chromedriver/chrome/browser_info_unittest.cc
@@ -114,7 +114,7 @@ TEST(ParseBrowserString, HeadlessChrome) {
   Status status = BrowserInfo::ParseBrowserString(
       false, "HeadlessChrome/39.0.2171.59", &browser_info);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ("headless chrome", browser_info.browser_name);
+  ASSERT_EQ("chrome-headless-shell", browser_info.browser_name);
   ASSERT_EQ("39.0.2171.59", browser_info.browser_version);
   ASSERT_EQ(39, browser_info.major_version);
   ASSERT_EQ(2171, browser_info.build_no);
@@ -175,7 +175,7 @@ TEST(FillFromBrowserVersionResponse, HeadlessChrome) {
   BrowserInfo browser_info;
   Status status = browser_info.FillFromBrowserVersionResponse(response);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ("headless chrome", browser_info.browser_name);
+  ASSERT_EQ("chrome-headless-shell", browser_info.browser_name);
   ASSERT_EQ("39.0.2171.59", browser_info.browser_version);
   ASSERT_EQ(39, browser_info.major_version);
   ASSERT_EQ(2171, browser_info.build_no);

diff --git a/chrome/test/chromedriver/chrome/chrome_finder.cc b/chrome/test/chromedriver/chrome/chrome_finder.cc
@@ -22,6 +22,7 @@
 #include "build/branding_buildflags.h"
 #include "build/build_config.h"
 #include "chrome/common/chrome_constants.h"
+#include "chrome/test/chromedriver/constants/version.h"
 
 #if BUILDFLAG(IS_WIN)
 #include "base/base_paths_win.h"
@@ -111,6 +112,44 @@ void GetPathsFromEnvironment(std::vector<base::FilePath>* paths) {
   }
 }
 
+std::vector<base::FilePath> GetChromeProgramNames() {
+  return {
+#if BUILDFLAG(IS_WIN)
+    base::FilePath(chrome::kBrowserProcessExecutablePath),
+        base::FilePath(FILE_PATH_LITERAL(
+            "chrome.exe")),  // Chrome for Testing or Google Chrome
+        base::FilePath(FILE_PATH_LITERAL("chromium.exe")),
+#elif BUILDFLAG(IS_MAC)
+    base::FilePath(chrome::kBrowserProcessExecutablePath),
+        base::FilePath(
+            chrome::kGoogleChromeForTestingBrowserProcessExecutablePath),
+        base::FilePath(chrome::kGoogleChromeBrowserProcessExecutablePath),
+        base::FilePath(chrome::kChromiumBrowserProcessExecutablePath),
+#elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
+    base::FilePath(chrome::kBrowserProcessExecutablePath),
+        base::FilePath("chrome"),  // Chrome for Testing or Google Chrome
+        base::FilePath("google-chrome"), base::FilePath("chromium"),
+        base::FilePath("chromium-browser"),
+#else
+    // it will compile but won't work on other OSes
+    base::FilePath()
+#endif
+  };
+}
+
+std::vector<base::FilePath> GetHeadlessShellProgramNames() {
+  return {
+#if BUILDFLAG(IS_WIN)
+    base::FilePath(FILE_PATH_LITERAL("chrome-headless-shell.exe")),
+#elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_CHROMEOS)
+    base::FilePath("chrome-headless-shell"),
+#else
+    // it will compile but won't work on other OSes
+    base::FilePath()
+#endif
+  };
+}
+
 }  // namespace
 
 namespace internal {
@@ -119,14 +158,14 @@ bool FindExe(
     const base::RepeatingCallback<bool(const base::FilePath&)>& exists_func,
     const std::vector<base::FilePath>& rel_paths,
     const std::vector<base::FilePath>& locations,
-    base::FilePath* out_path) {
+    base::FilePath& out_path) {
   for (auto& rel_path : rel_paths) {
     for (auto& location : locations) {
       base::FilePath path = location.Append(rel_path);
       VLOG(logging::LOGGING_INFO) << "Browser search. Trying... " << path;
       if (exists_func.Run(path)) {
         VLOG(logging::LOGGING_INFO) << "Browser search. Found at  " << path;
-        *out_path = path;
+        out_path = path;
         return true;
       }
     }
@@ -140,41 +179,39 @@ bool FindExe(
 void GetApplicationDirs(std::vector<base::FilePath>* locations);
 #endif
 
+bool FindBrowser(const std::string& browser_name, base::FilePath& browser_exe) {
+  return FindBrowser(browser_name, base::BindRepeating(&base::PathExists),
+                     browser_exe);
+}
+
 /**
- * Finds a chrome executable based on the following flavour priority:
+ * Finds a browser executable for the provided |browser_name|.
+ * For "chrome" each directory is searched in the following flavour priority:
  *   - `PRODUCT_STRING`
  *   - google chrome for testing
  *   - google chrome
  *   - chromium
+ * For "chrome-headless-shell" the executable name without extension is always
+ * expected to be chrome-headless-shell.
  */
-bool FindChrome(base::FilePath* browser_exe) {
-  base::FilePath browser_exes_array[] = {
-#if BUILDFLAG(IS_WIN)
-    base::FilePath(chrome::kBrowserProcessExecutablePath),
-    base::FilePath(FILE_PATH_LITERAL(
-        "chrome.exe")),  // Chrome for Testing or Google Chrome
-    base::FilePath(FILE_PATH_LITERAL("chromium.exe")),
-#elif BUILDFLAG(IS_MAC)
-    base::FilePath(chrome::kBrowserProcessExecutablePath),
-    base::FilePath(chrome::kGoogleChromeForTestingBrowserProcessExecutablePath),
-    base::FilePath(chrome::kGoogleChromeBrowserProcessExecutablePath),
-    base::FilePath(chrome::kChromiumBrowserProcessExecutablePath),
-#elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
-    base::FilePath(chrome::kBrowserProcessExecutablePath),
-    base::FilePath("chrome"),  // Chrome for Testing or Google Chrome
-    base::FilePath("google-chrome"),
-    base::FilePath("chromium"),
-    base::FilePath("chromium-browser"),
-#else
-    // it will compile but won't work on other OSes
-    base::FilePath()
-#endif
-  };
+bool FindBrowser(
+    const std::string& browser_name,
+    const base::RepeatingCallback<bool(const base::FilePath&)>& exists_func,
+    base::FilePath& browser_exe) {
+  std::vector<base::FilePath> browser_exes;
+  if (browser_name == kHeadlessShellCapabilityName) {
+    browser_exes = GetHeadlessShellProgramNames();
+  } else if (browser_name == kBrowserCapabilityName || browser_name.empty()) {
+    // Empty browser_name means that "browserName" capability was not provided.
+    // In this case ChromeDriver defaults to "chrome".
+    browser_exes = GetChromeProgramNames();
+  } else {
+    VLOG(logging::LOGGING_ERROR) << "Unknown browser name: " << browser_name;
+    return false;
+  }
 
-  LOG_IF(ERROR, browser_exes_array[0].empty()) << "Unsupported platform.";
+  LOG_IF(ERROR, browser_exes[0].empty()) << "Unsupported platform.";
 
-  std::vector<base::FilePath> browser_exes(
-      browser_exes_array, browser_exes_array + std::size(browser_exes_array));
   base::FilePath module_dir;
 #if BUILDFLAG(IS_FUCHSIA)
   // Use -1 to allow this to compile.
@@ -187,8 +224,8 @@ bool FindChrome(base::FilePath* browser_exe) {
     for (const base::FilePath& file_path : browser_exes) {
       base::FilePath path = module_dir.Append(file_path);
       VLOG(logging::LOGGING_INFO) << "Browser search. Trying... " << path;
-      if (base::PathExists(path)) {
-        *browser_exe = path;
+      if (exists_func.Run(path)) {
+        browser_exe = path;
         VLOG(logging::LOGGING_INFO) << "Browser search. Found at  " << path;
         return true;
       }
@@ -198,8 +235,8 @@ bool FindChrome(base::FilePath* browser_exe) {
   std::vector<base::FilePath> locations;
   GetApplicationDirs(&locations);
   GetPathsFromEnvironment(&locations);
-  bool found = internal::FindExe(base::BindRepeating(&base::PathExists),
-                                 browser_exes, locations, browser_exe);
+  bool found =
+      internal::FindExe(exists_func, browser_exes, locations, browser_exe);
   if (!found) {
     VLOG(logging::LOGGING_INFO) << "Browser search. Not found.";
   }

diff --git a/chrome/test/chromedriver/chrome/chrome_finder.h b/chrome/test/chromedriver/chrome/chrome_finder.h
@@ -13,16 +13,27 @@ namespace base {
 class FilePath;
 }
 
-// Gets the path to the default Chrome executable. Returns true on success.
-bool FindChrome(base::FilePath* browser_exe);
+// Gets the path to the default Chrome or Headless Shell executable.
+// Supported |browser_name| values are empty string, "chrome" and
+// "chrome-headless-shell". The empty string defaults to "chrome" with the
+// corresponding warning issued.
+// For other browser names the returned value is false.
+// Returns true on success.
+bool FindBrowser(const std::string& browser_name, base::FilePath& browser_exe);
+
+// The overload for testing purposes
+bool FindBrowser(
+    const std::string& browser_name,
+    const base::RepeatingCallback<bool(const base::FilePath&)>& exists_func,
+    base::FilePath& browser_exe);
 
 namespace internal {
 
 bool FindExe(
     const base::RepeatingCallback<bool(const base::FilePath&)>& exists_func,
     const std::vector<base::FilePath>& rel_paths,
     const std::vector<base::FilePath>& locations,
-    base::FilePath* out_path);
+    base::FilePath& out_path);
 
 }  // namespace internal
 

diff --git a/chrome/test/chromedriver/chrome/chrome_finder_unittest.cc b/chrome/test/chromedriver/chrome/chrome_finder_unittest.cc
@@ -7,21 +7,20 @@
 #include <string>
 #include <vector>
 
+#include "base/containers/contains.h"
 #include "base/files/file_path.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
+#include "base/strings/string_util.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/test/chromedriver/chrome/chrome_finder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 bool PathIn(const std::vector<base::FilePath>& list,
             const base::FilePath& path) {
-  for (size_t i = 0; i < list.size(); ++i) {
-    if (list[i] == path)
-      return true;
-  }
-  return false;
+  return base::Contains(list, path);
 }
 
 void AssertFound(const base::FilePath& found,
@@ -30,7 +29,7 @@ void AssertFound(const base::FilePath& found,
                  const std::vector<base::FilePath>& locations) {
   base::FilePath exe;
   ASSERT_TRUE(internal::FindExe(base::BindRepeating(&PathIn, existing_paths),
-                                rel_paths, locations, &exe));
+                                rel_paths, locations, exe));
   ASSERT_EQ(found, exe);
 }
 
@@ -93,12 +92,91 @@ TEST(ChromeFinderTest, FindExeNotFound) {
   locations.push_back(found.DirName());
   base::FilePath exe;
   ASSERT_FALSE(internal::FindExe(base::BindRepeating(&PathIn, existing_paths),
-                                 rel_paths, locations, &exe));
+                                 rel_paths, locations, exe));
 }
 
 TEST(ChromeFinderTest, NoCrash) {
   // It's not worthwhile to check the validity of the path, so just check
   // for crashes.
   base::FilePath exe;
-  FindChrome(&exe);
+  FindBrowser("chrome", exe);
+  FindBrowser("chrome-headless-shell", exe);
+  FindBrowser("", exe);
+  FindBrowser("quick-brown-fox", exe);
+}
+
+TEST(ChromeFinderTest, FindBrowserSearchesForChrome) {
+  // Verify that FindBrowser searches for Chrome when requested to
+  // find Chrome.
+  base::FilePath exe;
+  auto exists_func = base::BindRepeating([](const base::FilePath& path) {
+    std::vector<base::FilePath::StringType> components = path.GetComponents();
+    EXPECT_FALSE(components.empty());
+    if (components.empty()) {
+      return false;
+    }
+    return components.back() == chrome::kBrowserProcessExecutableName;
+  });
+  EXPECT_TRUE(FindBrowser("chrome", exists_func, exe));
+  // Empty string defaults to "chrome". This mimics the situation when the
+  // "browserName" capability is not provided.
+  EXPECT_TRUE(FindBrowser("", exists_func, exe));
+}
+
+TEST(ChromeFinderTest, FindBrowserSearchesForHeadlessShell) {
+  // Verify that FindBrowser searches for HeadlessShell when requested to
+  // find HeadlessShell.
+  base::FilePath exe;
+  auto exists_func = base::BindRepeating([](const base::FilePath& path) {
+    std::vector<base::FilePath::StringType> components = path.GetComponents();
+    EXPECT_FALSE(components.empty());
+    if (components.empty()) {
+      return false;
+    }
+    return base::StartsWith(components.back(),
+                            FILE_PATH_LITERAL("chrome-headless-shell"));
+  });
+  EXPECT_TRUE(FindBrowser("chrome-headless-shell", exists_func, exe));
+}
+
+TEST(ChromeFinderTest, FindBrowserDoesNotSearchForUnsupportedBrowser) {
+  // Verify that FindBrowser returns false for unknown browsers.
+  base::FilePath exe;
+  auto exists_func =
+      base::BindRepeating([](const base::FilePath& path) { return true; });
+  EXPECT_FALSE(FindBrowser("quick-brown-fox", exists_func, exe));
+}
+
+TEST(ChromeFinderTest, FindBrowserDoesNotSearchForChrome) {
+  // Verify that FindBrowser does not search for Chrome when requested to
+  // find HeadlessShell.
+  base::FilePath exe;
+  auto exists_func = base::BindRepeating([](const base::FilePath& path) {
+    std::vector<base::FilePath::StringType> components = path.GetComponents();
+    EXPECT_FALSE(components.empty());
+    if (components.empty()) {
+      return false;
+    }
+    return components.back() == chrome::kBrowserProcessExecutableName;
+  });
+  EXPECT_FALSE(FindBrowser("chrome-headless-shell", exists_func, exe));
+}
+
+TEST(ChromeFinderTest, FindBrowserDoesNotSearchForHeadlessShell) {
+  // Verify that FindBrowser does not search for HeadlessShell when requested to
+  // find Chrome.
+  base::FilePath exe;
+  auto exists_func = base::BindRepeating([](const base::FilePath& path) {
+    std::vector<base::FilePath::StringType> components = path.GetComponents();
+    EXPECT_FALSE(components.empty());
+    if (components.empty()) {
+      return false;
+    }
+    return base::StartsWith(components.back(),
+                            FILE_PATH_LITERAL("chrome-headless-shell"));
+  });
+  EXPECT_FALSE(FindBrowser("chrome", exists_func, exe));
+  // Empty string defaults to "chrome". This mimics the situation when the
+  // "browserName" capability is not provided.
+  EXPECT_FALSE(FindBrowser("", exists_func, exe));
 }
diff --git a/chrome/test/chromedriver/chrome_launcher.cc b/chrome/test/chromedriver/chrome_launcher.cc
@@ -130,9 +130,10 @@ Status PrepareDesktopCommandLine(const Capabilities& capabilities,
                                  base::FilePath& user_data_dir) {
   base::FilePath program = capabilities.binary;
   if (program.empty()) {
-    if (!FindChrome(&program))
+    if (!FindBrowser(capabilities.browser_name, program)) {
       return Status(kUnknownError, base::StringPrintf("cannot find %s binary",
                                                       kBrowserShortName));
+    }
   } else if (!base::PathExists(program)) {
     return Status(
         kUnknownError,

diff --git a/chrome/test/chromedriver/client/chromedriver.py b/chrome/test/chromedriver/client/chromedriver.py
@@ -106,7 +106,7 @@ def _InternalInit(self, server_url,
       send_w3c_capability=True, send_w3c_request=True,
       page_load_strategy=None, unexpected_alert_behaviour=None,
       devtools_events_to_log=None, accept_insecure_certs=None,
-      timeouts=None, test_name=None, web_socket_url=None):
+      timeouts=None, test_name=None, web_socket_url=None, browser_name=None):
     self._executor = command_executor.CommandExecutor(server_url)
     self._server_url = server_url
     self.w3c_compliant = False
@@ -226,6 +226,9 @@ def _InternalInit(self, server_url,
     if web_socket_url is not None:
       params['webSocketUrl'] = web_socket_url
 
+    if browser_name is not None:
+      params['browserName'] = browser_name
+
     if send_w3c_request:
       params = {'capabilities': {'alwaysMatch': params}}
     else:

diff --git a/chrome/test/chromedriver/constants/BRANDING b/chrome/test/chromedriver/constants/BRANDING
@@ -1,10 +1,11 @@
-COMPANY_FULLNAME=The Chromium Authors
-COMPANY_SHORTNAME=The Chromium Authors
-COMPANY_PREFIX=goog
-PRODUCT_FULLNAME=ChromeDriver
-PRODUCT_SHORTNAME=ChromeDriver
-BROWSER_OPTIONS_KEY=chromeOptions
-BROWSER_SHORTNAME=Chrome
-BROWSER_CAPABILITYNAME=chrome
-USER_AGENT_PRODUCT_NAME=Chrome
-HEADLESS_USER_AGENT_PRODUCT_NAME=HeadlessChrome
+COMPANY_FULLNAME=The Chromium Authors
+COMPANY_SHORTNAME=The Chromium Authors
+COMPANY_PREFIX=goog
+PRODUCT_FULLNAME=ChromeDriver
+PRODUCT_SHORTNAME=ChromeDriver
+BROWSER_OPTIONS_KEY=chromeOptions
+BROWSER_SHORTNAME=Chrome
+BROWSER_CAPABILITYNAME=chrome
+HEADLESS_CAPABILITYNAME=chrome-headless-shell
+USER_AGENT_PRODUCT_NAME=Chrome
+HEADLESS_USER_AGENT_PRODUCT_NAME=HeadlessChrome