Revert of Bind Application in renderer (patchset chromium#15 id:28000…

…1 of https://codereview.chromium.org/1452823003/ )

Reason for revert:
This change is making hundreds of webkit_tests crash on linux_chromium_rel_ng with:

[1:1:1118/141621:108326049405:FATAL:lock.cc(32)] Check failed: owning_thread_ref_.is_null().
#0 0x00000067e2de base::debug::StackTrace::StackTrace()
chromium#1 0x00000057a71f logging::LogMessage::~LogMessage()
chromium#2 0x0000005eb69b base::Lock::CheckUnheldAndMark()
chromium#3 0x000000518e1c base::Lock::Acquire()
chromium#4 0x000000518b43 base::AutoLock::AutoLock()
chromium#5 0x0000005b4014 base::SequenceCheckerImpl::CalledOnValidSequencedThread()
chromium#6 0x000006f074ae IDMap<>::Lookup()
chromium#7 0x000006f0598c content::RendererBlinkPlatformImpl::SetPlatformEventObserverForTesting()
chromium#8 0x000008459264 content::SetMockGamepadProvider()

Original issue's description:
> Bind Application in renderer.
>
> This involved changing how we get the client handle to the renderer. In the first iteration I was passing this on the command line but that turns out not to work with the sandbox. So instead I an approach used by the Mojo-in-Chrome MojoApplication class and pass the primordial handle via Chrome IPC.
>
> I had to twiddle a bunch of BUILD.gn files in content to get this to work without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.
>
> R=jam@chromium.org,tsepez@chromium.org
> http://crbug.com/551253
>
> Committed: https://crrev.com/3edb97198bc5fbc22c5cf13286e8af80449ddfb9
> Cr-Commit-Position: refs/heads/master@{#360293}
>
> Committed: https://crrev.com/2c716f9d5ebff610641f4506c17ec81d4b89b7bb
> Cr-Commit-Position: refs/heads/master@{#360396}

TBR=jam@chromium.org,tsepez@chromium.org,ben@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

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

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

components/mus/example/window_type_launcher/main.cc

@@ -61,17 +61,19 @@ int main(int argc, char** argv) {
                                    io_thread.task_runner().get(),
                                    mojo::embedder::ScopedPlatformHandle());
 
-    base::MessageLoop loop(mojo::common::MessagePumpMojo::Create());
+    mojo::InterfaceRequest<mojo::Application> application_request;
+    scoped_ptr<mojo::runner::RunnerConnection> connection(
+        mojo::runner::RunnerConnection::ConnectToRunner(&application_request));
+
     WindowTypeLauncher delegate;
     {
-      mojo::InterfaceRequest<mojo::Application> application_request;
-      scoped_ptr<mojo::runner::RunnerConnection> connection(
-          mojo::runner::RunnerConnection::ConnectToRunner(
-              &application_request, mojo::ScopedMessagePipeHandle()));
+      base::MessageLoop loop(mojo::common::MessagePumpMojo::Create());
       mojo::ApplicationImpl impl(&delegate, application_request.Pass());
       loop.Run();
     }
 
+    connection.reset();
+
     mojo::embedder::ShutdownIPCSupport();
   }
 

content/app/BUILD.gn

@@ -70,7 +70,6 @@ if (is_ios) {
 content_app_extra_configs = [
   "//build/config/compiler:wexit_time_destructors",
   "//content:content_implementation",
-  "//content/public/common:mojo_shell_client",
   "//v8:external_startup_data",
 ]
 

content/browser/BUILD.gn

@@ -12,10 +12,7 @@ source_set("browser") {
   # internal content ones) should depend on the public one.
   visibility = [ "//content/public/browser:browser_sources" ]
 
-  configs += [
-    "//build/config:precompiled_headers",
-    "//content/public/common:mojo_shell_client",
-  ]
+  configs += [ "//build/config:precompiled_headers" ]
   defines = []
   libs = []
   ldflags = []
@@ -107,10 +104,7 @@ source_set("browser") {
             ],
             ".")
 
-    sources += [
-      "mojo/mojo_shell_client_host.cc",
-      "mojo/mojo_shell_client_host.h",
-    ]
+    defines += [ "MOJO_SHELL_CLIENT" ]
 
     # Non-iOS deps.
     deps += [

content/browser/child_process_launcher.cc

@@ -48,6 +48,17 @@
 #include "gin/v8_initializer.h"
 #endif
 
+#if defined(MOJO_SHELL_CLIENT)
+#include "base/thread_task_runner_handle.h"
+#include "content/public/common/mojo_shell_connection.h"
+#include "mojo/application/public/cpp/application_impl.h"
+#include "mojo/converters/network/network_type_converters.h"
+#include "mojo/shell/application_manager.mojom.h"
+#include "third_party/mojo/src/mojo/edk/embedder/embedder.h"
+#include "third_party/mojo/src/mojo/edk/embedder/platform_channel_pair.h"
+#include "third_party/mojo/src/mojo/edk/embedder/scoped_platform_handle.h"
+#endif
+
 namespace content {
 
 namespace {
@@ -322,6 +333,10 @@ void SetProcessBackgroundedOnLauncherThread(base::Process process,
 #endif
 }
 
+#if defined(MOJO_SHELL_CLIENT)
+void DidCreateChannel(mojo::embedder::ChannelInfo* info) {}
+#endif
+
 }  // namespace
 
 ChildProcessLauncher::ChildProcessLauncher(
@@ -365,6 +380,10 @@ void ChildProcessLauncher::Launch(
     int child_process_id) {
   DCHECK(CalledOnValidThread());
 
+#if defined(MOJO_SHELL_CLIENT)
+  CreateMojoShellChannel(cmd_line, child_process_id);
+#endif
+
 #if defined(OS_ANDROID)
   // Android only supports renderer, sandboxed utility and gpu.
   std::string process_type =
@@ -504,6 +523,46 @@ void ChildProcessLauncher::Notify(
   }
 }
 
+#if defined(MOJO_SHELL_CLIENT)
+void ChildProcessLauncher::CreateMojoShellChannel(
+    base::CommandLine* command_line,
+    int child_process_id) {
+  // Some process types get created before the main message loop.
+  if (!MojoShellConnection::Get())
+    return;
+
+  // Create the channel to be shared with the target process.
+  mojo::embedder::HandlePassingInformation handle_passing_info;
+  mojo::embedder::PlatformChannelPair platform_channel_pair;
+
+  // Give one end to the shell so that it can create an instance.
+  mojo::embedder::ScopedPlatformHandle platform_channel =
+      platform_channel_pair.PassServerHandle();
+  mojo::ScopedMessagePipeHandle handle(mojo::embedder::CreateChannel(
+      platform_channel.Pass(), base::Bind(&DidCreateChannel),
+      base::ThreadTaskRunnerHandle::Get()));
+  mojo::shell::mojom::ApplicationManagerPtr application_manager;
+  MojoShellConnection::Get()->GetApplication()->ConnectToService(
+      mojo::URLRequest::From(std::string("mojo:shell")),
+      &application_manager);
+  // The content of the URL/qualifier we pass is actually meaningless, it's only
+  // important that they're unique per process.
+  // TODO(beng): We need to specify a restrictive CapabilityFilter here that
+  //             matches the needs of the target process. Figure out where that
+  //             specification is best determined (not here, this is a common
+  //             chokepoint for all process types) and how to wire it through.
+  //             http://crbug.com/555393
+  application_manager->CreateInstanceForHandle(
+      mojo::ScopedHandle(mojo::Handle(handle.release().value())),
+      "exe:chrome_renderer",  // See above about how this string is meaningless.
+      base::IntToString(child_process_id));
+
+  // Put the other end on the command line used to launch the target.
+  platform_channel_pair.PrepareToPassClientHandleToChildProcess(
+      command_line, &handle_passing_info);
+}
+#endif  // defined(MOJO_SHELL_CLIENT)
+
 bool ChildProcessLauncher::IsStarting() {
   // TODO(crbug.com/469248): This fails in some tests.
   // DCHECK(CalledOnValidThread());

content/browser/renderer_host/render_process_host_impl.cc

@@ -221,10 +221,6 @@
 #include "content/common/media/media_stream_messages.h"
 #endif
 
-#if defined(MOJO_SHELL_CLIENT)
-#include "content/browser/mojo/mojo_shell_client_host.h"
-#endif
-
 #if defined(OS_WIN)
 #define IntToStringType base::IntToString16
 #else
@@ -2454,12 +2450,6 @@ void RenderProcessHostImpl::OnProcessLaunched() {
                                          Source<RenderProcessHost>(this),
                                          NotificationService::NoDetails());
 
-#if defined(MOJO_SHELL_CLIENT)
-  // Send a handle that the external Mojo shell can use to pass an Application
-  // request to the child.
-  RegisterChildWithExternalShell(id_, GetHandle(), this);
-#endif
-
   // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
   // is fixed.
   tracked_objects::ScopedTracker tracking_profile4(

content/child/BUILD.gn

@@ -15,10 +15,7 @@ source_set("child") {
                         ".",
                         "//content")
 
-  configs += [
-    "//build/config:precompiled_headers",
-    "//content/public/common:mojo_shell_client",
-  ]
+  configs += [ "//build/config:precompiled_headers" ]
 
   public_deps = [
     "//third_party/mojo/src/mojo/edk/system",

content/child/child_thread_impl.cc

@@ -73,11 +73,6 @@
 #include "ui/ozone/public/client_native_pixmap_factory.h"
 #endif
 
-#if defined(MOJO_SHELL_CLIENT)
-#include "content/common/mojo/mojo_messages.h"
-#include "content/common/mojo/mojo_shell_connection_impl.h"
-#endif
-
 using tracked_objects::ThreadData;
 
 namespace content {
@@ -658,9 +653,6 @@ bool ChildThreadImpl::OnMessageReceived(const IPC::Message& msg) {
                         OnProcessBackgrounded)
 #if defined(USE_TCMALLOC)
     IPC_MESSAGE_HANDLER(ChildProcessMsg_GetTcmallocStats, OnGetTcmallocStats)
-#endif
-#if defined(MOJO_SHELL_CLIENT)
-    IPC_MESSAGE_HANDLER(MojoMsg_BindControllerHandle, OnBindControllerHandle)
 #endif
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
@@ -726,21 +718,6 @@ void ChildThreadImpl::OnGetTcmallocStats() {
 }
 #endif
 
-#if defined(MOJO_SHELL_CLIENT)
-void ChildThreadImpl::OnBindControllerHandle(
-    const IPC::PlatformFileForTransit& file) {
-#if defined(OS_POSIX)
-  base::PlatformFile handle = file.fd;
-#elif defined(OS_WIN)
-  base::PlatformFile handle = file;
-#endif
-  mojo::ScopedMessagePipeHandle message_pipe =
-      mojo_shell_channel_init_.Init(handle, GetIOTaskRunner());
-  DCHECK(message_pipe.is_valid());
-  MojoShellConnectionImpl::CreateWithMessagePipe(message_pipe.Pass());
-}
-#endif
-
 ChildThreadImpl* ChildThreadImpl::current() {
   return g_lazy_tls.Pointer()->Get();
 }

content/child/child_thread_impl.h

@@ -20,11 +20,6 @@
 #include "content/public/child/child_thread.h"
 #include "ipc/ipc_message.h"  // For IPC_MESSAGE_LOG_ENABLED.
 
-#if defined(MOJO_SHELL_CLIENT)
-#include "content/common/mojo/channel_init.h"
-#include "ipc/ipc_platform_file.h"
-#endif
-
 namespace base {
 class MessageLoop;
 
@@ -240,9 +235,6 @@ class CONTENT_EXPORT ChildThreadImpl
 #if defined(USE_TCMALLOC)
   void OnGetTcmallocStats();
 #endif
-#if defined(MOJO_SHELL_CLIENT)
-  void OnBindControllerHandle(const IPC::PlatformFileForTransit& file);
-#endif
 
   void EnsureConnected();
 
@@ -305,10 +297,6 @@ class CONTENT_EXPORT ChildThreadImpl
 
   scoped_refptr<base::SequencedTaskRunner> browser_process_io_runner_;
 
-#if defined(MOJO_SHELL_CLIENT)
-  ChannelInit mojo_shell_channel_init_;
-#endif
-
   base::WeakPtrFactory<ChildThreadImpl> channel_connected_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChildThreadImpl);

content/common/BUILD.gn

@@ -209,7 +209,7 @@ source_set("common") {
     ]
   }
 
-  defines = []
+  defines = [ "MOJO_SHELL_CLIENT" ]
   include_dirs = []
   libs = []
   ldflags = []

content/common/mojo/mojo_messages.h

@@ -21,8 +21,3 @@
 // Mojo IPC is bootstrapped over Chrome IPC via this message.
 IPC_MESSAGE_CONTROL1(MojoMsg_Activate,
                      IPC::PlatformFileForTransit /* handle */)
-
-// Mojo IPC to an external shell is bootstrapped over Chrome IPC via this
-// message.
-IPC_MESSAGE_CONTROL1(MojoMsg_BindControllerHandle,
-                     IPC::PlatformFileForTransit /* handle */)