[BrowserSwitcher] Add Mac OS support
This enables the build on Mac OS X, and refactors
AlternativeBrowserDriver to work correctly on Mac OS.
Bug: 881589
Change-Id: I6e73d33d83af5bc1c0d8bb3ec3e76fcf8409ee2e
Reviewed-on: https://chromium-review.googlesource.com/c/1283835
Reviewed-by: Julian Pastarmov <[email protected]>
Reviewed-by: Gabriel Charette <[email protected]>
Commit-Queue: Nicolas Ouellet-Payeur <[email protected]>
Cr-Commit-Position: refs/heads/master@{#601570}diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 7a38cfa..4883fe06 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -3438,10 +3438,9 @@
]
}
- if (is_win || (is_linux && !is_chromeos)) {
+ if (is_win || is_mac || is_desktop_linux) {
sources += [
"browser_switcher/alternative_browser_driver.h",
- "browser_switcher/alternative_browser_driver_linux.cc",
"browser_switcher/alternative_browser_driver_win.cc",
"browser_switcher/alternative_browser_launcher.cc",
"browser_switcher/alternative_browser_launcher.h",
@@ -3458,6 +3457,9 @@
"browser_switcher/ieem_sitelist_parser.cc",
"browser_switcher/ieem_sitelist_parser.h",
]
+ if (is_mac || is_desktop_linux) {
+ sources += [ "browser_switcher/alternative_browser_driver_posix.cc" ]
+ }
}
if (is_chromeos && use_cras) {
diff --git a/chrome/browser/browser_switcher/alternative_browser_driver.h b/chrome/browser/browser_switcher/alternative_browser_driver.h
index c6de679..7662f97 100644
--- a/chrome/browser/browser_switcher/alternative_browser_driver.h
+++ b/chrome/browser/browser_switcher/alternative_browser_driver.h
@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_BROWSER_SWITCHER_ALTERNATIVE_BROWSER_DRIVER_H_
#define CHROME_BROWSER_BROWSER_SWITCHER_ALTERNATIVE_BROWSER_DRIVER_H_
+#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/strings/string_piece_forward.h"
@@ -51,17 +52,13 @@
void SetBrowserParameters(const base::ListValue* parameters) override;
bool TryLaunch(const GURL& url) override;
- using StringType = base::FilePath::StringType;
-
- // Appends the arguments in |raw_args| to |argv|, performing substitution of
- // "${url}" at the same time. If none of |raw_args| contains "${url}", the URL
- // is appended as the last argument.
- static void AppendCommandLineArguments(
- std::vector<StringType>* argv,
- const std::vector<StringType>& raw_args,
- const GURL& url);
+ // Create the CommandLine object that would be used to launch an external
+ // process.
+ base::CommandLine CreateCommandLine(const GURL& url);
private:
+ using StringType = base::FilePath::StringType;
+
#if defined(OS_WIN)
bool TryLaunchWithDde(const GURL& url);
bool TryLaunchWithExec(const GURL& url);
diff --git a/chrome/browser/browser_switcher/alternative_browser_driver_linux.cc b/chrome/browser/browser_switcher/alternative_browser_driver_posix.cc
similarity index 61%
rename from chrome/browser/browser_switcher/alternative_browser_driver_linux.cc
rename to chrome/browser/browser_switcher/alternative_browser_driver_posix.cc
index 5e6dded..53531ec 100644
--- a/chrome/browser/browser_switcher/alternative_browser_driver_linux.cc
+++ b/chrome/browser/browser_switcher/alternative_browser_driver_posix.cc
@@ -4,11 +4,11 @@
#include "chrome/browser/browser_switcher/alternative_browser_driver.h"
-#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/process/launch.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
#include "chrome/browser/browser_switcher/alternative_browser_launcher.h"
#include "url/gurl.h"
@@ -22,13 +22,23 @@
const char kUrlVarName[] = "${url}";
+#if defined(OS_MACOSX)
+const char kChromeExecutableName[] = "Google Chrome";
+const char kFirefoxExecutableName[] = "Firefox";
+const char kOperaExecutableName[] = "Opera";
+const char kSafariExecutableName[] = "Safari";
+#else
const char kChromeExecutableName[] = "google-chrome";
const char kFirefoxExecutableName[] = "firefox";
const char kOperaExecutableName[] = "opera";
+#endif
const char kChromeVarName[] = "${chrome}";
const char kFirefoxVarName[] = "${firefox}";
const char kOperaVarName[] = "${opera}";
+#if defined(OS_MACOSX)
+const char kSafariVarName[] = "${safari}";
+#endif
const struct {
const char* var_name;
@@ -37,6 +47,9 @@
{kChromeVarName, kChromeExecutableName},
{kFirefoxVarName, kFirefoxExecutableName},
{kOperaVarName, kOperaExecutableName},
+#if defined(OS_MACOSX)
+ {kSafariVarName, kSafariExecutableName},
+#endif
};
bool ExpandUrlVarName(std::string* arg, const GURL& url) {
@@ -81,6 +94,33 @@
std::swap(out, *arg);
}
+#if defined(OS_MACOSX)
+bool ContainsUrlVarName(const std::vector<std::string>& tokens) {
+ return std::any_of(tokens.begin(), tokens.end(),
+ [](const std::string& token) {
+ return token.find(kUrlVarName) != std::string::npos;
+ });
+}
+#endif // defined(OS_MACOSX)
+
+// static
+void AppendCommandLineArguments(base::CommandLine* cmd_line,
+ const std::vector<std::string>& raw_args,
+ const GURL& url,
+ bool always_append_url) {
+ bool contains_url = false;
+ for (const auto& arg : raw_args) {
+ std::string expanded_arg = arg;
+ ExpandTilde(&expanded_arg);
+ ExpandEnvironmentVariables(&expanded_arg);
+ if (ExpandUrlVarName(&expanded_arg, url))
+ contains_url = true;
+ cmd_line->AppendArg(expanded_arg);
+ }
+ if (always_append_url && !contains_url)
+ cmd_line->AppendArg(url.spec());
+}
+
} // namespace
AlternativeBrowserDriver::~AlternativeBrowserDriver() {}
@@ -90,6 +130,13 @@
void AlternativeBrowserDriverImpl::SetBrowserPath(base::StringPiece path) {
browser_path_ = path.as_string();
+#if defined(OS_MACOSX)
+ // Unlike most POSIX platforms, MacOS always has another browser than Chrome,
+ // so admins don't have to explicitly configure one.
+ if (browser_path_.empty()) {
+ browser_path_ = kSafariExecutableName;
+ }
+#endif
for (const auto& mapping : kBrowserVarMappings) {
if (!browser_path_.compare(mapping.var_name)) {
browser_path_ = mapping.executable_name;
@@ -120,16 +167,7 @@
CHECK(url.SchemeIsHTTPOrHTTPS() || url.SchemeIsFile());
- const int max_num_args = browser_params_.size() + 2;
- std::vector<std::string> argv;
- argv.reserve(max_num_args);
- std::string path = browser_path_;
- ExpandTilde(&path);
- ExpandEnvironmentVariables(&path);
- argv.push_back(path);
- AppendCommandLineArguments(&argv, browser_params_, url);
-
- base::CommandLine cmd_line = base::CommandLine(argv);
+ auto cmd_line = CreateCommandLine(url);
base::LaunchOptions options;
// Don't close the alternative browser when Chrome exits.
options.new_process_group = true;
@@ -140,23 +178,45 @@
return true;
}
-// static
-void AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- std::vector<std::string>* argv,
- const std::vector<std::string>& raw_args,
+base::CommandLine AlternativeBrowserDriverImpl::CreateCommandLine(
const GURL& url) {
- // TODO(crbug/882520): Do environment variable and tilde expansion.
- bool contains_url = false;
- for (const auto& arg : raw_args) {
- std::string expanded_arg = arg;
- ExpandTilde(&expanded_arg);
- ExpandEnvironmentVariables(&expanded_arg);
- if (ExpandUrlVarName(&expanded_arg, url))
- contains_url = true;
- argv->push_back(expanded_arg);
+ std::string path = browser_path_;
+ ExpandTilde(&path);
+ ExpandEnvironmentVariables(&path);
+
+#if defined(OS_MACOSX)
+ // On MacOS, if the path doesn't start with a '/', it's probably not an
+ // executable path. It is probably a name for an application, e.g. "Safari" or
+ // "Google Chrome". Those can be launched using the `open(1)' command.
+ //
+ // It may use the following syntax (first syntax):
+ // open -a <browser_path> <url> [--args <browser_params...>]
+ //
+ // Or, if |browser_params| contains "${url}" (second syntax):
+ // open -a <browser_path> --args <browser_params...>
+ //
+ // Safari only supports the first syntax.
+ if (!browser_path_.empty() && browser_path_[0] != '/') {
+ base::CommandLine cmd_line(std::vector<std::string>{"open"});
+ cmd_line.AppendArg("-a");
+ cmd_line.AppendArg(path);
+ if (!ContainsUrlVarName(browser_params_)) {
+ // First syntax.
+ cmd_line.AppendArg(url.spec());
+ }
+ if (!browser_params_.empty()) {
+ // First or second syntax, depending on what is in |browser_params_|.
+ cmd_line.AppendArg("--args");
+ AppendCommandLineArguments(&cmd_line, browser_params_, url,
+ /* always_append_url */ false);
+ }
+ return cmd_line;
}
- if (!contains_url)
- argv->push_back(url.spec());
+#endif
+ base::CommandLine cmd_line(std::vector<std::string>{path});
+ AppendCommandLineArguments(&cmd_line, browser_params_, url,
+ /* always_append_url */ true);
+ return cmd_line;
}
} // namespace browser_switcher
diff --git a/chrome/browser/browser_switcher/alternative_browser_driver_unittest.cc b/chrome/browser/browser_switcher/alternative_browser_driver_unittest.cc
index 1a28507..640473c 100644
--- a/chrome/browser/browser_switcher/alternative_browser_driver_unittest.cc
+++ b/chrome/browser/browser_switcher/alternative_browser_driver_unittest.cc
@@ -20,17 +20,19 @@
StringType UTF8ToNative(base::StringPiece src) {
#if defined(OS_WIN)
return base::UTF8ToWide(src);
-#elif defined(OS_LINUX)
+#elif defined(OS_POSIX)
return src.as_string();
+#else
+#error "Invalid platform for browser_switcher"
#endif
}
-std::vector<StringType> UTF8VectorToNative(
+base::ListValue UTF8VectorToListValue(
const std::vector<base::StringPiece>& src) {
- std::vector<StringType> out;
- out.reserve(src.size());
- std::transform(src.begin(), src.end(), std::back_inserter(out),
- &UTF8ToNative);
+ base::ListValue out;
+ out.GetList().reserve(src.size());
+ for (base::StringPiece str : src)
+ out.GetList().push_back(base::Value(str));
return out;
}
@@ -38,108 +40,199 @@
class AlternativeBrowserDriverTest : public testing::Test {};
-TEST_F(AlternativeBrowserDriverTest, AppendCommandLineArguments) {
- std::vector<StringType> argv = {UTF8ToNative("wget")};
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, UTF8VectorToNative({"-O", "-", "--"}),
- GURL("https://example.com/"));
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLine) {
+ AlternativeBrowserDriverImpl driver;
+ driver.SetBrowserPath("/usr/bin/true");
+ auto params = UTF8VectorToListValue({"a", "b", "c"});
+ driver.SetBrowserParameters(¶ms);
+ auto cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ const base::CommandLine::StringVector& argv = cmd_line.argv();
EXPECT_EQ(5u, argv.size());
- EXPECT_EQ(UTF8ToNative("wget"), argv[0]);
- EXPECT_EQ(UTF8ToNative("-O"), argv[1]);
- EXPECT_EQ(UTF8ToNative("-"), argv[2]);
- EXPECT_EQ(UTF8ToNative("--"), argv[3]);
- EXPECT_EQ(UTF8ToNative("https://example.com/"), argv[4]);
+ EXPECT_EQ(UTF8ToNative("/usr/bin/true"), argv[0]);
+ EXPECT_EQ(UTF8ToNative("a"), argv[1]);
+ EXPECT_EQ(UTF8ToNative("b"), argv[2]);
+ EXPECT_EQ(UTF8ToNative("c"), argv[3]);
+ EXPECT_EQ(UTF8ToNative("http://example.com/"), argv[4]);
}
-TEST_F(AlternativeBrowserDriverTest, AppendCommandLineArgumentsExpandsUrl) {
- std::vector<StringType> argv = {UTF8ToNative("google-chrome")};
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, UTF8VectorToNative({"--app=${url}#fragment"}),
- GURL("https://example.com/"));
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLineExpandsUrl) {
+ AlternativeBrowserDriverImpl driver;
+ driver.SetBrowserPath("/usr/bin/true");
+ auto params = UTF8VectorToListValue({"--flag=${url}#fragment"});
+ driver.SetBrowserParameters(¶ms);
+ auto cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ const base::CommandLine::StringVector& argv = cmd_line.argv();
EXPECT_EQ(2u, argv.size());
- EXPECT_EQ(UTF8ToNative("google-chrome"), argv[0]);
- EXPECT_EQ(UTF8ToNative("--app=https://example.com/#fragment"), argv[1]);
+ EXPECT_EQ(UTF8ToNative("/usr/bin/true"), argv[0]);
+ EXPECT_EQ(UTF8ToNative("--flag=http://example.com/#fragment"), argv[1]);
}
#if defined(OS_WIN)
-TEST_F(AlternativeBrowserDriverTest, AppendCommandLineArgumentsExpandsEnvVars) {
- std::vector<std::wstring> argv = {L"something.exe"};
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLineExpandsEnvVars) {
_putenv("A=AAA");
_putenv("B=BBB");
_putenv("CC=CCC");
_putenv("D=DDD");
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv,
- {L"%A%", L"%B%", L"before_%CC%_between_%D%_after", L"%NONEXISTENT%"},
- GURL("https://example.com/"));
+ AlternativeBrowserDriverImpl driver;
+ driver.SetBrowserPath("something.exe");
+ auto params = UTF8VectorToListValue(
+ {"%A%", "%B%", "before_%CC%_between_%D%_after", "%NONEXISTENT%"});
+ driver.SetBrowserParameters(¶ms);
+ auto cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ const base::CommandLine::StringVector& argv = cmd_line.argv();
EXPECT_EQ(6u, argv.size());
EXPECT_EQ(L"something.exe", argv[0]);
EXPECT_EQ(L"AAA", argv[1]);
EXPECT_EQ(L"BBB", argv[2]);
EXPECT_EQ(L"before_CCC_between_DDD_after", argv[3]);
EXPECT_EQ(L"%NONEXISTENT%", argv[4]);
- EXPECT_EQ(L"https://example.com/", argv[5]);
+ EXPECT_EQ(L"http://example.com/", argv[5]);
}
TEST_F(AlternativeBrowserDriverTest,
AppendCommandLineArgumentDoesntExpandUrlContent) {
- std::vector<std::wstring> argv = {L"something.exe"};
_putenv("A=AAA");
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, {}, GURL("https://evil.com/%A%"));
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, {L"${url}"}, GURL("https://evil.com/%A%"));
- EXPECT_EQ(3u, argv.size());
- EXPECT_EQ(L"something.exe", argv[0]);
- EXPECT_EQ(L"https://evil.com/%A%", argv[1]);
- EXPECT_EQ(L"https://evil.com/%A%", argv[2]);
+ AlternativeBrowserDriverImpl driver;
+ driver.SetBrowserPath("something.exe");
+ auto cmd_line = driver.CreateCommandLine(GURL("http://evil.com/%A%"));
+ EXPECT_EQ(2u, cmd_line.argv().size());
+ EXPECT_EQ(L"something.exe", cmd_line.argv()[0]);
+ EXPECT_EQ(L"http://evil.com/%A%", cmd_line.argv()[1]);
+
+ auto params = UTF8VectorToListValue({"${url}"});
+ driver.SetBrowserParameters(¶ms);
+ cmd_line = driver.CreateCommandLine(GURL("http://evil.com/%A%"));
+ EXPECT_EQ(2u, cmd_line.argv().size());
+ EXPECT_EQ(L"something.exe", cmd_line.argv()[0]);
+ EXPECT_EQ(L"http://evil.com/%A%", cmd_line.argv()[1]);
}
#endif // defined(OS_WIN)
-#if defined(OS_POSIX)
-TEST_F(AlternativeBrowserDriverTest, AppendCommandLineArgumentsExpandsTilde) {
- std::vector<std::string> argv = {"some_script"};
- setenv("HOME", "/home/foobar", true);
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, {"~/file.txt", "/tmp/backup.txt~"}, GURL("https://example.com/"));
- EXPECT_EQ(4u, argv.size());
- EXPECT_EQ("some_script", argv[0]);
- EXPECT_EQ("/home/foobar/file.txt", argv[1]);
- EXPECT_EQ("/tmp/backup.txt~", argv[2]);
- EXPECT_EQ("https://example.com/", argv[3]);
+#if defined(OS_MACOSX)
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLineUsesOpen) {
+ // Use `open(1)' to launch browser paths that aren't absolute.
+ AlternativeBrowserDriverImpl driver;
+
+ // Default to launching Safari.
+ driver.SetBrowserPath("");
+ auto cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ EXPECT_EQ(4u, cmd_line.argv().size());
+ EXPECT_EQ("open", cmd_line.argv()[0]);
+ EXPECT_EQ("-a", cmd_line.argv()[1]);
+ EXPECT_EQ("Safari", cmd_line.argv()[2]);
+ EXPECT_EQ("http://example.com/", cmd_line.argv()[3]);
+
+ // Expand some "${...}" variables.
+ driver.SetBrowserPath("${safari}");
+ cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ EXPECT_EQ(4u, cmd_line.argv().size());
+ EXPECT_EQ("open", cmd_line.argv()[0]);
+ EXPECT_EQ("-a", cmd_line.argv()[1]);
+ EXPECT_EQ("Safari", cmd_line.argv()[2]);
+ EXPECT_EQ("http://example.com/", cmd_line.argv()[3]);
+
+ // Path looks like an application name => use `open'.
+ driver.SetBrowserPath("Safari");
+ cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ EXPECT_EQ(4u, cmd_line.argv().size());
+ EXPECT_EQ("open", cmd_line.argv()[0]);
+ EXPECT_EQ("-a", cmd_line.argv()[1]);
+ EXPECT_EQ("Safari", cmd_line.argv()[2]);
+ EXPECT_EQ("http://example.com/", cmd_line.argv()[3]);
}
-TEST_F(AlternativeBrowserDriverTest, AppendCommandLineArgumentsExpandsEnvVars) {
- std::vector<std::string> argv = {"some_script"};
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLineContainsUrl) {
+ // Use `open(1)' to launch browser paths that aren't absolute.
+ AlternativeBrowserDriverImpl driver;
+
+ // Args come after `--args', e.g.:
+ // open -a Safari http://example.com/ --args abc def
+ driver.SetBrowserPath("");
+ auto params = UTF8VectorToListValue({"abc", "def"});
+ driver.SetBrowserParameters(¶ms);
+ auto cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ EXPECT_EQ(7u, cmd_line.argv().size());
+ EXPECT_EQ("open", cmd_line.argv()[0]);
+ EXPECT_EQ("-a", cmd_line.argv()[1]);
+ EXPECT_EQ("Safari", cmd_line.argv()[2]);
+ EXPECT_EQ("http://example.com/", cmd_line.argv()[3]);
+ EXPECT_EQ("--args", cmd_line.argv()[4]);
+ EXPECT_EQ("abc", cmd_line.argv()[5]);
+ EXPECT_EQ("def", cmd_line.argv()[6]);
+
+ // If args contain "${url}", only put the URL together with the other
+ // args. e.g.:
+ // open -a Safari --args abc http://example.com/ def
+ params = UTF8VectorToListValue({"abc", "${url}", "def"});
+ driver.SetBrowserParameters(¶ms);
+ cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ EXPECT_EQ(7u, cmd_line.argv().size());
+ EXPECT_EQ("open", cmd_line.argv()[0]);
+ EXPECT_EQ("-a", cmd_line.argv()[1]);
+ EXPECT_EQ("Safari", cmd_line.argv()[2]);
+ EXPECT_EQ("--args", cmd_line.argv()[3]);
+ EXPECT_EQ("abc", cmd_line.argv()[4]);
+ EXPECT_EQ("http://example.com/", cmd_line.argv()[5]);
+ EXPECT_EQ("def", cmd_line.argv()[6]);
+}
+#endif // defined(OS_MACOSX)
+
+#if defined(OS_POSIX)
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLineExpandsTilde) {
+ setenv("HOME", "/home/foobar", true);
+
+ AlternativeBrowserDriverImpl driver;
+ driver.SetBrowserPath("/usr/bin/true");
+ auto params = UTF8VectorToListValue({"~/file.txt", "/tmp/backup.txt~"});
+ driver.SetBrowserParameters(¶ms);
+ auto cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ const base::CommandLine::StringVector& argv = cmd_line.argv();
+ EXPECT_EQ(4u, argv.size());
+ EXPECT_EQ(UTF8ToNative("/usr/bin/true"), argv[0]);
+ EXPECT_EQ(UTF8ToNative("/home/foobar/file.txt"), argv[1]);
+ EXPECT_EQ(UTF8ToNative("/tmp/backup.txt~"), argv[2]);
+ EXPECT_EQ(UTF8ToNative("http://example.com/"), argv[3]);
+}
+
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLineExpandsEnvVars) {
setenv("A", "AAA", true);
setenv("B", "BBB", true);
setenv("CC", "CCC", true);
setenv("D", "DDD", true);
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, {"$A", "${B}", "before_${CC}_between_${D}_after", "$NONEXISTENT"},
- GURL("https://example.com/"));
+
+ AlternativeBrowserDriverImpl driver;
+ driver.SetBrowserPath("/usr/bin/true");
+ auto params = UTF8VectorToListValue(
+ {"$A", "${B}", "before_${CC}_between_${D}_after", "$NONEXISTENT"});
+ driver.SetBrowserParameters(¶ms);
+ auto cmd_line = driver.CreateCommandLine(GURL("http://example.com/"));
+ const base::CommandLine::StringVector& argv = cmd_line.argv();
EXPECT_EQ(6u, argv.size());
- EXPECT_EQ("some_script", argv[0]);
- EXPECT_EQ("AAA", argv[1]);
- EXPECT_EQ("BBB", argv[2]);
+ EXPECT_EQ(UTF8ToNative("/usr/bin/true"), argv[0]);
+ EXPECT_EQ(UTF8ToNative("AAA"), argv[1]);
+ EXPECT_EQ(UTF8ToNative("BBB"), argv[2]);
EXPECT_EQ("before_CCC_between_DDD_after", argv[3]);
EXPECT_EQ("", argv[4]);
- EXPECT_EQ("https://example.com/", argv[5]);
+ EXPECT_EQ(UTF8ToNative("http://example.com/"), argv[5]);
}
-TEST_F(AlternativeBrowserDriverTest,
- AppendCommandLineArgumentDoesntExpandUrlContent) {
- std::vector<std::string> argv = {"something"};
+TEST_F(AlternativeBrowserDriverTest, CreateCommandLineDoesntExpandUrlContent) {
setenv("A", "AAA", true);
setenv("B", "BBB", true);
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, {}, GURL("https://evil.com/$A${B}"));
- AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- &argv, {"${url}"}, GURL("https://evil.com/$A${B}"));
- EXPECT_EQ(3u, argv.size());
- EXPECT_EQ("something", argv[0]);
- EXPECT_EQ("https://evil.com/$A$%7BB%7D", argv[1]);
- EXPECT_EQ("https://evil.com/$A$%7BB%7D", argv[2]);
+
+ AlternativeBrowserDriverImpl driver;
+ driver.SetBrowserPath("/usr/bin/true");
+ auto cmd_line = driver.CreateCommandLine(GURL("http://evil.com/$A${B}"));
+ EXPECT_EQ(2u, cmd_line.argv().size());
+ EXPECT_EQ(UTF8ToNative("/usr/bin/true"), cmd_line.argv()[0]);
+ EXPECT_EQ("http://evil.com/$A$%7BB%7D", cmd_line.argv()[1]);
+
+ auto params = UTF8VectorToListValue({"${url}"});
+ driver.SetBrowserParameters(¶ms);
+ cmd_line = driver.CreateCommandLine(GURL("http://evil.com/$A${B}"));
+ EXPECT_EQ(2u, cmd_line.argv().size());
+ EXPECT_EQ(UTF8ToNative("/usr/bin/true"), cmd_line.argv()[0]);
+ EXPECT_EQ("http://evil.com/$A$%7BB%7D", cmd_line.argv()[1]);
}
#endif // defined(OS_POSIX)
diff --git a/chrome/browser/browser_switcher/alternative_browser_driver_win.cc b/chrome/browser/browser_switcher/alternative_browser_driver_win.cc
index 2f82dc8..707385f 100644
--- a/chrome/browser/browser_switcher/alternative_browser_driver_win.cc
+++ b/chrome/browser/browser_switcher/alternative_browser_driver_win.cc
@@ -4,7 +4,6 @@
#include "chrome/browser/browser_switcher/alternative_browser_driver.h"
-#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/process/launch.h"
#include "base/strings/utf_string_conversions.h"
@@ -113,6 +112,23 @@
*arg = out.get();
}
+void AppendCommandLineArguments(base::CommandLine* cmd_line,
+ const std::vector<std::wstring>& raw_args,
+ const GURL& url) {
+ std::wstring url_spec = base::UTF8ToWide(url.spec());
+ std::vector<std::wstring> command_line;
+ bool contains_url = false;
+ for (const auto& arg : raw_args) {
+ std::wstring expanded_arg = arg;
+ ExpandEnvironmentVariables(&expanded_arg);
+ if (ExpandUrlVarName(&expanded_arg, url_spec))
+ contains_url = true;
+ cmd_line->AppendArgNative(expanded_arg);
+ }
+ if (!contains_url)
+ cmd_line->AppendArgNative(url_spec);
+}
+
} // namespace
AlternativeBrowserDriver::~AlternativeBrowserDriver() {}
@@ -217,17 +233,8 @@
CHECK(url.SchemeIsHTTPOrHTTPS() || url.SchemeIsFile());
- // We know that there will be at most browser_params_.size() arguments, plus
- // one for the executable, and possibly one for the URL.
- const int max_num_args = browser_params_.size() + 2;
- std::vector<std::wstring> argv;
- argv.reserve(max_num_args);
- std::wstring path = browser_path_;
- ExpandEnvironmentVariables(&path);
- argv.push_back(path);
- AppendCommandLineArguments(&argv, browser_params_, url);
+ auto cmd_line = CreateCommandLine(url);
- base::CommandLine cmd_line = base::CommandLine(argv);
base::LaunchOptions options;
if (!base::LaunchProcess(cmd_line, options).IsValid()) {
LOG(ERROR) << "Could not start the alternative browser! Error: "
@@ -237,23 +244,13 @@
return true;
}
-void AlternativeBrowserDriverImpl::AppendCommandLineArguments(
- std::vector<std::wstring>* argv,
- const std::vector<std::wstring>& raw_args,
+base::CommandLine AlternativeBrowserDriverImpl::CreateCommandLine(
const GURL& url) {
- // TODO(crbug/882520): Do environment variable expansion.
- std::wstring url_spec = base::UTF8ToWide(url.spec());
- std::vector<std::wstring> command_line;
- bool contains_url = false;
- for (const auto& arg : raw_args) {
- std::wstring expanded_arg = arg;
- ExpandEnvironmentVariables(&expanded_arg);
- if (ExpandUrlVarName(&expanded_arg, url_spec))
- contains_url = true;
- argv->push_back(expanded_arg);
- }
- if (!contains_url)
- argv->push_back(url_spec);
+ std::wstring path = browser_path_;
+ ExpandEnvironmentVariables(&path);
+ base::CommandLine cmd_line(std::vector<std::wstring>{path});
+ AppendCommandLineArguments(&cmd_line, browser_params_, url);
+ return cmd_line;
}
} // namespace browser_switcher
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 6cd351f..0e67cdb 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -412,11 +412,8 @@
#include "chrome/browser/webshare/share_service_impl.h"
#endif
-#if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS))
-#include "chrome/browser/browser_switcher/browser_switcher_navigation_throttle.h"
-#endif
-
-#if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+ (defined(OS_LINUX) && !defined(OS_CHROMEOS))
#include "chrome/browser/browser_switcher/browser_switcher_navigation_throttle.h"
#endif
@@ -4173,7 +4170,8 @@
if (previews_lite_page_throttle)
throttles.push_back(std::move(previews_lite_page_throttle));
-#if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+ (defined(OS_LINUX) && !defined(OS_CHROMEOS))
std::unique_ptr<content::NavigationThrottle> browser_switcher_throttle =
browser_switcher::BrowserSwitcherNavigationThrottle ::
MaybeCreateThrottleFor(handle);
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index d9fb637f..be3fc5c8 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -311,7 +311,8 @@
#include "chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_util.h"
#endif
-#if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+ (defined(OS_LINUX) && !defined(OS_CHROMEOS))
#include "chrome/browser/browser_switcher/browser_switcher_prefs.h"
#endif
@@ -743,7 +744,8 @@
safe_browsing::PostCleanupSettingsResetter::RegisterProfilePrefs(registry);
#endif
-#if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+ (defined(OS_LINUX) && !defined(OS_CHROMEOS))
browser_switcher::prefs::RegisterProfilePrefs(registry);
#endif
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 15668de..b204fcf 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -2032,10 +2032,6 @@
"../browser/renderer_context_menu/spelling_options_submenu_observer_browsertest.cc",
]
}
- if (is_win || (is_linux && !is_chromeos)) {
- sources +=
- [ "../browser/browser_switcher/ieem_sitelist_parser_browsertest.cc" ]
- }
if (!is_posix || is_chromeos) {
sources -= [ "../common/time_format_browsertest.cc" ]
}
@@ -2043,6 +2039,7 @@
if (is_mac || is_win || (is_linux && !is_chromeos)) {
sources += [
# Tests for non mobile and non CrOS (includes Linux, Win, Mac).
+ "../browser/browser_switcher/ieem_sitelist_parser_browsertest.cc",
"../browser/extensions/api/image_writer_private/image_writer_utility_client_browsertest.cc",
"../browser/profiles/profile_statistics_browsertest.cc",
]
@@ -3346,13 +3343,8 @@
}
if (is_win || (is_linux && !is_chromeos)) {
- sources += [
- "../browser/browser_switcher/alternative_browser_driver_unittest.cc",
- "../browser/browser_switcher/alternative_browser_launcher_unittest.cc",
- "../browser/browser_switcher/browser_switcher_navigation_throttle_unittest.cc",
- "../browser/browser_switcher/browser_switcher_sitelist_unittest.cc",
- "../browser/ui/views/quit_instruction_bubble_controller_unittest.cc",
- ]
+ sources +=
+ [ "../browser/ui/views/quit_instruction_bubble_controller_unittest.cc" ]
}
if (enable_native_notifications) {
@@ -4440,7 +4432,13 @@
}
if (is_win || is_mac || (is_linux && !is_chromeos)) {
- sources += [ "../browser/password_manager/password_store_signin_notifier_impl_unittest.cc" ]
+ sources += [
+ "../browser/browser_switcher/alternative_browser_driver_unittest.cc",
+ "../browser/browser_switcher/alternative_browser_launcher_unittest.cc",
+ "../browser/browser_switcher/browser_switcher_navigation_throttle_unittest.cc",
+ "../browser/browser_switcher/browser_switcher_sitelist_unittest.cc",
+ "../browser/password_manager/password_store_signin_notifier_impl_unittest.cc",
+ ]
}
if (enable_widevine && is_win) {
