Mac: Let out-of-process tests run in bundled mode for their whole lifetime.

Since out-of-process tests override the EXE path to look like the bundled app, it makes sense to override AmIBundled() as well.

This is important because the renderer process started from browser_tests runs as bundled, and if browser and renderer process don't agree on bundled-ness, the "load plugin" requests for internal plugins from the renderer have the wrong plugin path, causing the plugin load to fail.

Also add a DCHECK that makes sure that AmIBundled() doesn't flip-flop.

This makes PDFBrowserTest work on mac, so enable it.

It looks like even unit_tests uses the out-of-process test runner, so this change is a bit hairy :-/

BUG=61258,63183
TEST=all existing tests still pass, PDFBrowserTest.* passes.

Review URL: http://codereview.chromium.org/4947002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66156 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 84 insertions, 35 deletions

diff --git a/base/base_paths_mac.mm b/base/base_paths_mac.mm
index 793bece..34a3d23 100644
--- a/base/base_paths_mac.mm
+++ b/base/base_paths_mac.mm
@@ -52,7 +52,8 @@ bool PathProviderMac(int key, FilePath* result) {
     case base::DIR_APP_DATA:
       return mac_util::GetUserDirectory(NSApplicationSupportDirectory, result);
     case base::DIR_SOURCE_ROOT: {
-      if (GetNSExecutablePath(result)) {
+      // Go through PathService to catch overrides.
+      if (PathService::Get(base::FILE_EXE, result)) {
         // Start with the executable's directory.
         *result = result->DirName();
         if (mac_util::AmIBundled()) {
diff --git a/base/mac_util.h b/base/mac_util.h
index 182fcc8..d31bf82 100644
--- a/base/mac_util.h
+++ b/base/mac_util.h
@@ -50,6 +50,7 @@ bool FSRefFromPath(const std::string& path, FSRef* ref);
 
 // Returns true if the application is running from a bundle
 bool AmIBundled();
+void SetOverrideAmIBundled(bool value);
 
 // Returns true if this process is marked as a "Background only process".
 bool IsBackgroundOnlyProcess();
diff --git a/base/mac_util.mm b/base/mac_util.mm
index 4e6b105..9610d37 100644
--- a/base/mac_util.mm
+++ b/base/mac_util.mm
@@ -145,8 +145,14 @@ bool FSRefFromPath(const std::string& path, FSRef* ref) {
   return status == noErr;
 }
 
+static bool g_override_am_i_bundled = false;
+static bool g_override_am_i_bundled_value = false;
+
 // Adapted from http://developer.apple.com/carbon/tipsandtricks.html#AmIBundled
-bool AmIBundled() {
+static bool UncachedAmIBundled() {
+  if (g_override_am_i_bundled)
+    return g_override_am_i_bundled_value;
+
   ProcessSerialNumber psn = {0, kCurrentProcess};
 
   FSRef fsref;
@@ -167,6 +173,23 @@ bool AmIBundled() {
   return info.nodeFlags & kFSNodeIsDirectoryMask;
 }
 
+bool AmIBundled() {
+  // If the return value is not cached, this function will return different
+  // values depending on when it's called. This confuses some client code, see
+  // http://crbug.com/63183 .
+  static bool result = UncachedAmIBundled();
+  DCHECK_EQ(result, UncachedAmIBundled())
+      << "The return value of AmIBundled() changed. This will confuse tests. "
+      << "Call SetAmIBundled() override manually if your test binary "
+      << "delay-loads the framework.";
+  return result;
+}
+
+void SetOverrideAmIBundled(bool value) {
+  g_override_am_i_bundled = true;
+  g_override_am_i_bundled_value = value;
+}
+
 bool IsBackgroundOnlyProcess() {
   // This function really does want to examine NSBundle's idea of the main
   // bundle dictionary, and not the overriden MainAppBundle.  It needs to look
diff --git a/chrome/common/pepper_plugin_registry.cc b/chrome/common/pepper_plugin_registry.cc
index 50f50e7..f812afd 100644
--- a/chrome/common/pepper_plugin_registry.cc
+++ b/chrome/common/pepper_plugin_registry.cc
@@ -153,7 +153,7 @@ void PepperPluginRegistry::GetInternalPluginInfo(
   // RendererMain() and BrowserMain().  This seemed like the better tradeoff.
   //
   // TODO(ajwong): Think up a better way to maintain the plugin registration
-  // information. Pehraps by construction of a singly linked list of
+  // information. Perhaps by construction of a singly linked list of
   // plugin initializers that is built with static initializers?
 
 #if defined(ENABLE_REMOTING)
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index cd6669f..78781fc 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -43,14 +43,18 @@
 #include "net/test/test_server.h"
 #include "sandbox/src/dep.h"
 
-#if defined(OS_WIN)
-#include "chrome/browser/views/frame/browser_view.h"
-#endif
-
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #endif  // defined(OS_CHROMEOS)
 
+#if defined(OS_MACOSX)
+#include "base/mac_util.h"
+#endif
+
+#if defined(OS_WIN)
+#include "chrome/browser/views/frame/browser_view.h"
+#endif
+
 namespace {
 
 void InitializeBrowser(Browser* browser) {
@@ -75,12 +79,32 @@ static const char kBrowserTestType[] = "browser";
 
 InProcessBrowserTest::InProcessBrowserTest()
     : browser_(NULL),
-      test_server_(net::TestServer::TYPE_HTTP,
-                   FilePath(FILE_PATH_LITERAL("chrome/test/data"))),
       show_window_(false),
       dom_automation_enabled_(false),
       tab_closeable_state_watcher_enabled_(false),
       original_single_process_(false) {
+#if defined(OS_MACOSX)
+  mac_util::SetOverrideAmIBundled(true);
+#endif
+
+  // 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).
+  FilePath chrome_path;
+  CHECK(PathService::Get(base::FILE_EXE, &chrome_path));
+  chrome_path = chrome_path.DirName();
+#if defined(OS_WIN)
+  chrome_path = chrome_path.Append(chrome::kBrowserProcessExecutablePath);
+#elif defined(OS_POSIX)
+  chrome_path = chrome_path.Append(
+      WideToASCII(chrome::kBrowserProcessExecutablePath));
+#endif
+  CHECK(PathService::Override(base::FILE_EXE, chrome_path));
+
+  test_server_.reset(new net::TestServer(
+      net::TestServer::TYPE_HTTP,
+      FilePath(FILE_PATH_LITERAL("chrome/test/data"))));
 }
 
 InProcessBrowserTest::~InProcessBrowserTest() {
@@ -154,9 +178,6 @@ void InProcessBrowserTest::SetUp() {
   // they'll try to use browser_tests which doesn't contain ChromeMain.
   FilePath subprocess_path;
   PathService::Get(base::FILE_EXE, &subprocess_path);
-  subprocess_path = subprocess_path.DirName();
-  subprocess_path = subprocess_path.AppendASCII(WideToASCII(
-      chrome::kBrowserProcessExecutablePath));
 #if defined(OS_MACOSX)
   // Recreate the real environment, run the helper within the app bundle.
   subprocess_path = subprocess_path.DirName().DirName();
@@ -205,21 +226,6 @@ void InProcessBrowserTest::SetUp() {
 
   SetUpInProcessBrowserTestFixture();
 
-  // 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).
-  FilePath chrome_path;
-  CHECK(PathService::Get(base::FILE_EXE, &chrome_path));
-  chrome_path = chrome_path.DirName();
-#if defined(OS_WIN)
-  chrome_path = chrome_path.Append(chrome::kBrowserProcessExecutablePath);
-#elif defined(OS_POSIX)
-  chrome_path = chrome_path.Append(
-      WideToASCII(chrome::kBrowserProcessExecutablePath));
-#endif
-  CHECK(PathService::Override(base::FILE_EXE, chrome_path));
-
   BrowserMain(params);
   TearDownInProcessBrowserTestFixture();
 }
diff --git a/chrome/test/in_process_browser_test.h b/chrome/test/in_process_browser_test.h
index 67658b5..444519b 100644
--- a/chrome/test/in_process_browser_test.h
+++ b/chrome/test/in_process_browser_test.h
@@ -110,7 +110,7 @@ class InProcessBrowserTest : public testing::Test {
   virtual void CleanUpOnMainThread() {}
 
   // Returns the testing server. Guaranteed to be non-NULL.
-  net::TestServer* test_server() { return &test_server_; }
+  net::TestServer* test_server() { return test_server_.get(); }
 
   // Creates a browser with a single tab (about:blank), waits for the tab to
   // finish loading and shows the browser.
@@ -153,7 +153,7 @@ class InProcessBrowserTest : public testing::Test {
   Browser* browser_;
 
   // Testing server, started on demand.
-  net::TestServer test_server_;
+  scoped_ptr<net::TestServer> test_server_;
 
   // Whether this test requires the browser windows to be shown (interactive
   // tests for example need the windows shown).
diff --git a/chrome/test/plugin/pdf_browsertest.cc b/chrome/test/plugin/pdf_browsertest.cc
index 424fa49..df7a834 100644
--- a/chrome/test/plugin/pdf_browsertest.cc
+++ b/chrome/test/plugin/pdf_browsertest.cc
@@ -42,10 +42,7 @@ class PDFBrowserTest : public InProcessBrowserTest,
   virtual void SetUp() {
     FilePath pdf_path;
     PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_path);
-#if !defined(OS_MACOSX)
-    // http://crbug.com/61258: renderer always crashes on Mac.
     have_plugin_ = file_util::PathExists(pdf_path);
-#endif
     InProcessBrowserTest::SetUp();
   }
 
@@ -185,9 +182,16 @@ class PDFBrowserTest : public InProcessBrowserTest,
   FilePath snapshot_filename_;
 };
 
+#if defined(OS_MACOSX)
+// See http://crbug.com/63223
+#define MAYBE_Basic FLAKY_Basic
+#else
+#define MAYBE_Basic Basic
+#endif
+
 // Tests basic PDF rendering.  This can be broken depending on bad merges with
 // the vendor, so it's important that we have basic sanity checking.
-IN_PROC_BROWSER_TEST_F(PDFBrowserTest, Basic) {
+IN_PROC_BROWSER_TEST_F(PDFBrowserTest, MAYBE_Basic) {
   if (!have_plugin())
     return;
 
@@ -196,8 +200,15 @@ IN_PROC_BROWSER_TEST_F(PDFBrowserTest, Basic) {
   ASSERT_NO_FATAL_FAILURE(VerifySnapshot("pdf_browsertest.png"));
 }
 
+#if defined(OS_MACOSX)
+// See http://crbug.com/63223
+#define MAYBE_Scroll FLAKY_Scroll
+#else
+#define MAYBE_Scroll Scroll
+#endif
+
 // Tests that scrolling works.
-IN_PROC_BROWSER_TEST_F(PDFBrowserTest, Scroll) {
+IN_PROC_BROWSER_TEST_F(PDFBrowserTest, MAYBE_Scroll) {
   if (!have_plugin())
     return;
 
@@ -216,7 +227,14 @@ IN_PROC_BROWSER_TEST_F(PDFBrowserTest, Scroll) {
   ASSERT_NO_FATAL_FAILURE(VerifySnapshot("pdf_browsertest_scroll.png"));
 }
 
-IN_PROC_BROWSER_TEST_F(PDFBrowserTest, FindAndCopy) {
+#if defined(OS_MACOSX)
+// See http://crbug.com/63223
+#define MAYBE_FindAndCopy FLAKY_FindAndCopy
+#else
+#define MAYBE_FindAndCopy FindAndCopy
+#endif
+
+IN_PROC_BROWSER_TEST_F(PDFBrowserTest, MAYBE_FindAndCopy) {
   if (!have_plugin())
     return;