Simplify HostVarTracker handling of instance tear-down

- Rather than copy the NPObjectVar map to tear-down, iterate while !empty().
- Remove the SupportsWeakPtr base from NPObjectVar.


Review URL: https://chromiumcodereview.appspot.com/11578022

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

4 files changed, 13 insertions, 33 deletions

diff --git a/webkit/plugins/ppapi/host_var_tracker.cc b/webkit/plugins/ppapi/host_var_tracker.cc
index ed1b856..adeeaa1 100644
--- a/webkit/plugins/ppapi/host_var_tracker.cc
+++ b/webkit/plugins/ppapi/host_var_tracker.cc
@@ -43,8 +43,7 @@ void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
 
   DCHECK(np_object_map->find(object_var->np_object()) ==
          np_object_map->end()) << "NPObjectVar already in map";
-  np_object_map->insert(
-      std::make_pair(object_var->np_object(), object_var->AsWeakPtr()));
+  np_object_map->insert(std::make_pair(object_var->np_object(), object_var));
 }
 
 void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
@@ -69,10 +68,6 @@ void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
     return;
   }
   np_object_map->erase(found_object);
-
-  // Clean up when the map is empty.
-  if (np_object_map->empty())
-    instance_map_.erase(found_instance);
 }
 
 NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
@@ -108,18 +103,11 @@ void HostVarTracker::DidDeleteInstance(PP_Instance instance) {
     return;  // Nothing to do.
   NPObjectToNPObjectVarMap* np_object_map = found_instance->second.get();
 
-  // Force delete all var references. It's possible that deleting an object "A"
-  // will cause it to delete another object "B" it references, thus removing "B"
-  // from instance_map_. Therefore, we need to make a copy over which we can
-  // iterate safely. Furthermore, the maps contain WeakPtrs so that we can
-  // detect if the object is gone so that we don't dereference invalid memory.
-  NPObjectToNPObjectVarMap np_object_map_copy = *np_object_map;
-  NPObjectToNPObjectVarMap::iterator cur_var =
-      np_object_map_copy.begin();
-  while (cur_var != np_object_map_copy.end()) {
-    NPObjectToNPObjectVarMap::iterator current = cur_var++;
-    ForceReleaseNPObject(current->second);
-    np_object_map->erase(current->first);
+  // Force delete all var references. ForceReleaseNPObject() will cause
+  // this object, and potentially others it references, to be removed from
+  // |np_object_map|.
+  while (!np_object_map->empty()) {
+    ForceReleaseNPObject(np_object_map->begin()->second);
   }
 
   // Remove the record for this instance since it should be empty.
@@ -127,15 +115,9 @@ void HostVarTracker::DidDeleteInstance(PP_Instance instance) {
   instance_map_.erase(found_instance);
 }
 
-void HostVarTracker::ForceReleaseNPObject(
-    const base::WeakPtr< ::ppapi::NPObjectVar>& object) {
-  // There's a chance that the object was already deleted before we got here.
-  // See DidDeleteInstance for further explanation. If the object was deleted,
-  // the WeakPtr will return NULL.
-  if (!object.get())
-    return;
-  object->InstanceDeleted();
-  VarMap::iterator iter = live_vars_.find(object->GetExistingVarID());
+void HostVarTracker::ForceReleaseNPObject(::ppapi::NPObjectVar* object_var) {
+  object_var->InstanceDeleted();
+  VarMap::iterator iter = live_vars_.find(object_var->GetExistingVarID());
   if (iter == live_vars_.end()) {
     NOTREACHED();
     return;
diff --git a/webkit/plugins/ppapi/host_var_tracker.h b/webkit/plugins/ppapi/host_var_tracker.h
index 510e860..7629f9a 100644
--- a/webkit/plugins/ppapi/host_var_tracker.h
+++ b/webkit/plugins/ppapi/host_var_tracker.h
@@ -13,7 +13,6 @@
 #include "base/hash_tables.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "ppapi/c/pp_instance.h"
 #include "ppapi/c/pp_resource.h"
 #include "ppapi/shared_impl/resource_tracker.h"
@@ -66,9 +65,9 @@ class HostVarTracker : public ::ppapi::VarTracker {
 
   // Clear the reference count of the given object and remove it from
   // live_vars_.
-  void ForceReleaseNPObject(const base::WeakPtr< ::ppapi::NPObjectVar>& object);
+  void ForceReleaseNPObject(::ppapi::NPObjectVar* object_var);
 
-  typedef std::map<NPObject*, base::WeakPtr< ::ppapi::NPObjectVar> >
+  typedef std::map<NPObject*, ::ppapi::NPObjectVar*>
       NPObjectToNPObjectVarMap;
 
   // Lists all known NPObjects, first indexed by the corresponding instance,
diff --git a/webkit/plugins/ppapi/npobject_var.cc b/webkit/plugins/ppapi/npobject_var.cc
index 09e9f3c..5c89281 100644
--- a/webkit/plugins/ppapi/npobject_var.cc
+++ b/webkit/plugins/ppapi/npobject_var.cc
@@ -41,6 +41,7 @@ PP_VarType NPObjectVar::GetType() const {
 
 void NPObjectVar::InstanceDeleted() {
   DCHECK(pp_instance_);
+  HostGlobals::Get()->host_var_tracker()->RemoveNPObjectVar(this);
   pp_instance_ = 0;
 }
 
diff --git a/webkit/plugins/ppapi/npobject_var.h b/webkit/plugins/ppapi/npobject_var.h
index 1a187ac..6f31e3d 100644
--- a/webkit/plugins/ppapi/npobject_var.h
+++ b/webkit/plugins/ppapi/npobject_var.h
@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/compiler_specific.h"
-#include "base/memory/weak_ptr.h"
 #include "ppapi/c/pp_instance.h"
 #include "ppapi/shared_impl/var.h"
 #include "webkit/plugins/webkit_plugins_export.h"
@@ -28,8 +27,7 @@ namespace ppapi {
 // PP_Var IDs) for each module. This allows us to track all references owned by
 // a given module and free them when the plugin exits independently of other
 // plugins that may be running at the same time.
-class NPObjectVar : public Var,
-                    public base::SupportsWeakPtr<NPObjectVar> {
+class NPObjectVar : public Var {
  public:
   // You should always use FromNPObject to create an NPObjectVar. This function
   // guarantees that we maintain the 1:1 mapping between NPObject and