Move broker creation out of PepperHelperImpl to PPB_Broker_Impl in the effort to eliminate PepperHe…

content/renderer/pepper/ppb_broker_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.
 
 #include "content/renderer/pepper/ppb_broker_impl.h"
 
 #include "base/logging.h"
+#include "content/common/view_messages.h"
 #include "content/renderer/pepper/common.h"
 #include "content/renderer/pepper/host_globals.h"
 #include "content/renderer/pepper/pepper_broker.h"
 #include "content/renderer/pepper/pepper_helper_impl.h"
 #include "content/renderer/pepper/pepper_plugin_instance_impl.h"
 #include "content/renderer/pepper/plugin_module.h"
+#include "content/renderer/render_thread_impl.h"
+#include "content/renderer/render_view_impl.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/shared_impl/platform_file.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebElement.h"
 #include "third_party/WebKit/public/web/WebPluginContainer.h"
 
 using ppapi::PlatformFileToInt;
 using ppapi::thunk::PPB_Broker_API;
 using ppapi::TrackedCallback;
 
 namespace content {
 
 // PPB_Broker_Impl ------------------------------------------------------
 
 PPB_Broker_Impl::PPB_Broker_Impl(PP_Instance instance)
     : Resource(::ppapi::OBJECT_IS_IMPL, instance),
       broker_(NULL),
       connect_callback_(),
-      pipe_handle_(PlatformFileToInt(base::kInvalidPlatformFileValue)) {
+      pipe_handle_(PlatformFileToInt(base::kInvalidPlatformFileValue)),
+      routing_id_(RenderThreadImpl::current()->GenerateRoutingID()) {
+  ChildThread::current()->AddRoute(routing_id_, this);
 }
 
 PPB_Broker_Impl::~PPB_Broker_Impl() {
   if (broker_) {
     broker_->Disconnect(this);
     broker_ = NULL;
   }
 
   // The plugin owns the handle.
   pipe_handle_ = PlatformFileToInt(base::kInvalidPlatformFileValue);
+  ChildThread::current()->RemoveRoute(routing_id_);
 }
 
 PPB_Broker_API* PPB_Broker_Impl::AsPPB_Broker_API() {
   return this;
 }
 
 int32_t PPB_Broker_Impl::Connect(
     scoped_refptr<TrackedCallback> connect_callback) {
   // TODO(ddorwin): Return PP_ERROR_FAILED if plugin is in-process.
 
   if (broker_) {
     // May only be called once.
     return PP_ERROR_FAILED;
   }
 
   PepperPluginInstanceImpl* plugin_instance =
       HostGlobals::Get()->GetInstance(pp_instance());
   if (!plugin_instance)
     return PP_ERROR_FAILED;
+  PluginModule* module = plugin_instance->module();
+  const base::FilePath& broker_path = module->path();
 
   // The callback must be populated now in case we are connected to the broker
   // and BrokerConnected is called before ConnectToBroker returns.
   // Because it must be created now, it must be aborted and cleared if
   // ConnectToBroker fails.
   connect_callback_ = connect_callback;
 
-  broker_ = plugin_instance->helper()->ConnectToBroker(this);
+  broker_ = module->GetBroker();
   if (!broker_) {
-    connect_callback_->Abort();
-    return PP_ERROR_FAILED;
+    broker_ = new PepperBroker(module);
+
+    // Have the browser start the broker process for us.
+    RenderThreadImpl::current()->Send(new ViewHostMsg_OpenChannelToPpapiBroker(
+        routing_id_, broker_path));
   }
 
+  RenderThreadImpl::current()->Send(
+      new ViewHostMsg_RequestPpapiBrokerPermission(
+          plugin_instance->render_view()->GetRoutingID(),
+          routing_id_,
+          GetDocumentUrl(),
+          broker_path));
+
+  // Adds a reference, ensuring that the broker is not deleted when
+  // |broker| goes out of scope.
+  broker_->AddPendingConnect(this);

[dmichael (off chromium), 2013/08/06 18:43:31]

The comment is not quite right now; broker_ is just a raw pointer, and isn't a
stack variable.

[jam, 2013/08/06 19:34:09]

I believe that comment (which I moved) is referring to the ref which is taken
inside the AddPendingConnect method

[dmichael(do not use this one), 2013/08/06 19:48:05]

The old comment referred to a stack variable named |broker| which was a
scoped_refptr, which held the only reference, so would cause a delete if
AddPendingConnect wasn't called before exiting scope. Now, there's nothing that
releases a reference on leaving scope, and broker is broker_, so the comment
should probably be something like:
// Adds a reference, ensuring that the broker is not deleted until
// we call Disconnect.

[jam, 2013/08/06 19:55:45]

ah, I missed that. i'll update in my next cl, thanks

+
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t PPB_Broker_Impl::GetHandle(int32_t* handle) {
   if (pipe_handle_ == PlatformFileToInt(base::kInvalidPlatformFileValue))
     return PP_ERROR_FAILED;  // Handle not set yet.
   *handle = pipe_handle_;
   return PP_OK;
 }
 
@@ skipping 11 matching lines @@
          handle == PlatformFileToInt(base::kInvalidPlatformFileValue));
 
   pipe_handle_ = handle;
 
   // Synchronous calls are not supported.
   DCHECK(TrackedCallback::IsPending(connect_callback_));
 
   connect_callback_->Run(result);
 }
 
+bool PPB_Broker_Impl::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(PPB_Broker_Impl, message)
+    IPC_MESSAGE_HANDLER(ViewMsg_PpapiBrokerChannelCreated,
+                        OnPpapiBrokerChannelCreated)
+    IPC_MESSAGE_HANDLER(ViewMsg_PpapiBrokerPermissionResult,
+                        OnPpapiBrokerPermissionResult)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void PPB_Broker_Impl::OnPpapiBrokerChannelCreated(
+    base::ProcessId broker_pid,
+    const IPC::ChannelHandle& handle) {
+  broker_->OnBrokerChannelConnected(broker_pid, handle);
+}
+
+void PPB_Broker_Impl::OnPpapiBrokerPermissionResult(bool result) {
+ broker_->OnBrokerPermissionResult(this, result);

[dmichael (off chromium), 2013/08/06 18:43:31]

nit: indentation is off

[jam, 2013/08/06 19:34:09]

Done.

+}
+
 }  // namespace content