Cope gracefully with plugin being destroyed during NPObject Invoke or Evaluate.

BUG=89422,69934
TEST=See bug repro steps.

Review URL: http://codereview.chromium.org/7454003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@93456 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 28 insertions, 43 deletions

diff --git a/content/plugin/npobject_stub.cc b/content/plugin/npobject_stub.cc
index fb0d026..a191b81 100644
--- a/content/plugin/npobject_stub.cc
+++ b/content/plugin/npobject_stub.cc
@@ -36,9 +36,16 @@ NPObjectStub::NPObjectStub(
 
 NPObjectStub::~NPObjectStub() {
   channel_->RemoveRoute(route_id_);
+  CHECK(!npobject_);
+}
+
+void NPObjectStub::DeleteSoon(bool release_npobject) {
   if (npobject_) {
     channel_->RemoveMappingForNPObjectStub(route_id_, npobject_);
-    WebBindings::releaseObject(npobject_);
+    if (release_npobject)
+      WebBindings::releaseObject(npobject_);
+    npobject_ = NULL;
+    MessageLoop::current()->DeleteSoon(FROM_HERE, this);
   }
 }
 
@@ -46,18 +53,6 @@ bool NPObjectStub::Send(IPC::Message* msg) {
   return channel_->Send(msg);
 }
 
-void NPObjectStub::OnPluginDestroyed() {
-  channel_->RemoveMappingForNPObjectStub(route_id_, npobject_);
-  // We null out the underlying NPObject pointer since it's not valid anymore (
-  // ScriptController manually deleted the object).  As a result,
-  // OnMessageReceived won't dispatch any more messages.  Since this includes
-  // OnRelease, this object won't get deleted until OnChannelError which might
-  // not happen for a long time if this renderer process has a long lived
-  // plugin instance to the same process.  So we delete this object manually.
-  npobject_ = NULL;
-  MessageLoop::current()->DeleteSoon(FROM_HERE, this);
-}
-
 NPObject* NPObjectStub::GetUnderlyingNPObject() {
   return npobject_;
 }
@@ -68,7 +63,6 @@ IPC::Channel::Listener* NPObjectStub::GetChannelListener() {
 
 bool NPObjectStub::OnMessageReceived(const IPC::Message& msg) {
   content::GetContentClient()->SetActiveURL(page_url_);
-
   if (!npobject_) {
     if (msg.is_sync()) {
       // The object could be garbage because the frame has gone away, so
@@ -101,14 +95,12 @@ bool NPObjectStub::OnMessageReceived(const IPC::Message& msg) {
 }
 
 void NPObjectStub::OnChannelError() {
-  // If npobject_ is NULLed out, that means a DeleteSoon is happening.
-  if (npobject_)
-    delete this;
+  DeleteSoon(true);
 }
 
 void NPObjectStub::OnRelease(IPC::Message* reply_msg) {
   Send(reply_msg);
-  delete this;
+  DeleteSoon(true);
 }
 
 void NPObjectStub::OnHasMethod(const NPIdentifier_Param& name,
@@ -133,7 +125,6 @@ void NPObjectStub::OnInvoke(bool is_default,
                             const NPIdentifier_Param& method,
                             const std::vector<NPVariant_Param>& args,
                             IPC::Message* reply_msg) {
-  scoped_refptr<PluginChannelBase> local_channel = channel_;
   bool return_value = false;
   NPVariant_Param result_param;
   NPVariant result_var;
@@ -145,11 +136,11 @@ void NPObjectStub::OnInvoke(bool is_default,
   NPVariant* args_var = new NPVariant[arg_count];
   for (int i = 0; i < arg_count; ++i) {
     if (!CreateNPVariant(
-            args[i], local_channel, &(args_var[i]), containing_window_,
+            args[i], channel_, &(args_var[i]), containing_window_,
             page_url_)) {
       NPObjectMsg_Invoke::WriteReplyParams(reply_msg, result_param,
                                            return_value);
-      local_channel->Send(reply_msg);
+      channel_->Send(reply_msg);
       return;
     }
   }
@@ -187,10 +178,10 @@ void NPObjectStub::OnInvoke(bool is_default,
   delete[] args_var;
 
   CreateNPVariantParam(
-      result_var, local_channel, &result_param, true, containing_window_,
+      result_var, channel_, &result_param, true, containing_window_,
       page_url_);
   NPObjectMsg_Invoke::WriteReplyParams(reply_msg, result_param, return_value);
-  local_channel->Send(reply_msg);
+  channel_->Send(reply_msg);
 }
 
 void NPObjectStub::OnHasProperty(const NPIdentifier_Param& name,
@@ -328,7 +319,6 @@ void NPObjectStub::OnEnumeration(std::vector<NPIdentifier_Param>* value,
 
 void NPObjectStub::OnConstruct(const std::vector<NPVariant_Param>& args,
                                IPC::Message* reply_msg) {
-  scoped_refptr<PluginChannelBase> local_channel = channel_;
   bool return_value = false;
   NPVariant_Param result_param;
   NPVariant result_var;
@@ -339,11 +329,11 @@ void NPObjectStub::OnConstruct(const std::vector<NPVariant_Param>& args,
   NPVariant* args_var = new NPVariant[arg_count];
   for (int i = 0; i < arg_count; ++i) {
     if (!CreateNPVariant(
-           args[i], local_channel, &(args_var[i]), containing_window_,
+           args[i], channel_, &(args_var[i]), containing_window_,
            page_url_)) {
       NPObjectMsg_Invoke::WriteReplyParams(reply_msg, result_param,
                                            return_value);
-      local_channel->Send(reply_msg);
+      channel_->Send(reply_msg);
       return;
     }
   }
@@ -367,10 +357,10 @@ void NPObjectStub::OnConstruct(const std::vector<NPVariant_Param>& args,
   delete[] args_var;
 
   CreateNPVariantParam(
-      result_var, local_channel, &result_param, true, containing_window_,
+      result_var, channel_, &result_param, true, containing_window_,
       page_url_);
   NPObjectMsg_Invoke::WriteReplyParams(reply_msg, result_param, return_value);
-  local_channel->Send(reply_msg);
+  channel_->Send(reply_msg);
 }
 
 void NPObjectStub::OnEvaluate(const std::string& script,
@@ -381,12 +371,6 @@ void NPObjectStub::OnEvaluate(const std::string& script,
     return;
   }
 
-  // Grab a reference to the underlying channel, as the NPObjectStub
-  // instance can be destroyed in the context of NPN_Evaluate. This
-  // can happen if the containing plugin instance is destroyed in
-  // NPN_Evaluate.
-  scoped_refptr<PluginChannelBase> local_channel = channel_;
-
   NPVariant result_var;
   NPString script_string;
   script_string.UTF8Characters = script.c_str();
@@ -397,8 +381,7 @@ void NPObjectStub::OnEvaluate(const std::string& script,
 
   NPVariant_Param result_param;
   CreateNPVariantParam(
-      result_var, local_channel, &result_param, true, containing_window_,
-      page_url_);
+      result_var, channel_, &result_param, true, containing_window_, page_url_);
   NPObjectMsg_Evaluate::WriteReplyParams(reply_msg, result_param, return_value);
-  local_channel->Send(reply_msg);
+  channel_->Send(reply_msg);
 }
diff --git a/content/plugin/npobject_stub.h b/content/plugin/npobject_stub.h
index c100fe4..3584cfe 100644
--- a/content/plugin/npobject_stub.h
+++ b/content/plugin/npobject_stub.h
@@ -38,17 +38,19 @@ class NPObjectStub : public IPC::Channel::Listener,
                const GURL& page_url);
   virtual ~NPObjectStub();
 
+  // Cause the stub to ignore any further IPC messages, and to tear itself down
+  // the next time control returns to the message loop.
+  // The NPObject will be released only if |release_npobject| is true.
+  // This is used for the window script object stub in the renderer, which is
+  // freed with NPN_DeallocateObject to avoid leaks, and so we must not try to
+  // release it.
+  void DeleteSoon(bool release_npobject);
+
   // IPC::Message::Sender implementation:
   virtual bool Send(IPC::Message* msg);
 
-  // Called when the plugin widget that this NPObject came from is destroyed.
-  // This is needed because the renderer calls NPN_DeallocateObject on the
-  // window script object on destruction to avoid leaks.
-  void OnPluginDestroyed();
-
   // NPObjectBase implementation.
   virtual NPObject* GetUnderlyingNPObject();
-
   virtual IPC::Channel::Listener* GetChannelListener();
 
  private: