Maintain a map of all resources in the resource tracker and clear instance back pointers when needed,

BUG=85808
Review URL: http://codereview.chromium.org/7196001

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

7 files changed, 62 insertions, 31 deletions

diff --git a/webkit/plugins/ppapi/ppb_url_loader_impl.cc b/webkit/plugins/ppapi/ppb_url_loader_impl.cc
index 3b57d12..103ea5c 100644
--- a/webkit/plugins/ppapi/ppb_url_loader_impl.cc
+++ b/webkit/plugins/ppapi/ppb_url_loader_impl.cc
@@ -86,13 +86,9 @@ PPB_URLLoader_API* PPB_URLLoader_Impl::AsPPB_URLLoader_API() {
   return this;
 }
 
-void PPB_URLLoader_Impl::LastPluginRefWasDeleted(bool instance_destroyed) {
-  Resource::LastPluginRefWasDeleted(instance_destroyed);
-  if (instance_destroyed) {
-    // Free the WebKit request when the instance has been destroyed to avoid
-    // using bandwidth just in case this object lives longer than the instance.
-    loader_.reset();
-  }
+void PPB_URLLoader_Impl::ClearInstance() {
+  Resource::ClearInstance();
+  loader_.reset();
 }
 
 int32_t PPB_URLLoader_Impl::Open(PP_Resource request_id,
diff --git a/webkit/plugins/ppapi/ppb_url_loader_impl.h b/webkit/plugins/ppapi/ppb_url_loader_impl.h
index f931406..e2794e0 100644
--- a/webkit/plugins/ppapi/ppb_url_loader_impl.h
+++ b/webkit/plugins/ppapi/ppb_url_loader_impl.h
@@ -42,7 +42,7 @@ class PPB_URLLoader_Impl : public Resource,
   virtual ::ppapi::thunk::PPB_URLLoader_API* AsPPB_URLLoader_API() OVERRIDE;
 
   // Resource overrides.
-  virtual void LastPluginRefWasDeleted(bool instance_destroyed) OVERRIDE;
+  virtual void ClearInstance() OVERRIDE;
 
   // PPB_URLLoader_API implementation.
   virtual int32_t Open(PP_Resource request_id,
diff --git a/webkit/plugins/ppapi/ppb_widget_impl.cc b/webkit/plugins/ppapi/ppb_widget_impl.cc
index 300955d..c079a74 100644
--- a/webkit/plugins/ppapi/ppb_widget_impl.cc
+++ b/webkit/plugins/ppapi/ppb_widget_impl.cc
@@ -95,6 +95,8 @@ void PPB_Widget_Impl::SetLocation(const PP_Rect* location) {
 }
 
 void PPB_Widget_Impl::Invalidate(const PP_Rect* dirty) {
+  if (!instance())
+    return;
   const PPP_Widget_Dev* widget = static_cast<const PPP_Widget_Dev*>(
       instance()->module()->GetPluginInterface(PPP_WIDGET_DEV_INTERFACE));
   if (!widget)
diff --git a/webkit/plugins/ppapi/resource.cc b/webkit/plugins/ppapi/resource.cc
index 9619861..bb9ad4d 100644
--- a/webkit/plugins/ppapi/resource.cc
+++ b/webkit/plugins/ppapi/resource.cc
@@ -15,9 +15,11 @@ namespace ppapi {
 
 Resource::Resource(PluginInstance* instance)
     : resource_id_(0), instance_(instance) {
+  ResourceTracker::Get()->ResourceCreated(this, instance_);
 }
 
 Resource::~Resource() {
+  ResourceTracker::Get()->ResourceDestroyed(this);
 }
 
 PP_Resource Resource::GetReference() {
@@ -33,14 +35,11 @@ PP_Resource Resource::GetReferenceNoAddRef() const {
   return resource_id_;
 }
 
-void Resource::LastPluginRefWasDeleted(bool instance_destroyed) {
+void Resource::LastPluginRefWasDeleted() {
   DCHECK(resource_id_ != 0);
   instance()->module()->GetCallbackTracker()->PostAbortForResource(
       resource_id_);
   resource_id_ = 0;
-
-  if (instance_destroyed)
-    instance_ = NULL;
 }
 
 #define DEFINE_TYPE_GETTER(RESOURCE)            \
diff --git a/webkit/plugins/ppapi/resource.h b/webkit/plugins/ppapi/resource.h
index 0f86fa1..0d3b99e 100644
--- a/webkit/plugins/ppapi/resource.h
+++ b/webkit/plugins/ppapi/resource.h
@@ -67,6 +67,12 @@ class Resource : public base::RefCountedThreadSafe<Resource>,
   // PPAPI implementation is keeping a reference for some reason.
   PluginInstance* instance() const { return instance_; }
 
+  // Clears the instance pointer when the associated PluginInstance will be
+  // destroyed.
+  //
+  // If you override this, be sure to call the base class' implementation.
+  virtual void ClearInstance() { instance_ = NULL; }
+
   // Cast the resource into a specified type. This will return NULL if the
   // resource does not match the specified type. Specializations of this
   // template call into As* functions.
@@ -110,16 +116,8 @@ class Resource : public base::RefCountedThreadSafe<Resource>,
   // stay alive if there are other references held by the PPAPI implementation
   // (possibly for callbacks and things).
   //
-  // When the plugin instance is deleted, all resources associated with that
-  // plugin will have their plugin references force-deleted and this function
-  // will be called with instance_destroyed as true. If the plugin normally
-  // Release()s a reference before the instance is destroyed,
-  // instance_destroyed will be false. It's possible in some rare cases for the
-  // plugin to get a new reference to the object in this latter case, if it's
-  // stored internal to the PPAPI implementation and returned by some function.
-  //
   // If you override this, be sure to call the base class' implementation.
-  virtual void LastPluginRefWasDeleted(bool instance_destroyed);
+  virtual void LastPluginRefWasDeleted();
 
  private:
   // Type-specific getters for individual resource types. These will return
diff --git a/webkit/plugins/ppapi/resource_tracker.cc b/webkit/plugins/ppapi/resource_tracker.cc
index 1787012..ed65a2a 100644
--- a/webkit/plugins/ppapi/resource_tracker.cc
+++ b/webkit/plugins/ppapi/resource_tracker.cc
@@ -66,7 +66,8 @@ struct ResourceTracker::InstanceData {
   PluginInstance* instance;
 
   // Resources and object vars associated with the instance.
-  ResourceSet resources;
+  ResourceSet ref_resources;
+  std::set<Resource*> assoc_resources;
   VarSet object_vars;
 
   // Lazily allocated function proxies for the different interfaces.
@@ -107,6 +108,27 @@ ResourceTracker* ResourceTracker::Get() {
   return global_tracker_;
 }
 
+void ResourceTracker::ResourceCreated(Resource* resource,
+                                      PluginInstance* instance) {
+  if (!instance)
+    return;
+  PP_Instance pp_instance = instance->pp_instance();
+  DCHECK(pp_instance);
+  DCHECK(instance_map_.find(pp_instance) != instance_map_.end());
+  instance_map_[pp_instance]->assoc_resources.insert(resource);
+}
+
+void ResourceTracker::ResourceDestroyed(Resource* resource) {
+  if (!resource->instance())
+    return;
+
+  PP_Instance pp_instance = resource->instance()->pp_instance();
+  DCHECK(pp_instance);
+  DCHECK(instance_map_.find(pp_instance) != instance_map_.end());
+
+  instance_map_[pp_instance]->assoc_resources.erase(resource);
+}
+
 PP_Resource ResourceTracker::AddResource(Resource* resource) {
   // If the plugin manages to create 1 billion resources, don't do crazy stuff.
   if (last_resource_id_ ==
@@ -120,7 +142,7 @@ PP_Resource ResourceTracker::AddResource(Resource* resource) {
   // Track associated with the instance.
   PP_Instance pp_instance = resource->instance()->pp_instance();
   DCHECK(instance_map_.find(pp_instance) != instance_map_.end());
-  instance_map_[pp_instance]->resources.insert(new_id);
+  instance_map_[pp_instance]->ref_resources.insert(new_id);
   return new_id;
 }
 
@@ -169,9 +191,9 @@ bool ResourceTracker::UnrefResource(PP_Resource res) {
       // LastPluginRefWasDeleted will clear the instance pointer, so save it
       // first.
       PP_Instance instance = to_release->instance()->pp_instance();
-      to_release->LastPluginRefWasDeleted(false);
+      to_release->LastPluginRefWasDeleted();
 
-      instance_map_[instance]->resources.erase(res);
+      instance_map_[instance]->ref_resources.erase(res);
       live_resources_.erase(i);
     }
     return true;
@@ -193,8 +215,8 @@ void ResourceTracker::CleanupInstanceData(PP_Instance instance,
 
   // Force release all plugin references to resources associated with the
   // deleted instance.
-  ResourceSet::iterator cur_res = data.resources.begin();
-  while (cur_res != data.resources.end()) {
+  ResourceSet::iterator cur_res = data.ref_resources.begin();
+  while (cur_res != data.ref_resources.end()) {
     ResourceMap::iterator found_resource = live_resources_.find(*cur_res);
     if (found_resource == live_resources_.end()) {
       NOTREACHED();
@@ -203,7 +225,7 @@ void ResourceTracker::CleanupInstanceData(PP_Instance instance,
 
       // Must delete from the resource set first since the resource's instance
       // pointer will get zeroed out in LastPluginRefWasDeleted.
-      resource->LastPluginRefWasDeleted(true);
+      resource->LastPluginRefWasDeleted();
       live_resources_.erase(*cur_res);
     }
 
@@ -211,9 +233,9 @@ void ResourceTracker::CleanupInstanceData(PP_Instance instance,
     // are being deleted as long as we're careful not to delete the item we're
     // holding an iterator to.
     ResourceSet::iterator current = cur_res++;
-    data.resources.erase(current);
+    data.ref_resources.erase(current);
   }
-  DCHECK(data.resources.empty());
+  DCHECK(data.ref_resources.empty());
 
   // Force delete all var references.
   VarSet::iterator cur_var = data.object_vars.begin();
@@ -234,6 +256,13 @@ void ResourceTracker::CleanupInstanceData(PP_Instance instance,
   }
   DCHECK(data.object_vars.empty());
 
+  // Clear any resources that still reference this instance.
+  for (std::set<Resource*>::iterator res = data.assoc_resources.begin();
+       res != data.assoc_resources.end();
+       ++res)
+    (*res)->ClearInstance();
+  data.assoc_resources.clear();
+
   if (delete_instance)
     instance_map_.erase(found);
 }
@@ -243,7 +272,7 @@ uint32 ResourceTracker::GetLiveObjectsForInstance(
   InstanceMap::const_iterator found = instance_map_.find(instance);
   if (found == instance_map_.end())
     return 0;
-  return static_cast<uint32>(found->second->resources.size() +
+  return static_cast<uint32>(found->second->ref_resources.size() +
                              found->second->object_vars.size());
 }
 
diff --git a/webkit/plugins/ppapi/resource_tracker.h b/webkit/plugins/ppapi/resource_tracker.h
index d5618e6..b728895 100644
--- a/webkit/plugins/ppapi/resource_tracker.h
+++ b/webkit/plugins/ppapi/resource_tracker.h
@@ -120,6 +120,13 @@ class ResourceTracker : public ::ppapi::TrackerBase {
   ResourceTracker();
   ~ResourceTracker();
 
+  // Called when a new resource is created and associates it with its
+  // PluginInstance.
+  void ResourceCreated(Resource* resource, PluginInstance* instance);
+
+  // Removes a resource from the resource map.
+  void ResourceDestroyed(Resource* resource);
+
   // Adds the given resource to the tracker and assigns it a resource ID and
   // refcount of 1. The assigned resource ID will be returned. Used only by the
   // Resource class.