Renderers should obtain browser InterfaceProvider by connecting to browser

content/browser/renderer_host/render_process_host_impl.cc

 // Copyright 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 130 matching lines @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host_factory.h"
 #include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/render_widget_host_view_frame_subscriber.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/worker_service.h"
 #include "content/public/common/child_process_host.h"
+#include "content/public/common/connection_filter.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/mojo_channel_switches.h"
 #include "content/public/common/process_type.h"
 #include "content/public/common/resource_type.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/sandboxed_process_launcher_delegate.h"
 #include "content/public/common/url_constants.h"
 #include "device/battery/battery_monitor_impl.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gpu_switches.h"
 #include "gpu/command_buffer/common/gles2_cmd_utils.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "ipc/attachment_broker.h"
 #include "ipc/attachment_broker_privileged.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_channel_mojo.h"
 #include "ipc/ipc_logging.h"
 #include "ipc/ipc_switches.h"
 #include "media/base/media_switches.h"
 #include "mojo/edk/embedder/embedder.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "ppapi/shared_impl/ppapi_switches.h"
+#include "services/shell/public/cpp/connection.h"
 #include "services/shell/public/cpp/interface_provider.h"
 #include "services/shell/public/cpp/interface_registry.h"
 #include "services/shell/runner/common/switches.h"
 #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/display/display_switches.h"
 #include "ui/events/event_switches.h"
 #include "ui/gfx/switches.h"
 #include "ui/gl/gl_switches.h"
@@ skipping 261 matching lines @@
   BrowserMainLoop::GetInstance()->memory_coordinator()->CreateHandle(
       render_process_id, std::move(request));
 }
 
 }  // namespace
 
 RendererMainThreadFactoryFunction g_renderer_main_thread_factory = NULL;
 
 base::MessageLoop* g_in_process_thread;
 
+// Stores the maximum number of renderer processes the content module can
+// create.
+static size_t g_max_renderer_count_override = 0;
+
+// static
+bool g_run_renderer_in_process_ = false;
+
+// Held by the RPH's BrowserContext's MojoShellConnection, ownership transferred
+// back to RPH upon RPH destruction.
+class RenderProcessHostImpl::ConnectionFilterImpl : public ConnectionFilter {
+ public:
+  ConnectionFilterImpl(
+      const shell::Identity& child_identity,
+      std::unique_ptr<shell::InterfaceRegistry> registry)
+      : child_identity_(child_identity),
+        registry_(std::move(registry)) {}
+  ~ConnectionFilterImpl() override {}
+
+ private:
+  // ConnectionFilter:
+  bool OnConnect(shell::Connection* connection,
+                 shell::Connector* connector) override {
+    // We only fulfill connections from the renderer we host.
+    const shell::Identity& remote_identity = connection->GetRemoteIdentity();
+    if (child_identity_.name() != remote_identity.name() ||

[Ken Rockot(use gerrit already), 2016/07/29 17:09:45]

Could we just add a != operator to Identity? Shouldn't the user_id also be
checked?

[Ben Goodger (Google), 2016/07/29 19:35:02]

Can't use user_id since user_id for outbound connections is kInheritUserId

+        child_identity_.instance() != remote_identity.instance()) {
+      return false;
+    }
+
+    std::set<std::string> interface_names;
+    registry_->GetInterfaceNames(&interface_names);
+    for (auto& interface_name : interface_names) {
+      connection->GetInterfaceRegistry()->AddInterface(
+          interface_name,
+          base::Bind(&ConnectionFilterImpl::GetInterface,
+                     base::Unretained(this),

[Ken Rockot(use gerrit already), 2016/07/29 17:09:46]

This is not safe. The registered callbacks may call into the
ConnectionFilterImpl as long as the Connection (and thus the IOThreadContext of
the MojoChildConnection) is alive. That can live arbitrarily longer than the
RPHI which created it, since it lives on the IO thread.

I think we need to use WeakPtrs here.

+                     interface_name));
+    }
+    return true;
+  }
+
+  void GetInterface(const std::string& interface_name,
+                    mojo::ScopedMessagePipeHandle handle) {
+    shell::mojom::InterfaceProvider* provider = registry_.get();
+    provider->GetInterface(interface_name, std::move(handle));
+  }
+
+  shell::Identity child_identity_;
+  std::unique_ptr<shell::InterfaceRegistry> registry_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConnectionFilterImpl);
+};
+
 base::MessageLoop*
 RenderProcessHostImpl::GetInProcessRendererThreadForTesting() {
   return g_in_process_thread;
 }
 
-// Stores the maximum number of renderer processes the content module can
-// create.
-static size_t g_max_renderer_count_override = 0;
-
 // static
 size_t RenderProcessHost::GetMaxRendererProcessCount() {
   if (g_max_renderer_count_override)
     return g_max_renderer_count_override;
 
 #if defined(OS_ANDROID)
   // On Android we don't maintain a limit of renderer process hosts - we are
   // happy with keeping a lot of these, as long as the number of live renderer
   // processes remains reasonable, and on Android the OS takes care of that.
   return std::numeric_limits<size_t>::max();
@@ skipping 27 matching lines @@
     max_count /= kEstimatedWebContentsMemoryUsage;
 
     const size_t kMinRendererProcessCount = 3;
     max_count = std::max(max_count, kMinRendererProcessCount);
     max_count = std::min(max_count, kMaxRendererProcessCount);
   }
   return max_count;
 }
 
 // static
-bool g_run_renderer_in_process_ = false;
-
-// static
 void RenderProcessHost::SetMaxRendererProcessCount(size_t count) {
   g_max_renderer_count_override = count;
 }
 
 #if defined(OS_POSIX) && !defined(OS_ANDROID) && !defined(OS_MACOSX)
 // static
 void RenderProcessHostImpl::EarlyZygoteLaunch() {
   DCHECK(!g_render_zygote);
   // TODO(kerrnel): Investigate doing this without the ZygoteHostImpl as a
   // proxy. It is currently done this way due to concerns about race
@@ skipping 217 matching lines @@
 #endif  // !defined(OS_MACOSX)
 
 #if defined(OS_ANDROID)
   // Initialize the java audio manager so that media session tests will pass.
   // See internal b/29872494.
   static_cast<media::AudioManagerAndroid*>(media::AudioManager::Get())->
       InitializeIfNeeded();
 #endif  // defined(OS_ANDROID)
 
   CreateMessageFilters();
-  RegisterMojoInterfaces();
+  std::unique_ptr<shell::InterfaceRegistry> registry(

[Ken Rockot(use gerrit already), 2016/07/29 17:09:45]

Can't you just put all this junk in RegisterMojoInterfaces() with no argument?

+      new shell::InterfaceRegistry(nullptr));
+#if defined(OS_ANDROID)
+  interface_registry_android_ =
+      InterfaceRegistryAndroid::Create(registry.get());
+  InterfaceRegistrarAndroid::ExposeInterfacesToRenderer(
+      interface_registry_android_.get());
+#endif
+
+  RegisterMojoInterfaces(registry.get());
+  MojoShellConnection* mojo_shell_connection =
+      BrowserContext::GetMojoShellConnectionFor(browser_context_);
+  std::unique_ptr<ConnectionFilterImpl> connection_filter(
+      new ConnectionFilterImpl(mojo_child_connection_->child_identity(),
+                               std::move(registry)));
+  connection_filter_ = connection_filter.get();
+  mojo_shell_connection->AddConnectionFilter(std::move(connection_filter));
 
   if (run_renderer_in_process()) {
     DCHECK(g_renderer_main_thread_factory);
     // Crank up a thread and run the initialization there.  With the way that
     // messages flow between the browser and renderer, this thread is required
     // to prevent a deadlock in single-process mode.  Since the primordial
     // thread in the renderer process runs the WebKit code and can sometimes
     // make blocking calls to the UI thread (i.e. this thread), they need to run
     // on separate threads.
     in_process_renderer_.reset(
@@ skipping 277 matching lines @@
   AddFilter(new ProfilerMessageFilter(PROCESS_TYPE_RENDERER));
   AddFilter(new HistogramMessageFilter());
   AddFilter(new MemoryMessageFilter(this));
   AddFilter(new PushMessagingMessageFilter(
       GetID(), storage_partition_impl_->GetServiceWorkerContext()));
 #if defined(OS_ANDROID)
   AddFilter(new ScreenOrientationMessageFilterAndroid());
 #endif
 }
 
-void RenderProcessHostImpl::RegisterMojoInterfaces() {
+void RenderProcessHostImpl::RegisterMojoInterfaces(
+    shell::InterfaceRegistry* registry) {
+  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner =
+      BrowserThread::GetTaskRunnerForThread(BrowserThread::UI);
 #if !defined(OS_ANDROID)
-  GetInterfaceRegistry()->AddInterface(
-      base::Bind(&device::BatteryMonitorImpl::Create));
+  registry->AddInterface(base::Bind(&device::BatteryMonitorImpl::Create),
+                         ui_task_runner);

[Ken Rockot(use gerrit already), 2016/07/29 17:09:45]

It's pretty unfortunate that we have to add ui_task_runner arguments to so many
AddInterface calls. I guess I don't have any better ideas for now.

 #endif
-
-  GetInterfaceRegistry()->AddInterface(
+  registry->AddInterface(
       base::Bind(&PermissionServiceContext::CreateService,
-                 base::Unretained(permission_service_context_.get())));
-
+                 base::Unretained(permission_service_context_.get())),
+      ui_task_runner);
   // TODO(mcasas): finalize arguments.
-  GetInterfaceRegistry()->AddInterface(
-      base::Bind(&ImageCaptureImpl::Create));
-
-  GetInterfaceRegistry()->AddInterface(
-      base::Bind(&OffscreenCanvasSurfaceImpl::Create));
-
-  GetInterfaceRegistry()->AddInterface(base::Bind(
-      &BackgroundSyncContext::CreateService,
-      base::Unretained(storage_partition_impl_->GetBackgroundSyncContext())));
-
-  GetInterfaceRegistry()->AddInterface(base::Bind(
-      &PlatformNotificationContextImpl::CreateService,
-      base::Unretained(
-          storage_partition_impl_->GetPlatformNotificationContext()), GetID()));
-
-  GetInterfaceRegistry()->AddInterface(
+  registry->AddInterface(base::Bind(&ImageCaptureImpl::Create),
+                         ui_task_runner);
+  registry->AddInterface(base::Bind(&OffscreenCanvasSurfaceImpl::Create),
+                         ui_task_runner);
+  registry->AddInterface(
+      base::Bind(&BackgroundSyncContext::CreateService,
+                 base::Unretained(
+                     storage_partition_impl_->GetBackgroundSyncContext())),
+      ui_task_runner);
+  registry->AddInterface(
+      base::Bind(&PlatformNotificationContextImpl::CreateService,
+                 base::Unretained(
+                     storage_partition_impl_->GetPlatformNotificationContext()),
+                 GetID()),
+      ui_task_runner);
+  registry->AddInterface(
       base::Bind(&RenderProcessHostImpl::CreateStoragePartitionService,
-                 base::Unretained(this)));
-
-  GetInterfaceRegistry()->AddInterface(
+                 base::Unretained(this)),
+      ui_task_runner);
+  registry->AddInterface(
       base::Bind(&BroadcastChannelProvider::Connect,
                  base::Unretained(
-                     storage_partition_impl_->GetBroadcastChannelProvider())));
+                     storage_partition_impl_->GetBroadcastChannelProvider())),
+      ui_task_runner);
+  if (memory_coordinator::IsEnabled()) {
+    registry->AddInterface(base::Bind(&CreateMemoryCoordinatorHandle, GetID()),
+                           ui_task_runner);
+  }
 
   scoped_refptr<base::SingleThreadTaskRunner> file_task_runner =
       BrowserThread::GetTaskRunnerForThread(BrowserThread::FILE);
-  GetInterfaceRegistry()->AddInterface(
-      base::Bind(&MimeRegistryImpl::Create), file_task_runner);
-
+  registry->AddInterface(base::Bind(&MimeRegistryImpl::Create),
+                         file_task_runner);
 #if defined(USE_MINIKIN_HYPHENATION)
-  GetInterfaceRegistry()->AddInterface(
-      base::Bind(&hyphenation::HyphenationImpl::Create), file_task_runner);
+  registry->AddInterface(base::Bind(&hyphenation::HyphenationImpl::Create),
+                         file_task_runner);
 #endif
 
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner =
       BrowserThread::GetTaskRunnerForThread(BrowserThread::IO);
-  GetInterfaceRegistry()->AddInterface(base::Bind(&DeviceLightHost::Create),
-                                       io_task_runner);
-  GetInterfaceRegistry()->AddInterface(base::Bind(&DeviceMotionHost::Create),
-                                       io_task_runner);
-  GetInterfaceRegistry()->AddInterface(
-      base::Bind(&DeviceOrientationHost::Create), io_task_runner);
-  GetInterfaceRegistry()->AddInterface(
-      base::Bind(&DeviceOrientationAbsoluteHost::Create), io_task_runner);
+  registry->AddInterface(base::Bind(&DeviceLightHost::Create), io_task_runner);

[Ken Rockot(use gerrit already), 2016/07/29 17:09:45]

io_task_runner isn't necessary now, it just needlessly defers the binding

+  registry->AddInterface(base::Bind(&DeviceMotionHost::Create), io_task_runner);
+  registry->AddInterface(base::Bind(&DeviceOrientationHost::Create),
+                         io_task_runner);
+  registry->AddInterface(base::Bind(&DeviceOrientationAbsoluteHost::Create),
+                         io_task_runner);
 
-  if (memory_coordinator::IsEnabled()) {
-    GetInterfaceRegistry()->AddInterface(
-        base::Bind(&CreateMemoryCoordinatorHandle, GetID()));
-  }
-
-#if defined(OS_ANDROID)
-  InterfaceRegistrarAndroid::ExposeInterfacesToRenderer(
-      mojo_child_connection_->interface_registry_android());
-#endif
-
-  GetContentClient()->browser()->ExposeInterfacesToRenderer(
-      GetInterfaceRegistry(), this);
+  GetContentClient()->browser()->ExposeInterfacesToRenderer(registry, this);
 }
 
 void RenderProcessHostImpl::CreateStoragePartitionService(
     mojo::InterfaceRequest<mojom::StoragePartitionService> request) {
   // DO NOT REMOVE THIS COMMAND LINE CHECK WITHOUT SECURITY REVIEW!
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kMojoLocalStorage)) {
     storage_partition_impl_->Bind(std::move(request));
   }
 }
@@ skipping 785 matching lines @@
           RenderProcessHostObserver, observers_,
           RenderProcessExited(this, base::TERMINATION_STATUS_NORMAL_TERMINATION,
                               0));
     }
     FOR_EACH_OBSERVER(RenderProcessHostObserver, observers_,
                       RenderProcessHostDestroyed(this));
     NotificationService::current()->Notify(
         NOTIFICATION_RENDERER_PROCESS_TERMINATED,
         Source<RenderProcessHost>(this), NotificationService::NoDetails());
 
+    if (connection_filter_) {
+      MojoShellConnection* mojo_shell_connection =
+          BrowserContext::GetMojoShellConnectionFor(browser_context_);
+      // Throw the result away, so the connection filter is deleted.
+      mojo_shell_connection->RemoveConnectionFilter(connection_filter_);
+    }
+
 #ifndef NDEBUG
     is_self_deleted_ = true;
 #endif
     base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
     deleting_soon_ = true;
 
 #if USE_ATTACHMENT_BROKER
     IPC::AttachmentBroker::GetGlobal()->DeregisterCommunicationChannel(
         channel_.get());
 #endif
@@ skipping 829 matching lines @@
   LOG(ERROR) << "Terminating render process for bad Mojo message: " << error;
 
   // The ReceivedBadMessage call below will trigger a DumpWithoutCrashing. Alias
   // enough information here so that we can determine what the bad message was.
   base::debug::Alias(&error);
   bad_message::ReceivedBadMessage(process.get(),
                                   bad_message::RPH_MOJO_PROCESS_ERROR);
 }
 
 }  // namespace content