[PPAPI] Added a new Var subclass, ResourceVar.

ppapi/shared_impl/var.h

Index: ppapi/shared_impl/var.h
diff --git a/ppapi/shared_impl/var.h b/ppapi/shared_impl/var.h
index bded4656f7ed78f5370faeec815d5908c241e4e4..2005b7fb537c5d4ea730b249ab66bd55689c7092 100644
--- a/ppapi/shared_impl/var.h
+++ b/ppapi/shared_impl/var.h
@@ -11,6 +11,8 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/shared_memory.h"
 #include "base/platform_file.h"
+#include "ppapi/c/pp_file_info.h"
+#include "ppapi/c/pp_resource.h"
 #include "ppapi/c/pp_var.h"
 #include "ppapi/shared_impl/host_resource.h"
 #include "ppapi/shared_impl/ppapi_shared_export.h"
@@ -22,6 +24,7 @@ class ArrayVar;
 class DictionaryVar;
 class NPObjectVar;
 class ProxyObjectVar;
+class ResourceVar;
 class StringVar;
 class VarTracker;
 
@@ -41,6 +44,7 @@ class PPAPI_SHARED_EXPORT Var : public base::RefCounted<Var> {
   virtual ProxyObjectVar* AsProxyObjectVar();
   virtual ArrayVar* AsArrayVar();
   virtual DictionaryVar* AsDictionaryVar();
+  virtual ResourceVar* AsResourceVar();
 
   // Creates a PP_Var corresponding to this object. The return value will have
   // one reference addrefed on behalf of the caller.
@@ -187,6 +191,74 @@ class PPAPI_SHARED_EXPORT ArrayBufferVar : public Var {
   DISALLOW_COPY_AND_ASSIGN(ArrayBufferVar);
 };
 
+// ResourceVar -----------------------------------------------------------------

[yzshen1, 2013/08/26 17:20:46]

Please consider a separate file. (Just as array and dict var.)

[Matt Giuca, 2013/08/28 07:12:14]

Done.

+
+// The information about a resource var that is serialized.
+struct PPAPI_SHARED_EXPORT ResourceInfo {

[Matt Giuca, 2013/08/23 08:41:16]

The reason I put this in a separate struct (instead of just putting these fields
in ResourceVar) is because we need the exact same struct in ResourceRawVarData
in the next CL. (It is not possible to include ResourceVar in the
ResourceRawVarData because of its base class.)

[Matt Giuca, 2013/08/28 07:12:14]

Gone.

+  ResourceInfo();  // Makes a null resource info.
+
+  // Pending host resource ID in the renderer. 0 if there is already a real

[yzshen1, 2013/08/26 17:20:46]

nit, but better to be consistent: this is a "resource host" ID, not a "host
resource" ID.

[Matt Giuca, 2013/08/28 07:12:14]

Gone.

+  // resource.
+  int pending_renderer_id;

[yzshen1, 2013/08/26 17:20:46]

how about pending_*host*_id or pending_renderer_host_id?

[Matt Giuca, 2013/08/28 07:12:14]

Gone.

+
+  // TODO(mgiuca): Also add a pending_browser_id when we create a separate
+  // pending host resource in the browser.
+
+  // Real resource ID in the plugin. 0 if one has not yet been created
+  // (indicating that there is a pending host resource).

[yzshen1, 2013/08/26 17:20:46]

nit, but better to be consistent: pending "resource host", not "host resource".
(There are other places below)

[Matt Giuca, 2013/08/28 07:12:14]

Gone.

+  PP_Resource resource_id;
+
+  // Type of the file system (for file system resources only).
+  PP_FileSystemType file_system_type;

[raymes, 2013/08/26 00:06:11]

As discussed could this be an IPC::Message which is the create message instead?

[yzshen1, 2013/08/26 17:20:46]

+1 to use some more generic format instead of having specific data fields for
different resources.

[Matt Giuca, 2013/08/28 07:12:14]

Done.

+};
+
+// Represents a resource Var.
+class PPAPI_SHARED_EXPORT ResourceVar : public Var {

[Matt Giuca, 2013/08/23 08:41:16]

Now I am a bit uncertain about exactly what methods it is appropriate to expose
here. It's possible we won't need all of these and the best policy may be to
only add them as necessary. I'll have more of a think about this on Monday.

[raymes, 2013/08/26 00:06:11]

I would just have a set/get ResourceInfo. A setter may not even be needed?

[Matt Giuca, 2013/08/28 07:12:14]

Done.

+ public:
+  ResourceVar();  // Makes a null resource var.
+  virtual ~ResourceVar();
+
+  // Sets the pending host resource ID.
+  void SetPendingId(int pending_renderer_id);

[yzshen1, 2013/08/26 17:20:46]

The setters and getters are using different naming format. Is there any reason
for that?

[Matt Giuca, 2013/08/28 07:12:14]

Gone.

+
+  // Sets the resource ID.
+  void SetResourceId(int resource_id);
+
+  // Sets the file system type.
+  void SetFileSystemType(PP_FileSystemType file_system_type);
+
+  // Gets the resource ID associated with this var.

[yzshen1, 2013/08/26 17:20:46]

"pending host ID", please

[Matt Giuca, 2013/08/28 07:12:14]

Gone.

+  // This is 0 if a resource is still pending.
+  int pending_renderer_id() const { return data_.pending_renderer_id; }
+
+  // Gets the resource ID associated with this var.
+  // This is 0 if a resource is still pending.
+  PP_Resource resource_id() const { return data_.resource_id; }
+
+  // Gets the file system type.
+  // Only valid for file system resources.
+  PP_FileSystemType file_system_type() const { return data_.file_system_type; }
+
+  // Get the data associated with this resource.
+  const ResourceInfo& data() const { return data_; }
+
+  // Get the data associated with this resource.
+  ResourceInfo* mutable_data() { return &data_; }

[yzshen1, 2013/08/26 17:20:46]

Maybe the following two signatures look more consistent?

const ResourceInfo* data() const;
ResourceInfo* data();

[Matt Giuca, 2013/08/28 07:12:14]

Gone.

But for reference, I'm following the pattern from the protocol buffers C++ API,
which makes a const version that returns a reference, and a "mutable_" non-const
version that returns a pointer. I think this is good because const references
should be preferred over pointers where possible.

[yzshen1, 2013/08/28 17:31:58]

Okay.

+
+  // Var override.
+  virtual ResourceVar* AsResourceVar() OVERRIDE;
+  virtual PP_VarType GetType() const OVERRIDE;
+
+  // Helper function that converts a PP_Var to a ResourceVar. This will
+  // return NULL if the PP_Var is not of Resource type.
+  static ResourceVar* FromPPVar(PP_Var var);
+
+ private:
+  ResourceInfo data_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourceVar);
+};
+
 }  // namespace ppapi
 
 #endif  // PPAPI_SHARED_IMPL_VAR_H_