Remove the \servers build output directory and place Chrome Frame parts into the root output folder next to chrome.exe et al.

Also add a more helpful error message when failing to register the Chrome Frame dll.
Review URL: http://codereview.chromium.org/8013001

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

11 files changed, 130 insertions, 115 deletions

diff --git a/chrome_frame/cfproxy_proxy.cc b/chrome_frame/cfproxy_proxy.cc
index bd38337..680588e 100644
--- a/chrome_frame/cfproxy_proxy.cc
+++ b/chrome_frame/cfproxy_proxy.cc
@@ -64,7 +64,7 @@ void CFProxy::InitInIoThread(const ProxyParams& params) {
   ipc_sender_ = api_->CreateChannel(channel_id, this);
   std::wstring cmd_line = BuildCmdLine(channel_id, params.profile_path,
                                        params.extra_params);
-  if (api_->LaunchApp(cmd_line)) {
+  if (!cmd_line.empty() && api_->LaunchApp(cmd_line)) {
     CancelableTask* launch_timeout = NewRunnableMethod(this,
         &CFProxy::LaunchTimeOut);
     ipc_thread_.message_loop()->PostDelayedTask(FROM_HERE, launch_timeout,
diff --git a/chrome_frame/cfproxy_support.cc b/chrome_frame/cfproxy_support.cc
index 4e7d8070..777c854 100644
--- a/chrome_frame/cfproxy_support.cc
+++ b/chrome_frame/cfproxy_support.cc
@@ -275,35 +275,38 @@ std::string GenerateChannelId() {
 std::wstring BuildCmdLine(const std::string& channel_id,
                           const FilePath& profile_path,
                           const std::wstring& extra_args) {
-  scoped_ptr<CommandLine> command_line(
-      chrome_launcher::CreateLaunchCommandLine());
-  command_line->AppendSwitchASCII(switches::kAutomationClientChannelID,
-      channel_id);
-  // Run Chrome in Chrome Frame mode. In practice, this modifies the paths
-  // and registry keys that Chrome looks in via the BrowserDistribution
-  // mechanism.
-  command_line->AppendSwitch(switches::kChromeFrame);
-  // Chrome Frame never wants Chrome to start up with a First Run UI.
-  command_line->AppendSwitch(switches::kNoFirstRun);
-  command_line->AppendSwitch(switches::kDisablePopupBlocking);
-
-#ifndef NDEBUG
-  // Disable the "Whoa! Chrome has crashed." dialog, because that isn't very
-  // useful for Chrome Frame users.
-  command_line->AppendSwitch(switches::kNoErrorDialogs);
-#endif
-
-  // In headless mode runs like reliability test runs we want full crash dumps
-  // from chrome.
-  if (IsHeadlessMode())
-    command_line->AppendSwitch(switches::kFullMemoryCrashReport);
-
-  command_line->AppendSwitchPath(switches::kUserDataDir, profile_path);
-
-  std::wstring command_line_string(command_line->GetCommandLineString());
-  if (!extra_args.empty()) {
-    command_line_string.append(L" ");
-    command_line_string.append(extra_args);
+  std::wstring command_line_string;
+  scoped_ptr<CommandLine> command_line;
+  if (chrome_launcher::CreateLaunchCommandLine(&command_line)) {
+    command_line->AppendSwitchASCII(switches::kAutomationClientChannelID,
+        channel_id);
+    // Run Chrome in Chrome Frame mode. In practice, this modifies the paths
+    // and registry keys that Chrome looks in via the BrowserDistribution
+    // mechanism.
+    command_line->AppendSwitch(switches::kChromeFrame);
+    // Chrome Frame never wants Chrome to start up with a First Run UI.
+    command_line->AppendSwitch(switches::kNoFirstRun);
+    command_line->AppendSwitch(switches::kDisablePopupBlocking);
+
+  #ifndef NDEBUG
+    // Disable the "Whoa! Chrome has crashed." dialog, because that isn't very
+    // useful for Chrome Frame users.
+    command_line->AppendSwitch(switches::kNoErrorDialogs);
+  #endif
+
+    // In headless mode runs like reliability test runs we want full crash dumps
+    // from chrome.
+    if (IsHeadlessMode())
+      command_line->AppendSwitch(switches::kFullMemoryCrashReport);
+
+    command_line->AppendSwitchPath(switches::kUserDataDir, profile_path);
+
+    command_line_string = command_line->GetCommandLineString();
+    if (!extra_args.empty()) {
+      command_line_string.append(L" ");
+      command_line_string.append(extra_args);
+    }
   }
+
   return command_line_string;
 }
diff --git a/chrome_frame/chrome_frame.gyp b/chrome_frame/chrome_frame.gyp
index 7a8dd10..3318cb1 100644
--- a/chrome_frame/chrome_frame.gyp
+++ b/chrome_frame/chrome_frame.gyp
@@ -901,8 +901,6 @@
           },
           'msvs_settings': {
             'VCLinkerTool': {
-              'OutputFile':
-                  '$(OutDir)\\servers\\$(ProjectName).dll',
               'DelayLoadDLLs': [],
               'BaseAddress': '0x33000000',
               # Set /SUBSYSTEM:WINDOWS (for consistency).
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 792f047..e120f14 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -249,57 +249,60 @@ void AutomationProxyCacheEntry::CreateProxy(ChromeFrameLaunchParams* params,
   proxy->set_perform_version_check(params->version_check());
 
   // Launch browser
-  scoped_ptr<CommandLine> command_line(
-      chrome_launcher::CreateLaunchCommandLine());
-  command_line->AppendSwitchASCII(switches::kAutomationClientChannelID,
-                                  channel_id);
+  std::wstring command_line_string;
+  scoped_ptr<CommandLine> command_line;
+  if (chrome_launcher::CreateLaunchCommandLine(&command_line)) {
+    command_line->AppendSwitchASCII(switches::kAutomationClientChannelID,
+                                    channel_id);
 
-  // Run Chrome in Chrome Frame mode. In practice, this modifies the paths
-  // and registry keys that Chrome looks in via the BrowserDistribution
-  // mechanism.
-  command_line->AppendSwitch(switches::kChromeFrame);
+    // Run Chrome in Chrome Frame mode. In practice, this modifies the paths
+    // and registry keys that Chrome looks in via the BrowserDistribution
+    // mechanism.
+    command_line->AppendSwitch(switches::kChromeFrame);
 
-  // Chrome Frame never wants Chrome to start up with a First Run UI.
-  command_line->AppendSwitch(switches::kNoFirstRun);
+    // Chrome Frame never wants Chrome to start up with a First Run UI.
+    command_line->AppendSwitch(switches::kNoFirstRun);
 
-  command_line->AppendSwitch(switches::kDisablePopupBlocking);
+    command_line->AppendSwitch(switches::kDisablePopupBlocking);
 
-  // Disable the "Whoa! Chrome has crashed." dialog, because that isn't very
-  // useful for Chrome Frame users.
+    // Disable the "Whoa! Chrome has crashed." dialog, because that isn't very
+    // useful for Chrome Frame users.
 #ifndef NDEBUG
-  command_line->AppendSwitch(switches::kNoErrorDialogs);
+    command_line->AppendSwitch(switches::kNoErrorDialogs);
 #endif
 
-  // In headless mode runs like reliability test runs we want full crash dumps
-  // from chrome.
-  if (IsHeadlessMode())
-    command_line->AppendSwitch(switches::kFullMemoryCrashReport);
+    // In headless mode runs like reliability test runs we want full crash dumps
+    // from chrome.
+    if (IsHeadlessMode())
+      command_line->AppendSwitch(switches::kFullMemoryCrashReport);
 
-  // In accessible mode automation tests expect renderer accessibility to be
-  // enabled in chrome.
-  if (IsAccessibleMode())
-    command_line->AppendSwitch(switches::kForceRendererAccessibility);
+    // In accessible mode automation tests expect renderer accessibility to be
+    // enabled in chrome.
+    if (IsAccessibleMode())
+      command_line->AppendSwitch(switches::kForceRendererAccessibility);
 
-  DVLOG(1) << "Profile path: " << params->profile_path().value();
-  command_line->AppendSwitchPath(switches::kUserDataDir,
-                                 params->profile_path());
+    DVLOG(1) << "Profile path: " << params->profile_path().value();
+    command_line->AppendSwitchPath(switches::kUserDataDir,
+                                   params->profile_path());
 
-  // Ensure that Chrome is running the specified version of chrome.dll.
-  command_line->AppendSwitchNative(switches::kChromeVersion,
-                                   GetCurrentModuleVersion());
+    // Ensure that Chrome is running the specified version of chrome.dll.
+    command_line->AppendSwitchNative(switches::kChromeVersion,
+                                     GetCurrentModuleVersion());
 
-  if (!params->language().empty())
-    command_line->AppendSwitchNative(switches::kLang, params->language());
+    if (!params->language().empty())
+      command_line->AppendSwitchNative(switches::kLang, params->language());
 
-  std::wstring command_line_string(command_line->GetCommandLineString());
-  // If there are any extra arguments, append them to the command line.
-  if (!params->extra_arguments().empty()) {
-    command_line_string += L' ' + params->extra_arguments();
+    command_line_string = command_line->GetCommandLineString();
+    // If there are any extra arguments, append them to the command line.
+    if (!params->extra_arguments().empty()) {
+      command_line_string += L' ' + params->extra_arguments();
+    }
   }
 
   automation_server_launch_start_time_ = base::TimeTicks::Now();
 
-  if (!base::LaunchProcess(command_line_string, base::LaunchOptions(), NULL)) {
+  if (command_line_string.empty() ||
+      !base::LaunchProcess(command_line_string, base::LaunchOptions(), NULL)) {
     // We have no code for launch failure.
     launch_result_ = AUTOMATION_LAUNCH_RESULT_INVALID;
   } else {
diff --git a/chrome_frame/chrome_frame_launcher.gyp b/chrome_frame/chrome_frame_launcher.gyp
index 030f405..87be09c 100644
--- a/chrome_frame/chrome_frame_launcher.gyp
+++ b/chrome_frame/chrome_frame_launcher.gyp
@@ -82,8 +82,6 @@
       ],
       'msvs_settings': {
         'VCLinkerTool': {
-          'OutputFile':
-              '$(OutDir)\\servers\\$(ProjectName).exe',
           # Set /SUBSYSTEM:WINDOWS since this is not a command-line program.
           'SubSystem': '2',
           'AdditionalDependencies': [
diff --git a/chrome_frame/chrome_launcher_utils.cc b/chrome_frame/chrome_launcher_utils.cc
index 4338033..c77b9a8 100644
--- a/chrome_frame/chrome_launcher_utils.cc
+++ b/chrome_frame/chrome_launcher_utils.cc
@@ -8,6 +8,7 @@
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/win/windows_version.h"
@@ -19,30 +20,31 @@ namespace {
 
 const char kUpdateCommandFlag[] = "--update-cmd";
 
-CommandLine* CreateChromeLauncherCommandLine() {
+// Searches for the path to chrome_launcher.exe. Will return false if this
+// executable cannot be found, otherwise the command line will be placed in
+// |command_line|.
+bool CreateChromeLauncherCommandLine(scoped_ptr<CommandLine>* command_line) {
+  DCHECK(command_line);
+  bool success = false;
   // The launcher EXE will be in the same directory as the Chrome Frame DLL,
-  // so create a full path to it based on this assumption.  Since our unit
-  // tests also use this function, and live in the directory above, we test
-  // existence of the file and try the path that includes the /servers/
-  // directory if needed.
+  // so create a full path to it based on this assumption.
   FilePath module_path;
   if (PathService::Get(base::FILE_MODULE, &module_path)) {
     FilePath current_dir = module_path.DirName();
-    FilePath same_dir_path = current_dir.Append(
+    FilePath chrome_launcher = current_dir.Append(
         chrome_launcher::kLauncherExeBaseName);
-    if (file_util::PathExists(same_dir_path)) {
-      return new CommandLine(same_dir_path);
-    } else {
-      FilePath servers_path = current_dir.Append(L"servers").Append(
-          chrome_launcher::kLauncherExeBaseName);
-      DCHECK(file_util::PathExists(servers_path)) <<
-          "What module is this? It's not in 'servers' or main output dir.";
-      return new CommandLine(servers_path);
+    if (file_util::PathExists(chrome_launcher)) {
+      command_line->reset(new CommandLine(chrome_launcher));
+      success = true;
     }
-  } else {
-    NOTREACHED();
-    return NULL;
   }
+
+  if (!success) {
+    NOTREACHED() << "Could not find " << chrome_launcher::kLauncherExeBaseName
+                 << " in output dir.";
+  }
+
+  return success;
 }
 
 }  // namespace
@@ -51,24 +53,30 @@ namespace chrome_launcher {
 
 const wchar_t kLauncherExeBaseName[] = L"chrome_launcher.exe";
 
-CommandLine* CreateUpdateCommandLine(const std::wstring& update_command) {
-  CommandLine* command_line = CreateChromeLauncherCommandLine();
+bool CreateUpdateCommandLine(const std::wstring& update_command,
+                             scoped_ptr<CommandLine>* command_line) {
+  DCHECK(command_line);
+  bool success = false;
 
-  if (command_line) {
-    command_line->AppendArg(kUpdateCommandFlag);
-    command_line->AppendArg(WideToASCII(update_command));
+  if (CreateChromeLauncherCommandLine(command_line)) {
+    (*command_line)->AppendArg(kUpdateCommandFlag);
+    (*command_line)->AppendArg(WideToASCII(update_command));
+    success = true;
   }
 
-  return command_line;
+  return success;
 }
 
-CommandLine* CreateLaunchCommandLine() {
+bool CreateLaunchCommandLine(scoped_ptr<CommandLine>* command_line) {
+  DCHECK(command_line);
+
   // Shortcut for OS versions that don't need the integrity broker.
   if (base::win::GetVersion() < base::win::VERSION_VISTA) {
-    return new CommandLine(GetChromeExecutablePath());
+    command_line->reset(new CommandLine(GetChromeExecutablePath()));
+    return true;
   }
 
-  return CreateChromeLauncherCommandLine();
+  return CreateChromeLauncherCommandLine(command_line);
 }
 
 FilePath GetChromeExecutablePath() {
diff --git a/chrome_frame/chrome_launcher_utils.h b/chrome_frame/chrome_launcher_utils.h
index 4f5cdad..699e75e 100644
--- a/chrome_frame/chrome_launcher_utils.h
+++ b/chrome_frame/chrome_launcher_utils.h
@@ -9,6 +9,7 @@
 
 class CommandLine;
 class FilePath;
+template <class C> class scoped_ptr;
 
 namespace chrome_launcher {
 
@@ -19,15 +20,19 @@ extern const wchar_t kLauncherExeBaseName[];
 // flags needed before launching.
 //
 // The command-line may use the Chrome executable directly, or use an in-between
-// process if needed for security/elevation purposes.  You must delete the
-// returned command line.
-CommandLine* CreateLaunchCommandLine();
+// process if needed for security/elevation purposes.
+//
+// On success, returns true and populates command_line, which must be non-NULL,
+// with the launch command line.
+bool CreateLaunchCommandLine(scoped_ptr<CommandLine>* command_line);
 
 // Creates a command line suitable for launching the specified command through
 // Google Update.
 //
-// You must delete the returned command line.
-CommandLine* CreateUpdateCommandLine(const std::wstring& update_command);
+// On success, returns true and populates command_line, which must be non-NULL,
+// with the update command line.
+bool CreateUpdateCommandLine(const std::wstring& update_command,
+                             scoped_ptr<CommandLine>* command_line);
 
 // Returns the full path to the Chrome executable.
 FilePath GetChromeExecutablePath();
diff --git a/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc b/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc
index 3e16551..c9084ca 100644
--- a/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc
+++ b/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc
@@ -59,10 +59,9 @@ HANDLE LaunchCommandDirectly(const std::wstring& command_field) {
 HANDLE LaunchCommandViaProcessLauncher(const std::wstring& command_field) {
   HANDLE launched_process = NULL;
 
-  scoped_ptr<CommandLine> command_line(
-      chrome_launcher::CreateUpdateCommandLine(command_field));
-
-  if (command_line != NULL) {
+  scoped_ptr<CommandLine> command_line;
+  if (chrome_launcher::CreateUpdateCommandLine(command_field, &command_line)) {
+    DCHECK(command_line != NULL);
     base::LaunchOptions options;
     options.start_hidden = true;
     base::LaunchProcess(*command_line, options, &launched_process);
diff --git a/chrome_frame/test/perf/chrome_frame_perftest.cc b/chrome_frame/test/perf/chrome_frame_perftest.cc
index 2faf2e8..0dd4579 100644
--- a/chrome_frame/test/perf/chrome_frame_perftest.cc
+++ b/chrome_frame/test/perf/chrome_frame_perftest.cc
@@ -292,8 +292,7 @@ class ChromeFrameStartupTest : public ChromeFramePerfTestBase {
 
     chrome_dll_ = dir_app_.Append(L"chrome.dll");
     chrome_exe_ = dir_app_.Append(chrome::kBrowserProcessExecutableName);
-    chrome_frame_dll_ = dir_app_.Append(L"servers");
-    chrome_frame_dll_ = chrome_frame_dll_.Append(kChromeFrameDllName);
+    chrome_frame_dll_ = dir_app_.Append(kChromeFrameDllName);
     icu_dll_ = dir_app_.Append(L"icudt.dll");
     avcodec_dll_ = dir_app_.Append(L"avcodec-53.dll");
     avformat_dll_ = dir_app_.Append(L"avformat-53.dll");
diff --git a/chrome_frame/test/test_with_web_server.cc b/chrome_frame/test/test_with_web_server.cc
index 7bc08c7..900427e 100644
--- a/chrome_frame/test/test_with_web_server.cc
+++ b/chrome_frame/test/test_with_web_server.cc
@@ -223,7 +223,6 @@ void ChromeFrameTestWithWebServer::VersionTest(BrowserKind browser,
     const wchar_t* page) {
   FilePath plugin_path;
   PathService::Get(base::DIR_MODULE, &plugin_path);
-  plugin_path = plugin_path.AppendASCII("servers");
   plugin_path = plugin_path.Append(kChromeFrameDllName);
 
   static FileVersionInfo* version_info =
diff --git a/chrome_frame/test_utils.cc b/chrome_frame/test_utils.cc
index f90db60..8a1c380 100644
--- a/chrome_frame/test_utils.cc
+++ b/chrome_frame/test_utils.cc
@@ -29,8 +29,7 @@ FilePath GetChromeFrameBuildPath() {
   FilePath build_path;
   PathService::Get(chrome::DIR_APP, &build_path);
 
-  FilePath dll_path = build_path.Append(L"servers").
-                                 Append(kChromeFrameDllName);
+  FilePath dll_path = build_path.Append(kChromeFrameDllName);
 
   if (!file_util::PathExists(dll_path)) {
     // Well, dang.. try looking in the current directory.
@@ -45,7 +44,7 @@ FilePath GetChromeFrameBuildPath() {
   return dll_path;
 }
 
-bool ScopedChromeFrameRegistrar::register_chrome_path_provider_ = false;;
+bool ScopedChromeFrameRegistrar::register_chrome_path_provider_ = false;
 
 // static
 void ScopedChromeFrameRegistrar::RegisterDefaults() {
@@ -53,8 +52,6 @@ void ScopedChromeFrameRegistrar::RegisterDefaults() {
     chrome::RegisterPathProvider();
     register_chrome_path_provider_ = true;
   }
-  FilePath dll_path = GetChromeFrameBuildPath();
-  RegisterAtPath(dll_path.value(), chrome_frame_test::GetTestBedType());
 }
 
 // static
@@ -63,7 +60,8 @@ void ScopedChromeFrameRegistrar::RegisterAtPath(
 
   ASSERT_FALSE(path.empty());
   HMODULE dll_handle = LoadLibrary(path.c_str());
-  ASSERT_TRUE(dll_handle != NULL);
+  ASSERT_TRUE(dll_handle != NULL) << "Failed to load " << path
+                                  << " , gle = " << GetLastError();
 
   typedef HRESULT (STDAPICALLTYPE* DllRegisterServerFn)();
   DllRegisterServerFn register_server = NULL;
@@ -76,9 +74,14 @@ void ScopedChromeFrameRegistrar::RegisterAtPath(
           dll_handle, "DllRegisterServer"));
   }
   ASSERT_TRUE(register_server != NULL);
-  EXPECT_HRESULT_SUCCEEDED((*register_server)());
-
+  HRESULT reg_result = (*register_server)();
   ASSERT_TRUE(FreeLibrary(dll_handle));
+
+  // Assert here after the FreeLibrary since otherwise we hit a
+  // multiple AtExitMananger crash for modules that use base.
+  ASSERT_HRESULT_SUCCEEDED(reg_result) << "Failed to register DLL at "
+                                       << path
+                                       << " , are you running as Admin?";
 }
 
 // static