[Mac] In browser tests, do not use a bundled child process executable.

Previously, browser test binaries (e.g. browser_tests, interactive_ui_tests,
etc.) set up paths so that child processes would use Chromium Helper.app, which
matches the production process launching path. However this prevents the child
process from linking in test-only code, because it is a distinct binary from
the main test executable that acts as the browser process.

This change makes it so that the child process binary is the same as the main
test binary, which matches how other platforms behave.

Bug: 877992
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ic04f0d47726be89d953ea227b8a104dc7bb88c0f
Reviewed-on: https://chromium-review.googlesource.com/1193969
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587705}

chrome/test/BUILD.gn

@@ -1909,27 +1909,10 @@ test("browser_tests") {
     }
 
     if (is_mac) {
-      # Other platforms only need .pak files, and can build this target
-      # standalone much faster.
-      deps += [
+      deps += [ "//third_party/ocmock" ]
+      data_deps += [
         "//chrome",
-        "//chrome/common:app_mode_app_support",
-
-        # TODO(GYP) Mac: GYP has this dependency. Uncommenting this line
-        # generates duplicate symbols between
-        #   obj/components/crash/content/app/breakpad_stubs/crash_reporter_client.o
-        #   obj/components/crash/content/app/lib/crash_reporter_client.o
-        # It's not clear how this is supposed to work. The intent seems to be
-        # to not link breakpad in the tests. The dependency on .../app:lib
-        # seems to come from //chrome/app:test_support. That reference maybe
-        # should be a dependency on the stubs instead because it could be all
-        # tests might want them. Or it could be that we need to make a new
-        # "headers" target to make GN check happy, and then force each
-        # executable to link the correct implementation. Somebody with a
-        # higher-level understanding of Mac crash reporting needs to think
-        # about this.
-        # "//components/crash/content/app:breakpad_stubs",
-        "//third_party/ocmock",
+        "//chrome:chrome_framework",
       ]
       sources += [
         "../browser/policy/cloud/machine_level_user_cloud_policy_browsertest_mac_util.h",
@@ -1968,11 +1951,6 @@ test("browser_tests") {
         ]
       }
 
-      data_deps += [
-        "//chrome",
-        "//chrome:chrome_framework",
-      ]
-
       # The browser window can be views or Cocoa on Mac, but this is chosen at
       # runtime. This block captures tests that only run with a Cocoa browser.
       # Note some tests are for obsolete Cocoa _secondary_ UI and can be removed

chrome/test/base/chrome_test_launcher.cc

@@ -7,12 +7,14 @@
 #include <memory>
 #include <utility>
 
+#include "base/base_paths.h"
 #include "base/command_line.h"
 #include "base/debug/leak_annotations.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/path_service.h"
 #include "base/process/process_metrics.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
@@ -34,6 +36,7 @@
 #include "ui/base/test/ui_controls.h"
 
 #if defined(OS_MACOSX)
+#include "base/mac/bundle_locations.h"
 #include "chrome/browser/chrome_browser_application_mac.h"
 #endif  // defined(OS_MACOSX)
 
@@ -130,7 +133,13 @@ int LaunchChromeTests(size_t parallel_jobs,
                       int argc,
                       char** argv) {
 #if defined(OS_MACOSX)
-  chrome_browser_application_mac::RegisterBrowserCrApp();
+  // Set up the path to the framework so resources can be loaded. This is also
+  // performed in ChromeTestSuite, but in browser tests that only affects the
+  // browser process. Child processes need access to the Framework bundle too.
+  base::FilePath path;
+  CHECK(base::PathService::Get(base::DIR_EXE, &path));
+  path = path.Append(chrome::kFrameworkName);
+  base::mac::SetOverrideFrameworkBundlePath(path);
 #endif
 
 #if defined(OS_WIN)

chrome/test/base/in_process_browser_test.cc

@@ -58,6 +58,7 @@
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/common/content_paths.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_launcher.h"
@@ -127,16 +128,19 @@ InProcessBrowserTest::InProcessBrowserTest()
 #if defined(OS_MACOSX)
   base::mac::SetOverrideAmIBundled(true);
 
-  // TODO(phajdan.jr): Make browser_tests self-contained on Mac, remove this.
-  // Before we run the browser, we have to hack the path to the exe to match
-  // what it would be if Chrome was running, because it is used to fork renderer
-  // processes, on Linux at least (failure to do so will cause a browser_test to
-  // be run instead of a renderer).
-  base::FilePath chrome_path;
-  CHECK(base::PathService::Get(base::FILE_EXE, &chrome_path));
-  chrome_path = chrome_path.DirName();
-  chrome_path = chrome_path.Append(chrome::kBrowserProcessExecutablePath);
+  base::FilePath file_exe;
+  CHECK(base::PathService::Get(base::FILE_EXE, &file_exe));
+
+  // Override the path to the running executable to make it look like it is
+  // the browser running as the bundled application.
+  base::FilePath chrome_path =
+      file_exe.DirName().Append(chrome::kBrowserProcessExecutablePath);
   CHECK(base::PathService::Override(base::FILE_EXE, chrome_path));
+
+  // Then override the path to the child process binaries to point back at
+  // the current test executable, otherwise the FILE_EXE overridden above would
+  // be used to launch children.
+  CHECK(base::PathService::Override(content::CHILD_PROCESS_EXE, file_exe));
 #endif  // defined(OS_MACOSX)
 
   CreateTestServer(base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
@@ -265,22 +269,6 @@ void InProcessBrowserTest::SetUpDefaultCommandLine(
   // not affect the results.
   command_line->AppendSwitchASCII(switches::kForceColorProfile, "srgb");
 
-#if defined(OS_MACOSX)
-  // Explicitly set the path of the binary used for child processes, otherwise
-  // they'll try to use browser_tests which doesn't contain ChromeMain.
-  base::FilePath subprocess_path;
-  base::PathService::Get(base::FILE_EXE, &subprocess_path);
-  // Recreate the real environment, run the helper within the app bundle.
-  subprocess_path = subprocess_path.DirName().DirName();
-  DCHECK_EQ(subprocess_path.BaseName().value(), "Contents");
-  subprocess_path =
-      subprocess_path.Append("Versions").Append(chrome::kChromeVersion);
-  subprocess_path =
-      subprocess_path.Append(chrome::kHelperProcessExecutablePath);
-  command_line->AppendSwitchPath(switches::kBrowserSubprocessPath,
-                                 subprocess_path);
-#endif
-
   // TODO(pkotwicz): Investigate if we can remove this switch.
   if (exit_when_last_browser_closes_)
     command_line->AppendSwitch(switches::kDisableZeroBrowsersOpenForTests);