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

content/renderer/pepper/ppb_broker_impl.cc

Index: content/renderer/pepper/ppb_broker_impl.cc
===================================================================
--- content/renderer/pepper/ppb_broker_impl.cc	(revision 215720)
+++ content/renderer/pepper/ppb_broker_impl.cc	(working copy)
@@ -5,12 +5,15 @@
 #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"
@@ -29,7 +32,9 @@
     : 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() {
@@ -40,6 +45,7 @@
 
   // The plugin owns the handle.
   pipe_handle_ = PlatformFileToInt(base::kInvalidPlatformFileValue);
+  ChildThread::current()->RemoveRoute(routing_id_);
 }
 
 PPB_Broker_API* PPB_Broker_Impl::AsPPB_Broker_API() {
@@ -59,6 +65,8 @@
       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.
@@ -66,12 +74,26 @@
   // 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;
 }
 
@@ -103,4 +125,26 @@
   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