Add an IPC channel between the NaCl loader process and the renderer.

webkit/plugins/ppapi/plugin_module.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 "webkit/plugins/ppapi/plugin_module.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 401 matching lines @@
                            const FilePath& path,
                            PluginDelegate::ModuleLifetime* lifetime_delegate)
     : lifetime_delegate_(lifetime_delegate),
       callback_tracker_(new ::ppapi::CallbackTracker),
       is_in_destructor_(false),
       is_crashed_(false),
       broker_(NULL),
       library_(NULL),
       name_(name),
       path_(path),
-      reserve_instance_id_(NULL) {
+      reserve_instance_id_(NULL),
+      nacl_ipc_proxy_(false) {
   // Ensure the globals object is created.
   if (!host_globals)
     host_globals = new HostGlobals;
 
   memset(&entry_points_, 0, sizeof(entry_points_));
   pp_module_ = HostGlobals::Get()->AddModule(this);
   GetMainThreadMessageLoop();  // Initialize the main thread message loop.
   GetLivePluginSet()->insert(this);
 }
 
@@ skipping 56 matching lines @@
   return true;
 }
 
 void PluginModule::InitAsProxied(
     PluginDelegate::OutOfProcessProxy* out_of_process_proxy) {
   DCHECK(!out_of_process_proxy_.get());
   out_of_process_proxy_.reset(out_of_process_proxy);
 }
 
 void PluginModule::InitAsProxiedNaCl(
-    PluginDelegate::OutOfProcessProxy* out_of_process_proxy,
+    scoped_ptr<PluginDelegate::OutOfProcessProxy> out_of_process_proxy,
     PP_Instance instance) {
-  InitAsProxied(out_of_process_proxy);
+  nacl_ipc_proxy_ = true;
+  InitAsProxied(out_of_process_proxy.release());
   // InitAsProxied (for the trusted/out-of-process case) initializes only the
   // module, and one or more instances are added later. In this case, the
   // PluginInstance was already created as in-process, so we missed the proxy
   // AddInstance step and must do it now.
   out_of_process_proxy_->AddInstance(instance);
 
   // In NaCl, we need to tell the instance to reset itself as proxied. This will
   // clear cached interface pointers and send DidCreate (etc) to the plugin
   // side of the proxy.
   PluginInstance* plugin_instance = host_globals->GetInstance(instance);
@@ skipping 41 matching lines @@
 }
 
 void PluginModule::InstanceCreated(PluginInstance* instance) {
   instances_.insert(instance);
 }
 
 void PluginModule::InstanceDeleted(PluginInstance* instance) {
   if (out_of_process_proxy_.get())
     out_of_process_proxy_->RemoveInstance(instance->pp_instance());
   instances_.erase(instance);
+
+  if (nacl_ipc_proxy_) {
+    out_of_process_proxy_.reset();
+    reserve_instance_id_ = NULL;
+  }

[dmichael (off chromium), 2012/06/21 18:18:16]

As we discussed, this is not going to work for multiple instances...  I'd argue
that it needs a TODO at least.

I'd also prefer if you don't add any more features to this CL, and save that
stuff for a later CL, so we can get it landed. Having to understand a bunch of
new stuff slows down the review cycle.

[bbudge, 2012/06/21 19:28:18]

I'll add a TODO for this. PluginModule is already broken for multiple instances
and this just makes things behave better in the single instance case. Sorry the
patch is growing. It's a definite improvement when playing with the new proxy as
you can test multiple nexes now.

 }
 
 scoped_refptr< ::ppapi::CallbackTracker> PluginModule::GetCallbackTracker() {
   return callback_tracker_;
 }
 
 void PluginModule::PluginCrashed() {
   DCHECK(!is_crashed_);  // Should only get one notification.
   is_crashed_ = true;
 
   // Notify all instances that they crashed.
   for (PluginInstanceSet::iterator i = instances_.begin();
        i != instances_.end(); ++i)
     (*i)->InstanceCrashed();
 
   if (lifetime_delegate_)
     lifetime_delegate_->PluginModuleDead(this);
 }
 
 void PluginModule::SetReserveInstanceIDCallback(
     PP_Bool (*reserve)(PP_Module, PP_Instance)) {
-  DCHECK(!reserve_instance_id_) << "Only expect one set.";
   reserve_instance_id_ = reserve;
 }
 
 bool PluginModule::ReserveInstanceID(PP_Instance instance) {
+  DCHECK(!reserve_instance_id_) << "Only expect one set.";

[dmichael (off chromium), 2012/06/21 18:18:16]

Why did you move this check? It doesn't make sense here to me.

[bbudge, 2012/06/21 19:28:18]

The HostDispatcher sets it and it can be set multiple times now. Discussed this
with Brett.

[dmichael (off chromium), 2012/06/21 19:42:30]

So that would argue for simply removing the check. It doesn't make sense here.
You're now DCHECKing that you do not have a reserve_instance_id function, when
in the out-of-process case, you need it. Your OOP tests are all crashing on the
try run.

   if (reserve_instance_id_)
     return PPBoolToBool(reserve_instance_id_(pp_module_, instance));
   return true;  // Instance ID is usable.
 }
 
 void PluginModule::SetBroker(PluginDelegate::Broker* broker) {
   DCHECK(!broker_ || !broker);
   broker_ = broker;
 }
 
 PluginDelegate::Broker* PluginModule::GetBroker() {
   return broker_;
 }
 
 bool PluginModule::InitializeModule(const EntryPoints& entry_points) {
   DCHECK(!out_of_process_proxy_.get()) << "Don't call for proxied modules.";
   DCHECK(entry_points.initialize_module != NULL);
   int retval = entry_points.initialize_module(pp_module(), &GetInterface);
   if (retval != 0) {
     LOG(WARNING) << "PPP_InitializeModule returned failure " << retval;
     return false;
   }
   return true;
 }
 
 }  // namespace ppapi
 }  // namespace webkit