Fix scripting during NPP_Destroy. Note that if the plugin is making a call t...

chrome/renderer/webplugin_delegate_proxy.cc

-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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 "chrome/renderer/webplugin_delegate_proxy.h"
 
 #include <algorithm>
 
 #include "app/gfx/canvas.h"
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
@@ skipping 146 matching lines @@
 WebPluginDelegateProxy::WebPluginDelegateProxy(
     const std::string& mime_type,
     const base::WeakPtr<RenderView>& render_view)
     : render_view_(render_view),
       plugin_(NULL),
       windowless_(false),
       window_(NULL),
       mime_type_(mime_type),
       instance_id_(MSG_ROUTING_NONE),
       npobject_(NULL),
-      window_script_object_(NULL),
       sad_plugin_(NULL),
       invalidate_pending_(false),
       transparent_(false),
       page_url_(render_view_->webview()->mainFrame()->url()) {
 }
 
 WebPluginDelegateProxy::~WebPluginDelegateProxy() {
 }
 
 void WebPluginDelegateProxy::PluginDestroyed() {
-  if (window_) {
+  if (window_)
     WillDestroyWindow();
-  }
-  plugin_ = NULL;

[jam, 2009/10/06 07:10:48]

setting plugin_ to null and calling RemoveRoute before calling the plugin were
done in http://chrome-corpsvn.mtv/viewvc?view=rev&root=chrome&revision=34289.  I
couldn't repro this crash, but I didn't use find any specific test cases. 
However I'm taking out the existing code and implementing the NPAPI functions
and will test with those changes.

-
-  if (npobject_) {
-    // When we destroy the plugin instance, the NPObjectStub NULLs out its
-    // pointer to the npobject (see NPObjectStub::OnChannelError).  Therefore,
-    // we release the object before destroying the instance to avoid leaking.
-    WebBindings::releaseObject(npobject_);

[jam, 2009/10/06 07:10:48]

this was added in
http://chrome-corpsvn.mtv/viewvc?view=rev&root=chrome&revision=32474 but isn't
needed anymore because of the corresponding change in
NPObjectStub::OnChannelError which stops nulling out npobject_.

-    npobject_ = NULL;
-  }
-
-  if (window_script_object_) {
-    // The ScriptController deallocates this object independent of its ref count
-    // to avoid leaks if the plugin forgets to release it.  So mark the object
-    // invalid to avoid accessing it past this point.
-    window_script_object_->set_proxy(NULL);
-    window_script_object_->set_invalid();
-  }
 
   if (channel_host_) {
+    Send(new PluginMsg_DestroyInstance(instance_id_));
+
+    // Must remove the route after sending the destroy message, since
+    // RemoveRoute can lead to all the outstanding NPObjects being told the
+    // channel went away if this was the last instance.
     channel_host_->RemoveRoute(instance_id_);
-    Send(new PluginMsg_DestroyInstance(instance_id_));
+
     // Release the channel host now. If we are is the last reference to the
     // channel, this avoids a race where this renderer asks a new connection to
     // the same plugin between now and the time 'this' is actually deleted.
     // Destroying the channel host is what releases the channel name -> FD
     // association on POSIX, and if we ask for a new connection before it is
     // released, the plugin will give us a new FD, and we'll assert when trying
     // to associate it with the channel name.
     channel_host_ = NULL;
   }
 
+  if (window_script_object_) {
+    // The ScriptController deallocates this object independent of its ref count
+    // to avoid leaks if the plugin forgets to release it.  So mark the object
+    // invalid to avoid accessing it past this point.  Note: only do this after
+    // the DestroyInstance message in case the window object is scripted by the
+    // plugin in NPP_Destroy.
+    window_script_object_->OnPluginDestroyed();
+  }
+
+  plugin_ = NULL;
+
   MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
 bool WebPluginDelegateProxy::Initialize(const GURL& url,
     const std::vector<std::string>& arg_names,
     const std::vector<std::string>& arg_values,
     webkit_glue::WebPlugin* plugin,
     bool load_manually) {
   IPC::ChannelHandle channel_handle;
   WebPluginInfo info;
@@ skipping 598 matching lines @@
   *success = false;
   NPObject* npobject = NULL;
   if (plugin_)
     npobject = plugin_->GetWindowScriptNPObject();
 
   if (!npobject)
     return;
 
   // The stub will delete itself when the proxy tells it that it's released, or
   // otherwise when the channel is closed.
-  NPObjectStub* stub = new NPObjectStub(
-      npobject, channel_host_.get(), route_id, 0, page_url_);
-  window_script_object_ = stub;
-  window_script_object_->set_proxy(this);
+  window_script_object_ = (new NPObjectStub(
+      npobject, channel_host_.get(), route_id, 0, page_url_))->AsWeakPtr();
   *success = true;
   *npobject_ptr = reinterpret_cast<intptr_t>(npobject);
 }
 
 void WebPluginDelegateProxy::OnGetPluginElement(
     int route_id, bool* success, intptr_t* npobject_ptr) {
   *success = false;
   NPObject* npobject = NULL;
   if (plugin_)
     npobject = plugin_->GetPluginElement();
@@ skipping 214 matching lines @@
     intptr_t existing_stream, bool notify_needed, intptr_t notify_data) {
   plugin_->InitiateHTTPRangeRequest(url.c_str(), range_info.c_str(),
                                     existing_stream, notify_needed,
                                     notify_data);
 }
 
 void WebPluginDelegateProxy::OnDeferResourceLoading(int resource_id,
                                                     bool defer) {
   plugin_->SetDeferResourceLoading(resource_id, defer);
 }