Replace NPObject usage in ppapi with gin

content/renderer/pepper/host_var_tracker.cc

Index: content/renderer/pepper/host_var_tracker.cc
diff --git a/content/renderer/pepper/host_var_tracker.cc b/content/renderer/pepper/host_var_tracker.cc
index 700fbaf28be3d8246c3d56965e81ba90d73c5c4b..46ebaf8e39841e64d0619ea394d31664c0a49708 100644
--- a/content/renderer/pepper/host_var_tracker.cc
+++ b/content/renderer/pepper/host_var_tracker.cc
@@ -6,16 +6,43 @@
 
 #include "base/logging.h"
 #include "content/renderer/pepper/host_array_buffer_var.h"
+#include "content/renderer/pepper/host_globals.h"
 #include "content/renderer/pepper/host_resource_var.h"
-#include "content/renderer/pepper/npobject_var.h"
 #include "content/renderer/pepper/pepper_plugin_instance_impl.h"
+#include "content/renderer/pepper/v8object_var.h"
 #include "ppapi/c/pp_var.h"
 
 using ppapi::ArrayBufferVar;
-using ppapi::NPObjectVar;
+using ppapi::V8ObjectVar;
 
 namespace content {
 
+HostVarTracker::V8ObjectVarKey::V8ObjectVarKey(V8ObjectVar* object_var)
+    : instance(object_var->instance()->pp_instance()) {
+  v8::Handle<v8::Object> object = object_var->GetHandle();

[dmichael (off chromium), 2014/08/20 23:09:16]

Could you use v8::Local here instead of Handle to make it clearer?

[raymes, 2014/08/22 08:28:39]

Done.

+  hash = object.IsEmpty() ? 0 : object->GetIdentityHash();
+}
+
+HostVarTracker::V8ObjectVarKey::V8ObjectVarKey(PP_Instance instance,
+                                               v8::Handle<v8::Object> object)
+    : instance(instance),
+      hash(object.IsEmpty() ? 0 : object->GetIdentityHash()) {}
+
+HostVarTracker::V8ObjectVarKey::~V8ObjectVarKey() {}
+
+
+bool HostVarTracker::V8ObjectVarKey::operator<(
+    const V8ObjectVarKey& other) const {
+  if (instance == other.instance)
+    return hash < other.hash;
+  return instance < other.instance;
+}
+
+bool HostVarTracker::V8ObjectVarKey::operator==(
+    const V8ObjectVarKey& other) const {
+  return instance == other.instance && hash == other.hash;
+}
+
 HostVarTracker::HostVarTracker()
     : VarTracker(SINGLE_THREADED), last_shared_memory_map_id_(0) {}
 
@@ -31,74 +58,42 @@ ArrayBufferVar* HostVarTracker::CreateShmArrayBuffer(
   return new HostArrayBufferVar(size_in_bytes, handle);
 }
 
-void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) {
+void HostVarTracker::AddV8ObjectVar(V8ObjectVar* object_var) {
   CheckThreadingPreconditions();
-
-  InstanceMap::iterator found_instance =
-      instance_map_.find(object_var->pp_instance());
-  if (found_instance == instance_map_.end()) {
-    // Lazily create the instance map.
-    DCHECK(object_var->pp_instance() != 0);
-    found_instance =
-        instance_map_.insert(std::make_pair(
-                                 object_var->pp_instance(),
-                                 linked_ptr<NPObjectToNPObjectVarMap>(
-                                     new NPObjectToNPObjectVarMap))).first;
-  }
-  NPObjectToNPObjectVarMap* np_object_map = found_instance->second.get();
-
-  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));
+  v8::HandleScope handle_scope(object_var->instance()->GetIsolate());
+  DCHECK(GetForV8Object(object_var->instance()->pp_instance(),
+                        object_var->GetHandle()) == object_map_.end());
+  object_map_.insert(std::make_pair(V8ObjectVarKey(object_var), object_var));
 }
 
-void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) {
+void HostVarTracker::RemoveV8ObjectVar(V8ObjectVar* object_var) {
   CheckThreadingPreconditions();
-
-  InstanceMap::iterator found_instance =
-      instance_map_.find(object_var->pp_instance());
-  if (found_instance == instance_map_.end()) {
-    NOTREACHED() << "NPObjectVar has invalid instance.";
-    return;
-  }
-  NPObjectToNPObjectVarMap* np_object_map = found_instance->second.get();
-
-  NPObjectToNPObjectVarMap::iterator found_object =
-      np_object_map->find(object_var->np_object());
-  if (found_object == np_object_map->end()) {
-    NOTREACHED() << "NPObjectVar not registered.";
-    return;
-  }
-  if (found_object->second != object_var) {
-    NOTREACHED() << "NPObjectVar doesn't match.";
-    return;
-  }
-  np_object_map->erase(found_object);
+  v8::HandleScope handle_scope(object_var->instance()->GetIsolate());
+  ObjectMap::iterator it = GetForV8Object(
+      object_var->instance()->pp_instance(), object_var->GetHandle());
+  DCHECK(it != object_map_.end());
+  object_map_.erase(it);
 }
 
-NPObjectVar* HostVarTracker::NPObjectVarForNPObject(PP_Instance instance,
-                                                    NPObject* np_object) {
+PP_Var HostVarTracker::V8ObjectVarForV8Object(PP_Instance instance,
+                                              v8::Handle<v8::Object> object) {
   CheckThreadingPreconditions();
-
-  InstanceMap::iterator found_instance = instance_map_.find(instance);
-  if (found_instance == instance_map_.end())
-    return NULL;  // No such instance.
-  NPObjectToNPObjectVarMap* np_object_map = found_instance->second.get();
-
-  NPObjectToNPObjectVarMap::iterator found_object =
-      np_object_map->find(np_object);
-  if (found_object == np_object_map->end())
-    return NULL;  // No such object.
-  return found_object->second;
+  ObjectMap::const_iterator it = GetForV8Object(instance, object);
+  if (it == object_map_.end())
+    return (new V8ObjectVar(instance, object))->GetPPVar();
+  return it->second->GetPPVar();
 }
 
-int HostVarTracker::GetLiveNPObjectVarsForInstance(PP_Instance instance) const {
+int HostVarTracker::GetLiveV8ObjectVarsForInstance(PP_Instance instance) {
   CheckThreadingPreconditions();
-
-  InstanceMap::const_iterator found = instance_map_.find(instance);
-  if (found == instance_map_.end())
-    return 0;
-  return static_cast<int>(found->second->size());
+  int count = 0;
+  ObjectMap::const_iterator it = object_map_.lower_bound(
+      V8ObjectVarKey(instance, v8::Handle<v8::Object>()));

[dmichael (off chromium), 2014/08/20 23:09:15]

suggestion: equal_range might be clearer and simplify the loop. (here and the
other places you use lower_bound)

[raymes, 2014/08/22 08:28:39]

I thought about this a bit. It means you have to write something like:
std::pair<ObjectMap::const_iterator, ObjectMap::const_iterator> range =
object_map_.equal_range(V8ObjectVarKey(instance, v8::Handle<v8::Object>(),
V8ObjectVarKey(instance + 1, v8::Handle<v8::Object>());

And then technically you should handle overflow and it's still quite verbose. If
you feel strongly one way I'll be happy to change it :)

[dmichael (off chromium), 2014/08/22 16:28:46]

I don't see why or how you would pass two keys? I'm talking about
std::multimap::equal_range. I don't think there's any overflow to worry about...
when you get the two iterators back, you just iterate over that range (which
might be empty). I think it should be fairly clean.

[raymes, 2014/08/25 01:40:00]

Sorry it was late Friday afternoon and I was not thinking nor being careful :(

You're right. But in most of the cases (like this one) equal_range won't suffice
because we want to iterate through all the of the objects that have the same
instance, but not necessarily the same hash. That is why the key constructed
here has an empty handle, so that the lower bound will include all objects with
the given instance. Does that make sense?

There is one case (GetForV8Object) where equal_range works and it does simplify
it. I've used it there :)

[dmichael (off chromium), 2014/08/25 17:31:11]

Ah, I completely missed that detail. Maybe you can add a comment to call out the
empty handle trick you're doing?

[raymes, 2014/08/26 05:15:11]

Done.

+  while (it != object_map_.end() && it->first.instance == instance) {
+    ++count;
+    ++it;
+  }
+  return count;
 }
 
 PP_Var HostVarTracker::MakeResourcePPVarFromMessage(
@@ -116,27 +111,24 @@ ppapi::ResourceVar* HostVarTracker::MakeResourceVar(PP_Resource pp_resource) {
   return new HostResourceVar(pp_resource);
 }
 
-void HostVarTracker::DidDeleteInstance(PP_Instance instance) {
+void HostVarTracker::DidDeleteInstance(PP_Instance pp_instance) {
   CheckThreadingPreconditions();
 
-  InstanceMap::iterator found_instance = instance_map_.find(instance);
-  if (found_instance == instance_map_.end())
-    return;  // Nothing to do.
-  NPObjectToNPObjectVarMap* np_object_map = found_instance->second.get();
-
-  // Force delete all var references. ForceReleaseNPObject() will cause
+  PepperPluginInstanceImpl* instance =
+      HostGlobals::Get()->GetInstance(pp_instance);
+  v8::HandleScope handle_scope(instance->GetIsolate());
+  // Force delete all var references. ForceReleaseV8Object() 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);
+  // |live_vars_|.
+  ObjectMap::iterator it = object_map_.lower_bound(
+      V8ObjectVarKey(pp_instance, v8::Handle<v8::Object>()));
+  while (it != object_map_.end() && it->first.instance == pp_instance) {
+    ForceReleaseV8Object(it->second);
+    object_map_.erase(it++);
   }
-
-  // Remove the record for this instance since it should be empty.
-  DCHECK(np_object_map->empty());
-  instance_map_.erase(found_instance);
 }
 
-void HostVarTracker::ForceReleaseNPObject(ppapi::NPObjectVar* object_var) {
+void HostVarTracker::ForceReleaseV8Object(ppapi::V8ObjectVar* object_var) {
   object_var->InstanceDeleted();
   VarMap::iterator iter = live_vars_.find(object_var->GetExistingVarID());
   if (iter == live_vars_.end()) {
@@ -148,6 +140,20 @@ void HostVarTracker::ForceReleaseNPObject(ppapi::NPObjectVar* object_var) {
   DeleteObjectInfoIfNecessary(iter);
 }
 
+HostVarTracker::ObjectMap::iterator HostVarTracker::GetForV8Object(
+    PP_Instance instance,
+    v8::Handle<v8::Object> object) {
+  V8ObjectVarKey key(instance, object);
+  ObjectMap::iterator it = object_map_.lower_bound(key);
+
+  while (it != object_map_.end() && it->first == key) {
+    if (object == it->second->GetHandle())
+      return it;
+    ++it;
+  }
+  return object_map_.end();
+}
+
 int HostVarTracker::TrackSharedMemoryHandle(PP_Instance instance,
                                             base::SharedMemoryHandle handle,
                                             uint32 size_in_bytes) {