Improve multi-process debugging.

1) --wait-for-debugger alone makes all child processes wait for a debugger
2) --wait-for-debugger=app1,app2... make only these apps wait for a debugger
3) the app name is added to the command line to make it easier to attach debugger after the process starts
4) MessageBox is used on Windows like chrome

BUG=478251

Review URL: https://codereview.chromium.org/1107633002

Cr-Commit-Position: refs/heads/master@{#326676}

8 files changed, 41 insertions, 9 deletions

diff --git a/mojo/shell/child_process.cc b/mojo/shell/child_process.cc
index eecb98c..df5eb498 100644
--- a/mojo/shell/child_process.cc
+++ b/mojo/shell/child_process.cc
@@ -4,9 +4,11 @@
 
 #include "mojo/shell/child_process.h"
 
+#include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
+#include "base/debug/debugger.h"
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/logging.h"
@@ -279,6 +281,19 @@ int ChildProcessMain() {
   DVLOG(2) << "ChildProcessMain()";
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kWaitForDebugger)) {
+    std::string app = command_line.GetSwitchValueASCII(switches::kApp);
+#if defined(OS_WIN)
+    MessageBox(NULL, command_line.GetSwitchValueNative(switches::kApp).c_str(),
+               command_line.GetSwitchValueNative(switches::kApp).c_str(),
+               MB_OK | MB_SETFOREGROUND);
+#else
+    LOG(ERROR) << command_line.GetSwitchValueASCII(switches::kApp)
+               << " waiting for GDB. pid: " << getpid();
+    base::debug::WaitForDebugger(60, true);
+#endif
+  }
+
   embedder::ScopedPlatformHandle platform_channel =
       embedder::PlatformChannelPair::PassClientHandleFromParentProcess(
           command_line);
diff --git a/mojo/shell/child_process_host.cc b/mojo/shell/child_process_host.cc
index 9d955e2..97c940b 100644
--- a/mojo/shell/child_process_host.cc
+++ b/mojo/shell/child_process_host.cc
@@ -13,6 +13,8 @@
 #include "base/message_loop/message_loop.h"
 #include "base/process/kill.h"
 #include "base/process/launch.h"
+#include "base/stl_util.h"
+#include "base/strings/string_split.h"
 #include "base/task_runner.h"
 #include "base/task_runner_util.h"
 #include "mojo/edk/embedder/embedder.h"
@@ -24,9 +26,10 @@
 namespace mojo {
 namespace shell {
 
-ChildProcessHost::ChildProcessHost(Context* context)
-    : context_(context), channel_info_(nullptr) {
+ChildProcessHost::ChildProcessHost(Context* context, const std::string& name)
+    : context_(context), name_(name), channel_info_(nullptr) {
   platform_channel_ = platform_channel_pair_.PassServerHandle();
+  DCHECK(!name.empty());
   CHECK(platform_channel_.is_valid());
 }
 
@@ -102,7 +105,16 @@ bool ChildProcessHost::DoLaunch() {
   base::CommandLine child_command_line(parent_command_line->GetProgram());
   child_command_line.CopySwitchesFrom(*parent_command_line, kForwardSwitches,
                                       arraysize(kForwardSwitches));
+  child_command_line.AppendSwitchASCII(switches::kApp, name_);
   child_command_line.AppendSwitch(switches::kChildProcess);
+  if (parent_command_line->HasSwitch(switches::kWaitForDebugger)) {
+    std::vector<std::string> apps_to_debug;
+    base::SplitString(
+        parent_command_line->GetSwitchValueASCII(switches::kWaitForDebugger),
+        ',', &apps_to_debug);
+    if (apps_to_debug.empty() || ContainsValue(apps_to_debug, name_))
+      child_command_line.AppendSwitch(switches::kWaitForDebugger);
+  }
 
   embedder::HandlePassingInformation handle_passing_info;
   platform_channel_pair_.PrepareToPassClientHandleToChildProcess(
diff --git a/mojo/shell/child_process_host.h b/mojo/shell/child_process_host.h
index 61b357b..bc30206 100644
--- a/mojo/shell/child_process_host.h
+++ b/mojo/shell/child_process_host.h
@@ -30,7 +30,8 @@ class Context;
 // remained alive until the |on_app_complete| callback is called.
 class ChildProcessHost {
  public:
-  explicit ChildProcessHost(Context* context);
+  // |name| is just for debugging ease.
+  ChildProcessHost(Context* context, const std::string& name);
   virtual ~ChildProcessHost();
 
   // |Start()|s the child process; calls |DidStart()| (on the thread on which
@@ -65,6 +66,7 @@ class ChildProcessHost {
   void DidCreateChannel(embedder::ChannelInfo* channel_info);
 
   Context* const context_;
+  const std::string name_;
   base::Process child_process_;
   embedder::PlatformChannelPair platform_channel_pair_;
   ChildControllerPtr controller_;
diff --git a/mojo/shell/child_process_host_unittest.cc b/mojo/shell/child_process_host_unittest.cc
index 8e8f16d..8798130 100644
--- a/mojo/shell/child_process_host_unittest.cc
+++ b/mojo/shell/child_process_host_unittest.cc
@@ -20,7 +20,8 @@ namespace {
 // Subclass just so we can observe |DidStart()|.
 class TestChildProcessHost : public ChildProcessHost {
  public:
-  explicit TestChildProcessHost(Context* context) : ChildProcessHost(context) {}
+  explicit TestChildProcessHost(Context* context)
+      : ChildProcessHost(context, "test") {}
   ~TestChildProcessHost() override {}
 
   void DidStart(bool success) override {
diff --git a/mojo/shell/context.cc b/mojo/shell/context.cc
index 0cc27d1..f938980 100644
--- a/mojo/shell/context.cc
+++ b/mojo/shell/context.cc
@@ -6,7 +6,6 @@
 
 #include <vector>
 
-#include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ -238,9 +237,6 @@ bool Context::Init() {
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
 
-  if (command_line.HasSwitch(switches::kWaitForDebugger))
-    base::debug::WaitForDebugger(60, true);
-
   EnsureEmbedderIsInitialized();
   task_runners_.reset(
       new TaskRunners(base::MessageLoop::current()->message_loop_proxy()));
diff --git a/mojo/shell/out_of_process_native_runner.cc b/mojo/shell/out_of_process_native_runner.cc
index 181a4d25..f09ea82 100644
--- a/mojo/shell/out_of_process_native_runner.cc
+++ b/mojo/shell/out_of_process_native_runner.cc
@@ -38,7 +38,8 @@ void OutOfProcessNativeRunner::Start(
   DCHECK(app_completed_callback_.is_null());
   app_completed_callback_ = app_completed_callback;
 
-  child_process_host_.reset(new ChildProcessHost(context_));
+  std::string name = app_path.BaseName().RemoveExtension().MaybeAsASCII();
+  child_process_host_.reset(new ChildProcessHost(context_, name));
   child_process_host_->Start();
 
   // TODO(vtl): |app_path.AsUTF8Unsafe()| is unsafe.
diff --git a/mojo/shell/switches.cc b/mojo/shell/switches.cc
index 1fff40d..e5c9d34 100644
--- a/mojo/shell/switches.cc
+++ b/mojo/shell/switches.cc
@@ -8,6 +8,10 @@
 
 namespace switches {
 
+// Used just for debugging to make it easier to attach debuggers. The actual app
+// path that is used is sent over IPC.
+const char kApp[] = "app";
+
 // Used internally by the main process to indicate that a new process should be
 // a child process. Not for user use.
 const char kChildProcess[] = "child-process";
diff --git a/mojo/shell/switches.h b/mojo/shell/switches.h
index 31cac8f..b7d9a37 100644
--- a/mojo/shell/switches.h
+++ b/mojo/shell/switches.h
@@ -12,6 +12,7 @@ namespace switches {
 
 // All switches in alphabetical order. The switches should be documented
 // alongside the definition of their values in the .cc file.
+extern const char kApp[];
 extern const char kChildProcess[];
 extern const char kContentHandlers[];
 extern const char kDisableCache[];