Unifying ChildProcessLauncher across platforms.

content/browser/child_process_launcher_android.cc

Index: content/browser/child_process_launcher_android.cc
diff --git a/content/browser/child_process_launcher_android.cc b/content/browser/child_process_launcher_android.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6068a6578045e9786c8afa845a2aaaf8eb566ded
--- /dev/null
+++ b/content/browser/child_process_launcher_android.cc
@@ -0,0 +1,190 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.

[boliu, 2017/01/12 02:00:07]

2017 for new files

[Jay Civelli, 2017/01/12 23:05:41]

Done.
This was written in 2016 though :-)

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/child_process_launcher.h"
+
+#include <memory>
+
+#include "base/i18n/icu_util.h"
+#include "base/logging.h"
+#include "base/metrics/field_trial.h"
+#include "content/browser/android/child_process_launcher_android_jni.h"
+#include "content/browser/child_process_launcher_posix.h"
+#include "content/browser/file_descriptor_info_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/common/content_descriptors.h"
+#include "content/public/common/content_switches.h"
+#include "gin/v8_initializer.h"
+
+namespace content {
+
+namespace {
+
+struct ChildProcessHelperHolder {
+ public:
+  explicit ChildProcessHelperHolder(ChildProcessLauncher::Helper* helper)
+      : helper(helper) {
+  }
+
+  scoped_refptr<ChildProcessLauncher::Helper> helper;
+};
+
+// Callback invoked from Java once the process has been started.
+void ChildProcessStartedCallback(
+    ChildProcessHelperHolder* helper_holder,
+    base::ProcessHandle handle,
+    int launch_result) {
+  // TODO(jcivelli): Remove this by defining better what happens on what thread
+  // in the corresponding Java code.
+  if (BrowserThread::CurrentlyOn(BrowserThread::PROCESS_LAUNCHER)) {
+    helper_holder->helper->PostLaunchOnLauncherThread(
+        base::Process(handle), nullptr, launch_result);
+  } else {
+    BrowserThread::PostTask(BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+        base::Bind(&ChildProcessLauncher::Helper::PostLaunchOnLauncherThread,

[boliu, 2017/01/12 02:00:07]

this is an extra thread hop compared to previous code. comment in old code said
this could already be the UI thread, so did a shortcut

[Jay Civelli, 2017/01/12 23:05:41]

True, this seems to be called mostly on the UI thread (from what I saw).
Changed it so PostLaunchOnLauncherThread() has a param whether to call
PostLaunchOnClientThread. I don't like that it makes the API more complicated
and the flow less clear (sometimes PostLaunchOnLauncherThread can be called
after PostLaunchOnClientThread), but that extra hop may affect startup
performance.

+                   helper_holder->helper,
+                   base::Passed(base::Process(handle)),
+                   nullptr,
+                   launch_result));
+  }
+}
+
+}  // namespace
+
+void ChildProcessLauncher::Helper::BeforeLaunchOnClientThread() {
+  // Android only supports renderer, sandboxed utility and gpu.
+  std::string process_type =
+      command_line()->GetSwitchValueASCII(switches::kProcessType);
+  CHECK(process_type == switches::kGpuProcess ||
+        process_type == switches::kRendererProcess ||
+#if BUILDFLAG(ENABLE_PLUGINS)
+        process_type == switches::kPpapiPluginProcess ||
+#endif
+        process_type == switches::kUtilityProcess)
+      << "Unsupported process type: " << process_type;
+
+  // Non-sandboxed utility or renderer process are currently not supported.
+  DCHECK(process_type == switches::kGpuProcess ||
+         !command_line()->HasSwitch(switches::kNoSandbox));
+}
+
+mojo::edk::ScopedPlatformHandle
+ChildProcessLauncher::Helper::PrepareMojoPipeHandlesOnClientThread() {
+  return mojo::edk::ScopedPlatformHandle();
+}
+
+std::unique_ptr<FileDescriptorInfo>
+ChildProcessLauncher::Helper::GetFilesToMap() {
+  DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
+
+  // Android WebView runs in single process, ensure that we never get here when
+  // running in single process mode.
+  CHECK(!command_line()->HasSwitch(switches::kSingleProcess));
+
+  std::unique_ptr<FileDescriptorInfo> files_to_register =
+      CreateDefaultPosixFilesToMap(*command_line(), child_process_id(),
+                                   mojo_client_handle());
+
+#if defined(V8_USE_EXTERNAL_STARTUP_DATA)
+  base::MemoryMappedFile::Region region;
+  auto maybe_register = [&region, &files_to_register](int key, int fd) {
+    if (fd != -1)
+      files_to_register->ShareWithRegion(key, fd, region);
+  };
+  maybe_register(
+      kV8NativesDataDescriptor,
+      gin::V8Initializer::GetOpenNativesFileForChildProcesses(&region));
+  maybe_register(
+      kV8SnapshotDataDescriptor32,
+      gin::V8Initializer::GetOpenSnapshotFileForChildProcesses(&region, true));
+  maybe_register(
+      kV8SnapshotDataDescriptor64,
+      gin::V8Initializer::GetOpenSnapshotFileForChildProcesses(&region, false));
+
+  if (GetProcessType() != switches::kZygoteProcess) {

[boliu, 2017/01/12 02:00:07]

this can be removed from android, zygote is not in the CHECK above?

[Jay Civelli, 2017/01/12 23:05:41]

Good point, done.

+    command_line()->AppendSwitch(::switches::kV8NativesPassedByFD);
+    command_line()->AppendSwitch(::switches::kV8SnapshotPassedByFD);
+  }
+#endif  // defined(V8_USE_EXTERNAL_STARTUP_DATA)
+
+#if ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE
+  int fd = base::i18n::GetIcuDataFileHandle(&region);
+  files_to_register->ShareWithRegion(kAndroidICUDataDescriptor, fd, region);
+#endif  // ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE
+
+  return files_to_register;
+}
+
+bool ChildProcessLauncher::Helper::ShouldForkAsZygote() {
+    return false;

[boliu, 2017/01/12 02:00:07]

indent

[Jay Civelli, 2017/01/12 23:05:41]

Done.

+}
+
+ZygoteHandle ChildProcessLauncher::Helper::ForkAsZygote(
+        std::unique_ptr<FileDescriptorInfo> files_to_register,
+        base::Process* process) {
+  NOTREACHED();
+  return nullptr;
+}
+
+void ChildProcessLauncher::Helper::BeforeLaunchOnLauncherThread(
+    const FileDescriptorInfo& files_to_register,
+    base::LaunchOptions* options) {
+}
+
+base::Process ChildProcessLauncher::Helper::LaunchProcessOnLauncherThread(
+    const base::LaunchOptions& options,
+    FileDescriptorInfo* files_to_register,
+    bool* is_synchronous_launch,
+    int* launch_result) {
+  *is_synchronous_launch = false;
+
+  StartChildProcess(command_line()->argv(),
+                    child_process_id(),
+                    files_to_register,
+                    base::Bind(&ChildProcessStartedCallback,
+                               new ChildProcessHelperHolder(this)));

[boliu, 2017/01/12 02:00:07]

what's responsible for deleting holder?

Can ChildProcessStartedCallback take a reference of the helper instead of using
a "Holder" here?

[Jay Civelli, 2017/01/12 23:05:41]

Good point, I can take care of increasing the ref count myself (to guarantee the
helper is still around by the time the callback is called) and then don't need
the holder.

[boliu, 2017/01/13 01:33:50]

This is not necessary, also not correct if the callback doesn't run. It should
just work if you change the function signature to take scoped_refptr:
https://chromium.googlesource.com/chromium/src/+/master/docs/callback.md#Pass...

[Jay Civelli, 2017/01/13 18:08:01]

The reason we don't need to do this most of the time is because the callback
object is passed to a task runner when posted, and then owned by that task
runner, keeping the bound parameters alive.
In this case we are not posting to a task runner, we are passing it to Java as a
ptr. The way we keep things alive is by making a copy of the callback allocated
on the heap (see
https://cs.chromium.org/chromium/src/content/browser/android/child_process_la...
the code has moved to child_process_launcher_android_jni.cc in this CL but is
unchanged). The callback deletes itself when invoked (by Java), so it will be
leaked if the callback is never called already.
To make along story short, you are right I do not need to addref/release, since
the callback allocated on the heap keeps the bound parameters alive.

+
+  return base::Process();
+}
+
+void ChildProcessLauncher::Helper::AfterLaunchOnLauncherThread(
+    const base::Process& process,
+    const base::LaunchOptions& options) {
+}
+
+// static
+bool ChildProcessLauncher::TerminateProcess(
+    const base::Process& process, int exit_code, bool wait) {
+  StopChildProcess(process.Handle());
+  return true;
+}
+
+// static
+void ChildProcessLauncher::TerminateOnLauncherThread(
+    ZygoteHandle zygote, base::Process process) {
+  DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
+  VLOG(1) << "ChromeProcess: Stopping process with handle "
+          << process.Handle();
+  StopChildProcess(process.Handle());
+}
+
+// static
+void ChildProcessLauncher::SetProcessBackgroundedOnLauncherThread(
+      base::Process process, bool background) {
+  SetChildProcessInForeground(process.Handle(), !background);
+}
+
+void ChildProcessLauncher::UpdateTerminationStatus(bool known_dead) {
+  DCHECK(CalledOnValidThread());
+  if (IsChildProcessOomProtected(process_.Handle())) {
+    termination_status_ = base::TERMINATION_STATUS_OOM_PROTECTED;
+  } else {
+    termination_status_ =
+      base::GetTerminationStatus(process_.Handle(), &exit_code_);
+  }
+}
+
+}  // namespace content