Track NPObject ownership by the originating plugins' NPP identifier. [2/3] (Chrome)

webkit/plugins/npapi/webplugin_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 "webkit/plugins/npapi/webplugin_impl.h"
 
 #include "base/bind.h"
 #include "base/debug/crash_logging.h"
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
@@ skipping 225 matching lines @@
   if (!page_delegate_) {
     LOG(ERROR) << "No page delegate";
     return false;
   }
 
   WebPluginDelegate* plugin_delegate = page_delegate_->CreatePluginDelegate(
       file_path_, mime_type_);
   if (!plugin_delegate)
     return false;
 
+  // Store the plugin's unique identifier, used by the container to track its
+  // script objects.
+  npp_ = plugin_delegate->GetPluginNPP();
+
   // Set the container before Initialize because the plugin may
-  // synchronously call NPN_GetValue to get its container during its
-  // initialization.
+  // synchronously call NPN_GetValue to get its container, or make calls
+  // passing script objects that need to be tracked, during initialization.
   SetContainer(container);
+
   bool ok = plugin_delegate->Initialize(
       plugin_url_, arg_names_, arg_values_, this, load_manually_);
   if (!ok) {
     LOG(ERROR) << "Couldn't initialize plug-in";
     plugin_delegate->PluginDestroyed();
 
     WebKit::WebPlugin* replacement_plugin =
         page_delegate_->CreatePluginReplacement(file_path_);
     if (!replacement_plugin || !replacement_plugin->initialize(container))
       return false;
@@ skipping 13 matching lines @@
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
 NPObject* WebPluginImpl::scriptableObject() {
   if (!delegate_)
     return NULL;
 
   return delegate_->GetPluginScriptableObject();
 }
 
+NPP WebPluginImpl::pluginNPP() {
+  CHECK(npp_);
+  // if (!delegate_)
+  //   return NULL;
+
+  return npp_; // delegate_->GetPluginNPP();

[jamesr, 2013/05/15 22:32:15]

hmm? what's up with the commented-out code here?

[Wez, 2013/05/15 23:14:16]

Sorry, looks like this patch-set is stale. :(

+}
+
 bool WebPluginImpl::getFormValue(WebKit::WebString& value) {
   if (!delegate_)
     return false;
   base::string16 form_value;
   if (!delegate_->GetFormValue(&form_value))
     return false;
   value = form_value;
   return true;
 }
 
@@ skipping 175 matching lines @@
     const WebPluginParams& params,
     const base::FilePath& file_path,
     const base::WeakPtr<WebPluginPageDelegate>& page_delegate)
     : windowless_(false),
       window_(gfx::kNullPluginWindow),
       accepts_input_events_(false),
       page_delegate_(page_delegate),
       webframe_(webframe),
       delegate_(NULL),
       container_(NULL),
+      npp_(NULL),
       plugin_url_(params.url),
       load_manually_(params.loadManually),
       first_geometry_update_(true),
       ignore_response_error_(false),
       file_path_(file_path),
       mime_type_(UTF16ToASCII(params.mimeType)),
       weak_factory_(this) {
   DCHECK_EQ(params.attributeNames.size(), params.attributeValues.size());
   StringToLowerASCII(&mime_type_);
 
@@ skipping 551 matching lines @@
       RemoveClient(i);
       return;
     }
   }
 }
 
 void WebPluginImpl::SetContainer(WebPluginContainer* container) {
   if (!container)
     TearDownPluginInstance(NULL);
   container_ = container;
+  if (container_)
+    container_->allowScriptObjects();
 }
 
 void WebPluginImpl::HandleURLRequest(const char* url,
                                      const char* method,
                                      const char* target,
                                      const char* buf,
                                      unsigned int len,
                                      int notify_id,
                                      bool popups_allowed,
                                      bool notify_redirects) {
@@ skipping 258 matching lines @@
 
   // The plugin move sequences accumulated via DidMove are sent to the browser
   // whenever the renderer paints. Force a paint here to ensure that changes
   // to the plugin window are propagated to the browser.
   container_->invalidate();
   return true;
 }
 
 void WebPluginImpl::TearDownPluginInstance(
     WebURLLoader* loader_to_ignore) {
-  // The container maintains a list of JSObjects which are related to this
-  // plugin.  Tell the frame we're gone so that it can invalidate all of
-  // those sub JSObjects.
+  // JavaScript garbage collection may cause plugin script object references to
+  // be retained long after the plugin is destroyed. Some plugins won't cope
+  // with their objects being released after they've been destroyed, and once
+  // we've actually unloaded the plugin the object's releaseobject() code may
+  // no longer be in memory. The container tracks the plugin's objects and lets
+  // us invalidate them, releasing the references to them held by the JavaScript
+  // runtime.
   if (container_) {
     container_->clearScriptObjects();
     container_->setWebLayer(NULL);
   }
 
+  // Call PluginDestroyed() first to prevent the plugin from calling us back
+  // in the middle of tearing down the render tree.
   if (delegate_) {
-    // Call PluginDestroyed() first to prevent the plugin from calling us back
-    // in the middle of tearing down the render tree.
+    // The plugin may call into the browser and pass script objects even during
+    // teardown, so temporarily re-enable plugin script objects.
+    DCHECK(container_);
+    container_->allowScriptObjects();
+
     delegate_->PluginDestroyed();
     delegate_ = NULL;
+
+    // Invalidate any script objects created during teardown here, before the
+    // plugin might actually be unloaded.
+    container_->clearScriptObjects();
   }
 
   // Cancel any pending requests because otherwise this deleted object will
   // be called by the ResourceDispatcher.
   std::vector<ClientInfo>::iterator client_index = clients_.begin();
   while (client_index != clients_.end()) {
     ClientInfo& client_info = *client_index;
 
     if (loader_to_ignore == client_info.loader) {
       client_index++;
@@ skipping 23 matching lines @@
       webframe_->setReferrerForRequest(*request, plugin_url_);
       break;
 
     default:
       break;
   }
 }
 
 }  // namespace npapi
 }  // namespace webkit