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}

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);

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(

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_;

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 {

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()));

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.

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";

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[];