[PPAPI] ResourceVar now reference counts its Resource in the plugin.

ResourceVar is now an abstract base class with subclasses
HostResourceVar and PluginResourceVar. The PluginResourceVar has a
reference counted Resource instead of a PP_Resource.

VarTracker has MakeResourceVar and MakeResourcePPVar methods, to
abstract over the creation of a resource var of the correct subclass.

Also, the creation_message is now NULL when empty, instead of being an
empty message object.

BUG=290713

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

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

diff --git a/content/content_renderer.gypi b/content/content_renderer.gypi
index 0d2039db8..46b7468 100644
--- a/content/content_renderer.gypi
+++ b/content/content_renderer.gypi

@@ -287,6 +287,8 @@
     'renderer/pepper/host_dispatcher_wrapper.h',
     'renderer/pepper/host_globals.cc',
     'renderer/pepper/host_globals.h',
+    'renderer/pepper/host_resource_var.cc',
+    'renderer/pepper/host_resource_var.h',
     'renderer/pepper/host_var_tracker.cc',
     'renderer/pepper/host_var_tracker.h',
     'renderer/pepper/message_channel.cc',

diff --git a/content/renderer/pepper/host_resource_var.cc b/content/renderer/pepper/host_resource_var.cc
new file mode 100644
index 0000000..a59bed37
--- /dev/null
+++ b/content/renderer/pepper/host_resource_var.cc

@@ -0,0 +1,32 @@
+// Copyright 2013 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 "content/renderer/pepper/host_resource_var.h"
+
+namespace content {
+
+HostResourceVar::HostResourceVar() : pp_resource_(0) {}
+
+HostResourceVar::HostResourceVar(PP_Resource pp_resource)
+    : pp_resource_(pp_resource) {}
+
+HostResourceVar::HostResourceVar(const IPC::Message& creation_message)
+    : pp_resource_(0),
+      creation_message_(new IPC::Message(creation_message)) {}
+
+PP_Resource HostResourceVar::GetPPResource() const {
+  return pp_resource_;
+}
+
+const IPC::Message* HostResourceVar::GetCreationMessage() const {
+  return creation_message_.get();
+}
+
+bool HostResourceVar::IsPending() const {
+  return pp_resource_ == 0 && creation_message_;
+}
+
+HostResourceVar::~HostResourceVar() {}
+
+}  // namespace content

diff --git a/content/renderer/pepper/host_resource_var.h b/content/renderer/pepper/host_resource_var.h
new file mode 100644
index 0000000..79fe487
--- /dev/null
+++ b/content/renderer/pepper/host_resource_var.h

@@ -0,0 +1,53 @@
+// Copyright 2013 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.
+
+#ifndef PPAPI_CONTENT_RENDERER_PEPPER_HOST_RESOURCE_VAR_H_
+#define PPAPI_CONTENT_RENDERER_PEPPER_HOST_RESOURCE_VAR_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "ipc/ipc_message.h"
+#include "ppapi/c/pp_resource.h"
+#include "ppapi/shared_impl/resource_var.h"
+#include "ppapi/shared_impl/var.h"
+
+namespace content {
+
+// Represents a resource var, usable on the host side. Can either have a
+// plugin-side resource or a pending resource host.
+class HostResourceVar : public ppapi::ResourceVar {
+ public:
+  // Makes a null resource var.
+  HostResourceVar();
+
+  // Makes a resource var with an existing plugin-side resource.
+  explicit HostResourceVar(PP_Resource pp_resource);
+
+  // Makes a resource var with a pending resource host.
+  // The |creation_message| contains data needed to create the plugin-side
+  // resource. Its type depends on the type of resource.
+  explicit HostResourceVar(const IPC::Message& creation_message);
+
+  // ResourceVar override.
+  virtual PP_Resource GetPPResource() const OVERRIDE;
+  virtual const IPC::Message* GetCreationMessage() const OVERRIDE;
+  virtual bool IsPending() const OVERRIDE;
+
+ protected:
+  virtual ~HostResourceVar();
+
+ private:
+  // Real resource ID in the plugin. 0 if one has not yet been created
+  // (indicating that there is a pending host resource).
+  PP_Resource pp_resource_;
+
+  // If the plugin-side resource has not yet been created, carries a message to
+  // create a resource of the specific type on the plugin side. Otherwise, NULL.
+  scoped_ptr<IPC::Message> creation_message_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostResourceVar);
+};
+
+}  // namespace content
+
+#endif

diff --git a/content/renderer/pepper/host_var_tracker.cc b/content/renderer/pepper/host_var_tracker.cc
index 71a568ac..ed67dba 100644
--- a/content/renderer/pepper/host_var_tracker.cc
+++ b/content/renderer/pepper/host_var_tracker.cc

@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "content/renderer/pepper/host_array_buffer_var.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 "ppapi/c/pp_var.h"
@@ -102,6 +103,10 @@
   return static_cast<int>(found->second->size());
 }
 
+ppapi::ResourceVar* HostVarTracker::MakeResourceVar(PP_Resource pp_resource) {
+  return new HostResourceVar(pp_resource);
+}
+
 void HostVarTracker::DidDeleteInstance(PP_Instance instance) {
   CheckThreadingPreconditions();
 

diff --git a/content/renderer/pepper/host_var_tracker.h b/content/renderer/pepper/host_var_tracker.h
index 5abdbff..858c301 100644
--- a/content/renderer/pepper/host_var_tracker.h
+++ b/content/renderer/pepper/host_var_tracker.h

@@ -55,6 +55,7 @@
       PP_Instance instance) const;
 
   // VarTracker public implementation.
+  virtual ppapi::ResourceVar* MakeResourceVar(PP_Resource pp_resource) OVERRIDE;
   virtual void DidDeleteInstance(PP_Instance instance) OVERRIDE;
 
   virtual int TrackSharedMemoryHandle(PP_Instance instance,

diff --git a/ppapi/ppapi_proxy.gypi b/ppapi/ppapi_proxy.gypi
index cf415d5..267402d6 100644
--- a/ppapi/ppapi_proxy.gypi
+++ b/ppapi/ppapi_proxy.gypi

@@ -105,6 +105,8 @@
           'proxy/plugin_resource.h',
           'proxy/plugin_resource_tracker.cc',
           'proxy/plugin_resource_tracker.h',
+          'proxy/plugin_resource_var.cc',
+          'proxy/plugin_resource_var.h',
           'proxy/plugin_var_serialization_rules.cc',
           'proxy/plugin_var_serialization_rules.h',
           'proxy/plugin_var_tracker.cc',

diff --git a/ppapi/proxy/plugin_resource_var.cc b/ppapi/proxy/plugin_resource_var.cc
new file mode 100644
index 0000000..0ccff74
--- /dev/null
+++ b/ppapi/proxy/plugin_resource_var.cc

@@ -0,0 +1,20 @@
+// Copyright 2013 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 "ppapi/proxy/plugin_resource_var.h"
+
+PluginResourceVar::PluginResourceVar() {}
+
+PluginResourceVar::PluginResourceVar(ppapi::Resource* resource)
+    : resource_(resource) {}
+
+PP_Resource PluginResourceVar::GetPPResource() const {
+  return resource_->pp_resource();
+}
+
+bool PluginResourceVar::IsPending() const {
+  return false;
+}
+
+PluginResourceVar::~PluginResourceVar() {}

diff --git a/ppapi/proxy/plugin_resource_var.h b/ppapi/proxy/plugin_resource_var.h
new file mode 100644
index 0000000..b02718b6
--- /dev/null
+++ b/ppapi/proxy/plugin_resource_var.h

@@ -0,0 +1,39 @@
+// Copyright 2013 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.
+
+#ifndef PPAPI_PROXY_PLUGIN_RESOURCE_VAR_H_
+#define PPAPI_PROXY_PLUGIN_RESOURCE_VAR_H_
+
+#include "ppapi/c/pp_resource.h"
+#include "ppapi/proxy/ppapi_proxy_export.h"
+#include "ppapi/shared_impl/resource.h"
+#include "ppapi/shared_impl/resource_var.h"
+#include "ppapi/shared_impl/var.h"
+
+// Represents a resource Var, usable on the plugin side.
+class PPAPI_PROXY_EXPORT PluginResourceVar : public ppapi::ResourceVar {
+ public:
+  // Makes a null resource var.
+  PluginResourceVar();
+
+  // Makes a resource var with an existing resource.
+  // Takes one reference to the given resource.
+  explicit PluginResourceVar(ppapi::Resource* resource);
+
+  // ResourceVar override.
+  virtual PP_Resource GetPPResource() const OVERRIDE;
+  virtual bool IsPending() const OVERRIDE;
+
+  scoped_refptr<ppapi::Resource> resource() const { return resource_; }
+
+ protected:
+  virtual ~PluginResourceVar();
+
+ private:
+  scoped_refptr<ppapi::Resource> resource_;
+
+  DISALLOW_COPY_AND_ASSIGN(PluginResourceVar);
+};
+
+#endif

diff --git a/ppapi/proxy/plugin_var_tracker.cc b/ppapi/proxy/plugin_var_tracker.cc
index ee29985..d2a4d74 100644
--- a/ppapi/proxy/plugin_var_tracker.cc
+++ b/ppapi/proxy/plugin_var_tracker.cc

@@ -10,10 +10,13 @@
 #include "ppapi/c/ppb_var.h"
 #include "ppapi/proxy/plugin_array_buffer_var.h"
 #include "ppapi/proxy/plugin_dispatcher.h"
+#include "ppapi/proxy/plugin_resource_var.h"
 #include "ppapi/proxy/ppapi_messages.h"
 #include "ppapi/proxy/proxy_object_var.h"
 #include "ppapi/shared_impl/api_id.h"
+#include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/proxy_lock.h"
+#include "ppapi/shared_impl/resource_tracker.h"
 #include "ppapi/shared_impl/var.h"
 
 namespace ppapi {
@@ -151,6 +154,14 @@
   ReleaseVar(found->second);
 }
 
+ResourceVar* PluginVarTracker::MakeResourceVar(PP_Resource pp_resource) {
+  ResourceTracker* resource_tracker = PpapiGlobals::Get()->GetResourceTracker();
+  ppapi::Resource* resource = resource_tracker->GetResource(pp_resource);
+  if (!resource)
+    return NULL;
+  return new PluginResourceVar(resource);
+}
+
 void PluginVarTracker::DidDeleteInstance(PP_Instance instance) {
   // Calling the destructors on plugin objects may in turn release other
   // objects which will mutate the map out from under us. So do a two-step

diff --git a/ppapi/proxy/plugin_var_tracker.h b/ppapi/proxy/plugin_var_tracker.h
index 670457f..126c8fc 100644
--- a/ppapi/proxy/plugin_var_tracker.h
+++ b/ppapi/proxy/plugin_var_tracker.h

@@ -59,6 +59,7 @@
                          const PP_Var& host_object);
 
   // VarTracker public overrides.
+  virtual ResourceVar* MakeResourceVar(PP_Resource pp_resource) OVERRIDE;
   virtual void DidDeleteInstance(PP_Instance instance) OVERRIDE;
   virtual int TrackSharedMemoryHandle(PP_Instance instance,
                                       base::SharedMemoryHandle file,

diff --git a/ppapi/shared_impl/resource_var.cc b/ppapi/shared_impl/resource_var.cc
index c5719aa..3e34c38 100644
--- a/ppapi/shared_impl/resource_var.cc
+++ b/ppapi/shared_impl/resource_var.cc

@@ -9,18 +9,8 @@
 
 namespace ppapi {
 
-ResourceVar::ResourceVar() : pp_resource_(0) {}
-
-ResourceVar::ResourceVar(PP_Resource pp_resource) : pp_resource_(pp_resource) {}
-
-ResourceVar::ResourceVar(const IPC::Message& creation_message)
-    : pp_resource_(0),
-      creation_message_(creation_message) {}
-
-ResourceVar::~ResourceVar() {}
-
-bool ResourceVar::IsPending() const {
-  return pp_resource_ == 0 && creation_message_.type() != 0;
+const IPC::Message* ResourceVar::GetCreationMessage() const {
+  return NULL;
 }
 
 ResourceVar* ResourceVar::AsResourceVar() {
@@ -42,4 +32,8 @@
   return var_object->AsResourceVar();
 }
 
+ResourceVar::ResourceVar() {}
+
+ResourceVar::~ResourceVar() {}
+
 }  // namespace ppapi

diff --git a/ppapi/shared_impl/resource_var.h b/ppapi/shared_impl/resource_var.h
index 70eabc6c..0b8bab2 100644
--- a/ppapi/shared_impl/resource_var.h
+++ b/ppapi/shared_impl/resource_var.h

@@ -5,42 +5,34 @@
 #ifndef PPAPI_SHARED_IMPL_RESOURCE_VAR_H_
 #define PPAPI_SHARED_IMPL_RESOURCE_VAR_H_
 
-#include "ipc/ipc_message.h"
 #include "ppapi/c/pp_resource.h"
 #include "ppapi/c/pp_var.h"
 #include "ppapi/shared_impl/ppapi_shared_export.h"
 #include "ppapi/shared_impl/var.h"
 
+namespace IPC {
+class Message;
+}
+
 namespace ppapi {
 
 // Represents a resource Var.
 class PPAPI_SHARED_EXPORT ResourceVar : public Var {
  public:
-  // Makes a null resource var.
-  ResourceVar();
-
-  // Makes a resource var with an existing plugin-side resource.
-  explicit ResourceVar(PP_Resource pp_resource);
-
-  // Makes a resource var with a pending resource host.
-  // The |creation_message| contains instructions on how to create the
-  // plugin-side resource. Its type depends on the type of resource.
-  explicit ResourceVar(const IPC::Message& creation_message);
-
-  virtual ~ResourceVar();
-
   // Gets the resource ID associated with this var.
-  // This is 0 if a resource is still pending.
-  PP_Resource pp_resource() const { return pp_resource_; }
+  // This is 0 if a resource is still pending (only possible on the host side).
+  // NOTE: This can return a PP_Resource with a reference count of 0 on the
+  // plugin side. It should be AddRef'd if the resource is passed to the user.
+  virtual PP_Resource GetPPResource() const = 0;
 
-  // Gets the message for creating a plugin-side resource.
-  // May be an empty message.
-  const IPC::Message& creation_message() const { return creation_message_; }
+  // Gets the message for creating a plugin-side resource. Returns NULL if the
+  // message is empty (which is always true on the plugin side).
+  virtual const IPC::Message* GetCreationMessage() const;
 
   // Determines whether this is a pending resource.
-  // This is true if the pp_resource is 0 and there is a non-empty
-  // creation_message.
-  bool IsPending() const;
+  // This is true if, on the host side, the there is a creation_message and no
+  // PP_Resource.
+  virtual bool IsPending() const = 0;
 
   // Var override.
   virtual ResourceVar* AsResourceVar() OVERRIDE;
@@ -50,16 +42,12 @@
   // return NULL if the PP_Var is not of Resource type.
   static ResourceVar* FromPPVar(PP_Var var);
 
+ protected:
+  ResourceVar();
+
+  virtual ~ResourceVar();
+
  private:
-  // Real resource ID in the plugin. 0 if one has not yet been created
-  // (indicating that there is a pending host resource).
-  PP_Resource pp_resource_;
-
-  // If the plugin-side resource has not yet been created, carries a message to
-  // create a resource of the specific type on the plugin side.
-  // Otherwise, carries an empty message.
-  IPC::Message creation_message_;
-
   DISALLOW_COPY_AND_ASSIGN(ResourceVar);
 };
 

diff --git a/ppapi/shared_impl/test_globals.h b/ppapi/shared_impl/test_globals.h
index 709438e..2a0ece3 100644
--- a/ppapi/shared_impl/test_globals.h
+++ b/ppapi/shared_impl/test_globals.h

@@ -18,6 +18,9 @@
  public:
   TestVarTracker() : VarTracker(THREAD_SAFE) {}
   virtual ~TestVarTracker() {}
+  virtual ResourceVar* MakeResourceVar(PP_Resource pp_resource) OVERRIDE {
+    return NULL;
+  }
   virtual ArrayBufferVar* CreateArrayBuffer(uint32 size_in_bytes) OVERRIDE {
     return NULL;
   }

diff --git a/ppapi/shared_impl/unittest_utils.cc b/ppapi/shared_impl/unittest_utils.cc
index f613882..6974bd20 100644
--- a/ppapi/shared_impl/unittest_utils.cc
+++ b/ppapi/shared_impl/unittest_utils.cc

@@ -8,6 +8,7 @@
 
 #include "base/containers/hash_tables.h"
 #include "base/logging.h"
+#include "ipc/ipc_message.h"
 #include "ppapi/shared_impl/array_var.h"
 #include "ppapi/shared_impl/dictionary_var.h"
 #include "ppapi/shared_impl/resource_var.h"
@@ -156,17 +157,19 @@
       ResourceVar* expected_var = ResourceVar::FromPPVar(expected);
       ResourceVar* actual_var = ResourceVar::FromPPVar(actual);
       DCHECK(expected_var && actual_var);
-      if (expected_var->pp_resource() != actual_var->pp_resource()) {
-        LOG(ERROR) << "expected: " << expected_var->pp_resource() << " actual: "
-                   << actual_var->pp_resource();
+      if (expected_var->GetPPResource() != actual_var->GetPPResource()) {
+        LOG(ERROR) << "expected: " << expected_var->GetPPResource()
+                   << " actual: " << actual_var->GetPPResource();
         return false;
       }
-      IPC::Message actual_message(actual_var->creation_message());
-      const IPC::Message& expected_message = expected_var->creation_message();
-      if (expected_message.size() != actual_message.size()) {
+
+      const IPC::Message* actual_message = actual_var->GetCreationMessage();
+      const IPC::Message* expected_message =
+          expected_var->GetCreationMessage();
+      if (expected_message->size() != actual_message->size()) {
         LOG(ERROR) << "expected creation message size: "
-                   << expected_message.size() << " actual: "
-                   << actual_message.size();
+                   << expected_message->size() << " actual: "
+                   << actual_message->size();
         return false;
       }
 
@@ -174,12 +177,13 @@
       // expected. This is an unpredictable reference number that changes
       // between serialization/deserialization, and we do not want it to cause
       // the comparison to fail.
-      actual_message.SetHeaderValues(actual_message.routing_id(),
-                                     actual_message.type(),
-                                     (expected_message.flags() & 0xffffff00) |
-                                      (actual_message.flags() & 0xff));
-      if (memcmp(expected_message.data(), actual_message.data(),
-                 expected_message.size()) != 0) {
+      IPC::Message local_actual_message(*actual_message);
+      local_actual_message.SetHeaderValues(
+          actual_message->routing_id(), actual_message->type(),
+          (expected_message->flags() & 0xffffff00) |
+           (actual_message->flags() & 0xff));
+      if (memcmp(expected_message->data(), local_actual_message.data(),
+                 expected_message->size()) != 0) {
         LOG(ERROR) << "expected creation message does not match actual.";
         return false;
       }

diff --git a/ppapi/shared_impl/var.cc b/ppapi/shared_impl/var.cc
index 9644a44..d128d902 100644
--- a/ppapi/shared_impl/var.cc
+++ b/ppapi/shared_impl/var.cc

@@ -71,8 +71,8 @@
 
       if (resource->IsPending()) {
         return base::StringPrintf("[Pending resource]");
-      } else if (resource->pp_resource()) {
-        return base::StringPrintf("[Resource %d]", resource->pp_resource());
+      } else if (resource->GetPPResource()) {
+        return base::StringPrintf("[Resource %d]", resource->GetPPResource());
       } else {
         return "[Null resource]";
       }

diff --git a/ppapi/shared_impl/var_tracker.cc b/ppapi/shared_impl/var_tracker.cc
index 254c51a..bada0ab 100644
--- a/ppapi/shared_impl/var_tracker.cc
+++ b/ppapi/shared_impl/var_tracker.cc

@@ -13,6 +13,7 @@
 #include "ppapi/shared_impl/host_resource.h"
 #include "ppapi/shared_impl/id_assignment.h"
 #include "ppapi/shared_impl/proxy_lock.h"
+#include "ppapi/shared_impl/resource_var.h"
 #include "ppapi/shared_impl/var.h"
 
 namespace ppapi {
@@ -234,6 +235,13 @@
   return array_buffer->GetPPVar();
 }
 
+PP_Var VarTracker::MakeResourcePPVar(PP_Resource pp_resource) {
+  CheckThreadingPreconditions();
+
+  ResourceVar* resource_var = MakeResourceVar(pp_resource);
+  return resource_var ? resource_var->GetPPVar() : PP_MakeNull();
+}
+
 std::vector<PP_Var> VarTracker::GetLiveVars() {
   CheckThreadingPreconditions();
 

diff --git a/ppapi/shared_impl/var_tracker.h b/ppapi/shared_impl/var_tracker.h
index d7fb49c..379f645b 100644
--- a/ppapi/shared_impl/var_tracker.h
+++ b/ppapi/shared_impl/var_tracker.h

@@ -85,6 +85,15 @@
   // usually immediately put this in a scoped_refptr).
   ArrayBufferVar* MakeArrayBufferVar(uint32 size_in_bytes, const void* data);
 
+  // Creates a new resource var that points to a given resource ID. Returns a
+  // PP_Var that references it and has an initial reference count of 1.
+  PP_Var MakeResourcePPVar(PP_Resource pp_resource);
+
+  // Creates a new resource var that points to a given resource ID. This is
+  // implemented by the host and plugin tracker separately, because the plugin
+  // keeps a reference to the resource, and the host does not.
+  virtual ResourceVar* MakeResourceVar(PP_Resource pp_resource) = 0;
+
   // Return a vector containing all PP_Vars that are in the tracker. This is
   // to help implement PPB_Testing_Dev.GetLiveVars and should generally not be
   // used in production code. The PP_Vars are returned in no particular order,