Provide a separate app bundle for subprocesses like the renderer on the Mac.

Remove LSUIElement and related hacks from the browser's app bundle.

BUG=8044
TEST=Observe one Chromium and one or more Chromium Helper processes in
     Activity Monitor
Review URL: http://codereview.chromium.org/164177

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

10 files changed, 232 insertions, 69 deletions

diff --git a/chrome/app/app-Info.plist b/chrome/app/app-Info.plist
index 51594c3..3a8e686 100644
--- a/chrome/app/app-Info.plist
+++ b/chrome/app/app-Info.plist
@@ -190,8 +190,6 @@
 	<true/>
 	<key>LSMinimumSystemVersion</key>
 	<string>10.5.0</string>
-	<key>LSUIElement</key>
-	<string>1</string>
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
diff --git a/chrome/app/helper-Info.plist b/chrome/app/helper-Info.plist
new file mode 100644
index 0000000..9aeacff
--- /dev/null
+++ b/chrome/app/helper-Info.plist
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>CFBundleDevelopmentRegion</key>
+	<string>English</string>
+	<key>CFBundleExecutable</key>
+	<string>${EXECUTABLE_NAME}</string>
+	<key>CFBundleIdentifier</key>
+	<string>${CHROMIUM_BUNDLE_ID}.helper</string>
+	<key>CFBundleInfoDictionaryVersion</key>
+	<string>6.0</string>
+	<key>CFBundleName</key>
+	<string>${CHROMIUM_SHORT_NAME} Helper</string>
+	<key>CFBundlePackageType</key>
+	<string>APPL</string>
+	<key>CFBundleSignature</key>
+	<string>????</string>
+	<key>CFBundleVersion</key>
+	<string>0.1</string>
+	<key>LSFileQuarantineEnabled</key>
+	<true/>
+	<key>LSMinimumSystemVersion</key>
+	<string>10.5.0</string>
+	<key>LSUIElement</key>
+	<string>1</string>
+</dict>
+</plist>
diff --git a/chrome/app/make_mac_app_symlinks b/chrome/app/make_mac_app_symlinks
new file mode 100755
index 0000000..5f3c040
--- /dev/null
+++ b/chrome/app/make_mac_app_symlinks
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+# Copyright (c) 2009 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# This script is intended to run from a "postbuild" action from within Xcode.
+# It sets up symbolic links for an app bundle's Resources and Frameworks
+# directories.  This is intended to be used for app bundles that live within
+# the Resources directory of a larger app bundle, when the sub-app's
+# Resources and Frameworks directories should point to the enclosing app's.
+
+set -e
+
+CONTENTS_PATH="${BUILT_PRODUCTS_DIR}/${WRAPPER_NAME}/Contents"
+
+ln -fhs ../.. "${CONTENTS_PATH}/Resources"
+ln -fhs ../../../Frameworks "${CONTENTS_PATH}/Frameworks"
diff --git a/chrome/browser/plugin_process_host.cc b/chrome/browser/plugin_process_host.cc
index 3c5f169..9a81b94 100644
--- a/chrome/browser/plugin_process_host.cc
+++ b/chrome/browser/plugin_process_host.cc
@@ -335,11 +335,10 @@ bool PluginProcessHost::Init(const WebPluginInfo& info,
 
   // build command line for plugin, we have to quote the plugin's path to deal
   // with spaces.
-  const CommandLine& browser_command_line = *CommandLine::ForCurrentProcess();
-  std::wstring exe_path =
-      browser_command_line.GetSwitchValue(switches::kBrowserSubprocessPath);
-  if (exe_path.empty() && !PathService::Get(base::FILE_EXE, &exe_path))
+  std::wstring exe_path = GetChildPath();
+  if (exe_path.empty()) {
     return false;
+  }
 
   CommandLine cmd_line(exe_path);
   if (logging::DialogsAreSuppressed())
@@ -368,6 +367,8 @@ bool PluginProcessHost::Init(const WebPluginInfo& info,
     switches::kEnableStatsTable,
   };
 
+  const CommandLine& browser_command_line = *CommandLine::ForCurrentProcess();
+
   for (size_t i = 0; i < arraysize(switch_names); ++i) {
     if (browser_command_line.HasSwitch(switch_names[i])) {
       cmd_line.AppendSwitchWithValue(
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 0fc5e05..07b7c7e 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -47,6 +47,7 @@
 #include "chrome/browser/visitedlink_master.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/child_process_info.h"
+#include "chrome/common/child_process_host.h"
 #include "chrome/common/chrome_descriptors.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/notification_service.h"
@@ -201,16 +202,6 @@ class VisitedLinkUpdater {
   VisitedLinkCommon::Fingerprints pending_;
 };
 
-
-// Used for a View_ID where the renderer has not been attached yet
-const int32 kInvalidViewID = -1;
-
-// Get the path to the renderer executable, which is the same as the
-// current executable.
-bool GetRendererPath(std::wstring* cmd_line) {
-  return PathService::Get(base::FILE_EXE, cmd_line);
-}
-
 BrowserRenderProcessHost::BrowserRenderProcessHost(Profile* profile)
     : RenderProcessHost(profile),
       visible_widgets_(0),
@@ -307,15 +298,12 @@ bool BrowserRenderProcessHost::Init() {
 
   // Build command line for renderer, we have to quote the executable name to
   // deal with spaces.
-  std::wstring renderer_path =
-      browser_command_line.GetSwitchValue(switches::kBrowserSubprocessPath);
+  std::wstring renderer_path = ChildProcessHost::GetChildPath();
   if (renderer_path.empty()) {
-    if (!GetRendererPath(&renderer_path)) {
-      // Need to reset the channel we created above or others might think the
-      // connection is live.
-      channel_.reset();
-      return false;
-    }
+    // Need to reset the channel we created above or others might think the
+    // connection is live.
+    channel_.reset();
+    return false;
   }
   CommandLine cmd_line(renderer_path);
   if (logging::DialogsAreSuppressed())
diff --git a/chrome/browser/utility_process_host.cc b/chrome/browser/utility_process_host.cc
index 149d296..c06749d 100644
--- a/chrome/browser/utility_process_host.cc
+++ b/chrome/browser/utility_process_host.cc
@@ -52,12 +52,7 @@ bool UtilityProcessHost::StartWebResourceUnpacker(const std::string& data) {
 }
 
 std::wstring UtilityProcessHost::GetUtilityProcessCmd() {
-  std::wstring exe_path = CommandLine::ForCurrentProcess()->GetSwitchValue(
-      switches::kBrowserSubprocessPath);
-  if (exe_path.empty()) {
-    PathService::Get(base::FILE_EXE, &exe_path);
-  }
-  return exe_path;
+  return GetChildPath();
 }
 
 bool UtilityProcessHost::StartProcess(const FilePath& exposed_dir) {
diff --git a/chrome/browser/worker_host/worker_process_host.cc b/chrome/browser/worker_host/worker_process_host.cc
index e1e7829..16cbada 100644
--- a/chrome/browser/worker_host/worker_process_host.cc
+++ b/chrome/browser/worker_host/worker_process_host.cc
@@ -88,8 +88,8 @@ bool WorkerProcessHost::Init() {
   if (!CreateChannel())
     return false;
 
-  std::wstring exe_path;
-  if (!PathService::Get(base::FILE_EXE, &exe_path))
+  std::wstring exe_path = GetChildPath();
+  if (exe_path.empty())
     return false;
 
   CommandLine cmd_line(exe_path);
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index f1d5200..122c1e5 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -65,7 +65,18 @@
       '../views/controls/label_unittest.cc',
       '../views/controls/table/table_view_unittest.cc',
       '../views/grid_layout_unittest.cc',
-    ]
+    ],
+    'conditions': [
+      ['OS=="mac"', {
+        'conditions': [
+          ['branding=="Chrome"', {
+            'bundle_id': 'com.google.Chrome',
+          }, {  # else: branding!="Chrome"
+            'bundle_id': 'org.chromium.Chromium',
+          }],  # branding
+        ],  # conditions
+      }],  # OS=="mac"
+    ],  # conditions
   },
   'includes': [
     '../build/common.gypi',
@@ -3009,9 +3020,6 @@
           'conditions': [
             ['branding=="Chrome"', {
               'mac_bundle_resources': ['app/theme/google_chrome/app.icns'],
-              'variables': {
-                'bundle_id': 'com.google.Chrome',
-              },
               'copies': [
                 {
                   'destination': '<(PRODUCT_DIR)/<(mac_product_name).app/Contents/MacOS/',
@@ -3023,9 +3031,6 @@
               ],
             }, {  # else: 'branding!="Chrome"
               'mac_bundle_resources': ['app/theme/chromium/app.icns'],
-              'variables': {
-                'bundle_id': 'org.chromium.Chromium',
-              },
               'copies': [
                 {
                   'destination': '<(PRODUCT_DIR)/<(mac_product_name).app/Contents/MacOS/',
@@ -3078,8 +3083,12 @@
             'CHROMIUM_BUNDLE_ID': '<(bundle_id)',
             'CHROMIUM_SHORT_NAME': '<(branding)',
           },
-          # Bring in pdfsqueeze and run it on all pdfs
+          'mac_bundle_resources': [
+            '<(PRODUCT_DIR)/<(mac_product_name) Helper.app',
+          ],
           'dependencies': [
+            'helper_app',
+            # Bring in pdfsqueeze and run it on all pdfs
             '../build/temp_gyp/pdfsqueeze.gyp:pdfsqueeze',
             'interpose_dependency_shim',
           ],
@@ -3102,6 +3111,23 @@
               'destination': '<(PRODUCT_DIR)/<(mac_product_name).app/Contents/Frameworks',
               'files': ['<(PRODUCT_DIR)/<(mac_product_name) Framework.framework'],
             },
+            {
+              # Copy web inspector resources to the Contents/Resources folder.
+              'destination': '<(PRODUCT_DIR)/<(mac_product_name).app/Contents/Resources',
+              'files': ['<(PRODUCT_DIR)/resources/inspector/'],
+            },
+          ],
+          'postbuilds': [
+            {
+              # Modify the Info.plist as needed.  The script explains why this
+              # is needed.  This is also done in the helper_app target.
+              'postbuild_name': 'Tweak Info.plist',
+              'action': ['<(DEPTH)/build/mac/tweak_app_infoplist',
+                         '-b<(mac_breakpad)',
+                         '-k<(mac_keystone)',
+                         '-s1',  # Include Subversion information
+                         '<(branding)'],
+            },
           ],
         }, { # else: OS != "mac"
           'conditions': [
@@ -3119,34 +3145,6 @@
             }],
           ],
         }],
-        ['OS=="mac"', {
-          'actions': [
-            {
-              # Mac adds an action to modify the Info.plist to meet our needs
-              # (see the script for why this is done).
-              'action_name': 'tweak_app_infoplist',
-              # We don't list any inputs or outputs because we always want
-              # the script to run.  Why?  Because it does thinks like record
-              # the svn revision into the info.plist, so there is no file to
-              # depend on that will change when ever that changes.
-              'inputs': [],
-              'outputs': [],
-              'action': ['<(DEPTH)/build/mac/tweak_app_infoplist',
-                         '-b<(mac_breakpad)',
-                         '-k<(mac_keystone)',
-                         '<(branding)'],
-            },
-          ],
-        }],
-        ['OS=="mac"', {
-          # Copy web inspector resources to the Contents/Resources folder.
-          'copies': [
-            {
-              'destination': '<(PRODUCT_DIR)/<(mac_product_name).app/Contents/Resources',
-              'files': ['<(PRODUCT_DIR)/resources/inspector/'],
-            },
-          ],
-        }],
         ['OS=="linux"', {
           'conditions': [
             ['branding=="Chrome"', {
@@ -4396,7 +4394,7 @@
               # app bundle, the only dependent of this target.
               # TODO(mark): Fix.
               'mac_bundle_resources/': [
-                ['exclude', ''],
+                ['exclude', '.*'],
               ],
               'direct_dependent_settings': {
                 'mac_bundle_resources': [
@@ -4475,6 +4473,71 @@
     ['OS=="mac"',
       { 'targets': [
         {
+          'target_name': 'helper_app',
+          'type': 'executable',
+          'product_name': '<(mac_product_name) Helper',
+          'mac_bundle': 1,
+          'dependencies': [
+            'chrome_dll',
+          ],
+          'sources': [
+            # chrome_exe_main.mm's main() is the entry point for the "chrome"
+            # (browser app) target.  All it does is jump to chrome_dll's
+            # ChromeMain.  This is appropriate for helper processes too,
+            # because the logic to discriminate between process types at run
+            # time is actually directed by the --type command line argument
+            # processed by ChromeMain.  Sharing chrome_exe_main.mm with the
+            # browser app will suffice for now.
+            'app/chrome_exe_main.mm',
+            'app/helper-Info.plist',
+          ],
+          # TODO(mark): Come up with a fancier way to do this.  It should only
+          # be necessary to list app-Info.plist once, not the three times it is
+          # listed here.
+          'mac_bundle_resources!': [
+            'app/helper-Info.plist',
+          ],
+          # TODO(mark): For now, don't put any resources into this app.  Its
+          # resources directory will be a symbolic link to the browser app's
+          # resources directory.
+          'mac_bundle_resources/': [
+            ['exclude', '.*'],
+          ],
+          'xcode_settings': {
+            'CHROMIUM_BUNDLE_ID': '<(bundle_id)',
+            'CHROMIUM_SHORT_NAME': '<(branding)',
+            'INFOPLIST_FILE': 'app/helper-Info.plist',
+          },
+          'postbuilds': [
+            {
+              'postbuild_name': 'Make Symbolic Links',
+              'action': ['app/make_mac_app_symlinks'],
+            },
+            {
+              # Modify the Info.plist as needed.  The script explains why this
+              # is needed.  This is also done in the chrome target.  In
+              # this case, -k0 is always used because Keystone never runs
+              # within the helper app.  -s0 is used to avoid placing Subversion
+              # data in the helper app's Info.plist.  It will be present in
+              # the main app's Info.plist, which is sufficient.
+              'postbuild_name': 'Tweak Info.plist',
+              'action': ['<(DEPTH)/build/mac/tweak_app_infoplist',
+                         '-b<(mac_breakpad)',
+                         '-k0',
+                         '-s0',
+                         '<(branding)'],
+            },
+          ],
+          'conditions': [
+            ['mac_breakpad==1', {
+              'variables': {
+                # A real .dSYM is needed for dump_syms to operate on.
+                'mac_real_dsym': 1,
+              },
+            }],
+          ],
+        },
+        {
           # Convenience target to build a disk image.
           'target_name': 'build_app_dmg',
           # Don't place this in the 'all' list; most won't want it.
diff --git a/chrome/common/child_process_host.cc b/chrome/common/child_process_host.cc
index b7cab7f..ca0f972 100644
--- a/chrome/common/child_process_host.cc
+++ b/chrome/common/child_process_host.cc
@@ -4,13 +4,17 @@
 
 #include "chrome/common/child_process_host.h"
 
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/file_path.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/path_service.h"
 #include "base/process_util.h"
 #include "base/singleton.h"
 #include "base/waitable_event.h"
 #include "chrome/browser/chrome_thread.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
 #include "chrome/common/plugin_messages.h"
@@ -65,6 +69,67 @@ ChildProcessHost::~ChildProcessHost() {
     ProcessWatcher::EnsureProcessTerminated(handle());
 }
 
+// static
+std::wstring ChildProcessHost::GetChildPath() {
+  std::wstring child_path = CommandLine::ForCurrentProcess()->GetSwitchValue(
+      switches::kBrowserSubprocessPath);
+  if (!child_path.empty())
+    return child_path;
+
+#if !defined(OS_MACOSX)
+  // On most platforms, the child executable is the same as the current
+  // executable.
+  PathService::Get(base::FILE_EXE, &child_path);
+  return child_path;
+#else
+  // On the Mac, the child executable lives at a predefined location within
+  // the current app bundle.
+
+  FilePath path;
+  if (!PathService::Get(base::FILE_EXE, &path))
+    return child_path;
+
+  // Figure out the current executable name.  In a browser, this will be
+  // "Chromium" or "Google Chrome".  The child name will be the browser
+  // executable name with " Helper" appended.  The child app bundle name will
+  // be that name with ".app" appended.
+  FilePath::StringType child_exe_name = path.BaseName().value();
+  const FilePath::StringType child_suffix = FILE_PATH_LITERAL(" Helper");
+
+  if (child_exe_name.size() > child_suffix.size()) {
+    size_t test_suffix_pos = child_exe_name.size() - child_suffix.size();
+    const FilePath::CharType* test_suffix =
+        child_exe_name.c_str() + test_suffix_pos;
+    if (strcmp(test_suffix, child_suffix.c_str()) == 0) {
+      // FILE_EXE already ends with the child suffix and therefore already
+      // refers to the child process path.  Just return it.
+      return path.ToWStringHack();
+    }
+  }
+
+  child_exe_name.append(child_suffix);
+  FilePath::StringType child_app_name = child_exe_name;
+  child_app_name.append(FILE_PATH_LITERAL(".app"));
+  // The renderer app bundle lives in the browser app bundle's Resources
+  // directory.  Take off the executable name.
+  path = path.DirName();
+
+  // Take off the MacOS component, after verifying that's what's there.
+  FilePath::StringType macos = path.BaseName().value();
+  DCHECK_EQ(macos, FILE_PATH_LITERAL("MacOS"));
+  path = path.DirName();
+
+  // Append the components to get to the sub-app bundle's executable.
+  path = path.Append(FILE_PATH_LITERAL("Resources"));
+  path = path.Append(child_app_name);
+  path = path.Append(FILE_PATH_LITERAL("Contents"));
+  path = path.Append(FILE_PATH_LITERAL("MacOS"));
+  path = path.Append(child_exe_name);
+
+  return path.ToWStringHack();
+#endif  // OS_MACOSX
+}
+
 bool ChildProcessHost::CreateChannel() {
   channel_id_ = GenerateRandomChannelID(this);
   channel_.reset(new IPC::Channel(
diff --git a/chrome/common/child_process_host.h b/chrome/common/child_process_host.h
index 926bdd2..95b97b6 100644
--- a/chrome/common/child_process_host.h
+++ b/chrome/common/child_process_host.h
@@ -22,6 +22,13 @@ class ChildProcessHost : public ResourceDispatcherHost::Receiver,
  public:
   virtual ~ChildProcessHost();
 
+  // Returns the pathname to be used for a child process.  If a subprocess
+  // pathname was specified on the command line, that will be used.  Otherwise,
+  // the default child process pathname will be returned.  On most platforms,
+  // this will be the same as the currently-executing process.  On failure,
+  // returns an empty wstring.
+  static std::wstring GetChildPath();
+
   // ResourceDispatcherHost::Receiver implementation:
   virtual bool Send(IPC::Message* msg);