Use StopChildProcess instead of base::KillProcess to kill a renderer process

content/browser/renderer_host/render_process_host_impl.cc

 // Copyright (c) 2012 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.
 
 // Represents the browser side of the browser <--> renderer communication
 // channel. There will be one RenderProcessHost per renderer process.
 
 #include "content/browser/renderer_host/render_process_host_impl.h"
 
 #include <algorithm>
@@ skipping 131 matching lines @@
 #include "storage/browser/fileapi/sandbox_file_system_backend.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/events/event_switches.h"
 #include "ui/gfx/switches.h"
 #include "ui/gl/gl_switches.h"
 #include "ui/gl/gpu_switching_manager.h"
 #include "ui/native_theme/native_theme_switches.h"
 
 #if defined(OS_ANDROID)
+#include "content/browser/android/child_process_launcher_android.h"
 #include "content/browser/media/android/browser_demuxer_android.h"
 #include "content/browser/screen_orientation/screen_orientation_message_filter_android.h"
 #endif
 
 #if defined(OS_WIN)
 #include "base/win/scoped_com_initializer.h"
 #include "content/common/font_cache_dispatcher_win.h"
 #include "content/common/sandbox_win.h"
 #include "ui/gfx/win/dpi.h"
 #endif
@@ skipping 795 matching lines @@
   if (command_line->HasSwitch(switches::kDisableKillAfterBadIPC))
     return;
 
   if (run_renderer_in_process()) {
     // In single process mode it is better if we don't suicide but just
     // crash.
     CHECK(false);
   }
   // We kill the renderer but don't include a NOTREACHED, because we want the
   // browser to try to survive when it gets illegal messages from the renderer.
-  base::KillProcess(GetHandle(), RESULT_CODE_KILLED_BAD_MESSAGE,
-                    false);
+  FastShutdown(RESULT_CODE_KILLED_BAD_MESSAGE, false);
 }
 
 void RenderProcessHostImpl::WidgetRestored() {
   // Verify we were properly backgrounded.
   DCHECK_EQ(backgrounded_, (visible_widgets_ == 0));
   visible_widgets_++;
   SetBackgrounded(false);
 }
 
 void RenderProcessHostImpl::WidgetHidden() {
@@ skipping 354 matching lines @@
   }
 
   // Set this before ProcessDied() so observers can tell if the render process
   // died due to fast shutdown versus another cause.
   fast_shutdown_started_ = true;
 
   ProcessDied(false /* already_dead */);
   return true;
 }
 
+bool RenderProcessHostImpl::FastShutdown(int exit_code, bool wait) {
+  if (run_renderer_in_process())
+    return false;  // Single process mode never shutdown the renderer.

[Charlie Reis, 2014/12/01 19:44:09]

nit: shutdown -> shuts down

[Jaekyun Seok (inactive), 2014/12/02 00:50:38]

Done.

+
+#if defined(OS_ANDROID)

[no sievers, 2014/12/01 21:32:13]

I'm still confused why we need an ifdef here.
Why can't all platforms go through the same shutdown path?

We have already abstracted the Android-specifics in ChildProcessLauncher.
Can't we just do the same as the function above, which is call ProcessDied(false
/* already dead */) (this should probably ), which then kills the channel and
Context which in turn causes StopChildProcess() to be called?

Is there really a reason why we want to defer the ProcessDied() here, i.e. it
getting called from OnChannelError instead with |already_dead == true|?

[Jaekyun Seok (inactive), 2014/12/02 00:50:38]

I wanted not to change any execution flow on non-android platforms. But using
ProcessDied() seems reasonable.

I will use it.

+  // Android requires a different approach for killing.
+  StopChildProcess(GetHandle());
+  return true;
+#else
+  return base::KillProcess(GetHandle(), exit_code, wait);
+#endif
+}
+
 void RenderProcessHostImpl::DumpHandles() {
 #if defined(OS_WIN)
   Send(new ChildProcessMsg_DumpHandles());
 #else
   NOTIMPLEMENTED();
 #endif
 }
 
 bool RenderProcessHostImpl::Send(IPC::Message* msg) {
   TRACE_EVENT0("renderer_host", "RenderProcessHostImpl::Send");
@@ skipping 895 matching lines @@
 
 void RenderProcessHostImpl::DecrementWorkerRefCount() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK_GT(worker_ref_count_, 0);
   --worker_ref_count_;
   if (worker_ref_count_ == 0)
     Cleanup();
 }
 
 }  // namespace content