[PPAPI] Added a new Var subclass, ResourceVar.

ppapi/shared_impl/resource_var.h

Index: ppapi/shared_impl/resource_var.h
diff --git a/ppapi/shared_impl/resource_var.h b/ppapi/shared_impl/resource_var.h
new file mode 100644
index 0000000000000000000000000000000000000000..3015f1a673bd479bd0dc532c24479baa6cfd7b8b
--- /dev/null
+++ b/ppapi/shared_impl/resource_var.h
@@ -0,0 +1,63 @@
+// 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_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 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.
+  ResourceVar(PP_Resource resource_id);

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

explicit?

[Matt Giuca, 2013/08/29 01:44:47]

Done.

+
+  // Makes a resource var with a pending resource host.
+  // The |creation_message| contains instructions on how to create the
+  // plugin-side resource host. Its type depends on the type of resource.

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

rule of thumb: "resource host" at the brower/renderer side; "resource" at the
plugin side. there is no "plugin-side" resource host. :)

[Matt Giuca, 2013/08/29 01:44:47]

Done.

+  ResourceVar(const IPC::Message& creation_message);

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

explicit, please.

[Matt Giuca, 2013/08/29 01:44:47]

Done.

+
+  virtual ~ResourceVar();
+
+  // Gets the resource ID associated with this var.
+  // This is 0 if a resource is still pending.
+  PP_Resource resource_id() const { return resource_id_; }

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

To be more consistent with other code, please consider to use pp_resource() and
pp_resource_.

[Matt Giuca, 2013/08/29 01:44:47]

Done.

+
+  // Gets the message for creating a plugin-side resource.
+  // May be an empty message.
+  const IPC::Message& creation_message() const { return creation_message_; }
+
+  // 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:
+  // Real resource ID in the plugin. 0 if one has not yet been created
+  // (indicating that there is a pending host resource).
+  PP_Resource resource_id_;

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

Are you planning to use this same ResourceVar class in both the renderer process
and the plugin process?

One thing that you will need to handle is how to keep the resource corresponding
to |resource_id_| alive. When a resource PP_Var is alive, the resource it points
to should stay alive.

At the plugin side, we can hold 1 ref to |resource_id_|. At the host side, we
cannot do so, because the ref count of |resource_id_| is not managed there.
(This isn't true in the proxy/impl old design. But file system is using the new
design now.)

What if a resource var is passed from plugin process to renderer process? One
way I can think up is to make sure that the plugin side holds 1 ref to the
resource var (which holds 1 ref to |resource_id_|) when the resource var is
alive at the renderer side.
But I am okay that we do that in future changes, if we make sure that currently
we only pass newly-created resource var from renderer to plugin.

[Matt Giuca, 2013/08/29 01:44:47]

I haven't thought much about the reference counting here, but it doesn't seem
like there'll be a huge problem.

When passing a resource from host to plugin, we create it on the plugin and
increment it to 1. When the user is finished with it, we remove the reference
and it gets freed on both the plugin and host side automatically, correct?

When passing a resource from plugin to host, it will have a reference count of
1. When it arrives on the host side, part of the process of converting to a V8
value will take all the important information from the resource and store it in
the V8 value. The host side will not need to hold a reference to the resource.
Which means the plugin can free the resource when it needs to; the JavaScript
value should remain intact.

Or, is the problem the asynchronous nature of the protocol? What if I pass a
resource to the host, and then decrement its reference, causing it to be deleted
in the host? Is there a possibility that the "delete resource host" message will
arrive before the "post message"?

[yzshen1, 2013/08/30 00:25:27]

Correct. This part is relatively easy.
Correct. (But please also see my comments for your next paragraph.)

My concern is not directly related to passing a object into JS.
It is that all the other kinds of PP_Var are able to live "stably" at the
renderer side. In other words, at the renderer side once you hold a reference to
a PP_Var, you know that the contents it points to will be alive as long as you
want.
It is good to make resource PP_Var behave in the same way.
But maybe this is not a blocking issue, as long as we make sure resource PP_Vars
are used only in posting messages. Just something to keep in mind.
No, the messages will arrive in order.
However, if we need to post a task to run on the message loop and the resource
var is needed there, the "delete resource host" message can get in the middle.

[Matt Giuca, 2013/08/30 02:55:20]

Thanks for explaining.

OK it sounds like none of these will be an issue for this CL. Can we proceed
with committing this so I can move on to the next CL? (I have about 4 WIP CLs
lined up behind this one ;))

+
+  // 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);
+};
+
+}  // namespace ppapi
+
+#endif  // PPAPI_SHARED_IMPL_RESOURCE_VAR_H_