[PPAPI] Moving pp::VarResource_Dev API into pp::Var (now stable).

ppapi/cpp/var.cc

Index: ppapi/cpp/var.cc
diff --git a/ppapi/cpp/var.cc b/ppapi/cpp/var.cc
index da4c12c4388294f13b62ce142dbf9c7bd5e2ef90..079af011413e2fb42d5f936429ee4fb92849b4a9 100644
--- a/ppapi/cpp/var.cc
+++ b/ppapi/cpp/var.cc
@@ -25,6 +25,9 @@ namespace pp {
 
 namespace {
 
+template <> const char* interface_name<PPB_Var_1_2>() {
+  return PPB_VAR_INTERFACE_1_2;
+}
 template <> const char* interface_name<PPB_Var_1_1>() {
   return PPB_VAR_INTERFACE_1_1;
 }
@@ -42,7 +45,9 @@ inline bool NeedsRefcounting(const PP_Var& var) {
 // This helper function uses the latest available version of VarFromUtf8. Note
 // that version 1.0 of this method has a different API to later versions.
 PP_Var VarFromUtf8Helper(const char* utf8_str, uint32_t len) {
-  if (has_interface<PPB_Var_1_1>()) {
+  if (has_interface<PPB_Var_1_2>()) {
+    return get_interface<PPB_Var_1_2>()->VarFromUtf8(utf8_str, len);
+  } else if (has_interface<PPB_Var_1_1>()) {
     return get_interface<PPB_Var_1_1>()->VarFromUtf8(utf8_str, len);
   } else if (has_interface<PPB_Var_1_0>()) {
     return get_interface<PPB_Var_1_0>()->VarFromUtf8(Module::Get()->pp_module(),
@@ -56,7 +61,10 @@ PP_Var VarFromUtf8Helper(const char* utf8_str, uint32_t len) {
 // This helper function uses the latest available version of AddRef.
 // Returns true on success, false if no appropriate interface was available.
 bool AddRefHelper(const PP_Var& var) {
-  if (has_interface<PPB_Var_1_1>()) {
+  if (has_interface<PPB_Var_1_2>()) {
+    get_interface<PPB_Var_1_2>()->AddRef(var);
+    return true;
+  } else if (has_interface<PPB_Var_1_1>()) {

[yzshen1, 2014/02/10 17:00:53]

nit: According to style guide, it is preferred to omit all "else" in this case:
(multiple places in this file)
if (...) {
  return true;
}
if (...) {
  return true;
}
return false;

[dmichael (off chromium), 2014/02/10 18:31:56]

I wasn't aware of that; can you point me at the part of the style guide where it
says that? I think Matt got this format from file_io.cc, which I pointed him at.
Sorry if that was not right. Here's that example:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/cpp/file_io....

I personally find "else if" more readable, because you don't have to look inside
of the body of each "if" to know if the body of more than 1 "if" condition can
be executed. The existing way also is more consistent with how we've done other
files.
(See also
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/cpp/input_ev...
... and probably other examples)

I would be fine either way, except I do favor consistency.

[yzshen1, 2014/02/10 19:44:14]

The style guide is at: http://www.chromium.org/developers/coding-style
(Please look at the part "Don't use else after return".)

There are quite some file using the no-else-after-return way. So I think we are
already not very consistent. :/ Hmm...


On 2014/02/10 18:31:56, dmichael wrote:

[dmichael (off chromium), 2014/02/10 20:12:51]

Thanks, I was looking at the broader open source Google style guide that one
references.

I wouldn't have read that to apply to "else if", though I agree with you that it
would apply to the final "else" (like line 73-75).

I think:
if (...) {
}
if (...) {
}
is not as consistent with our other C++ wrapper fallback code I pointed to (I
didn't look at other examples that are in other parts of Pepper; I was trying to
find the closest analogue to this code).

But it's really not important to me; we can do it the way you prefer.

[yzshen1, 2014/02/10 22:38:41]

I agree that it is not very important. :)
Please pick whatever you would like to do, Matt. I am fine with David's
approach, too.

Thanks!

On 2014/02/10 20:12:51, dmichael wrote:

[Matt Giuca, 2014/02/11 02:41:17]

I've changed the else but not the else-ifs.

     get_interface<PPB_Var_1_1>()->AddRef(var);
     return true;
   } else if (has_interface<PPB_Var_1_0>()) {
@@ -70,7 +78,10 @@ bool AddRefHelper(const PP_Var& var) {
 // This helper function uses the latest available version of Release.
 // Returns true on success, false if no appropriate interface was available.
 bool ReleaseHelper(const PP_Var& var) {
-  if (has_interface<PPB_Var_1_1>()) {
+  if (has_interface<PPB_Var_1_2>()) {
+    get_interface<PPB_Var_1_2>()->Release(var);
+    return true;
+  } else if (has_interface<PPB_Var_1_1>()) {
     get_interface<PPB_Var_1_1>()->Release(var);
     return true;
   } else if (has_interface<PPB_Var_1_0>()) {
@@ -128,6 +139,19 @@ Var::Var(const std::string& utf8_str) {
   is_managed_ = true;
 }
 
+Var::Var(const pp::Resource& resource) {
+  if (has_interface<PPB_Var_1_2>()) {
+    var_ = get_interface<PPB_Var_1_2>()->VarFromResource(
+        resource.pp_resource());
+  } else {
+    PP_NOTREACHED();
+    return;
+  }
+  // Set |is_managed_| to true, so |var_| will be properly released upon
+  // destruction.
+  is_managed_ = true;
+}
+
 
 Var::Var(const PP_Var& var) {
   var_ = var;
@@ -233,7 +257,9 @@ std::string Var::AsString() const {
 
   uint32_t len;
   const char* str;
-  if (has_interface<PPB_Var_1_1>())
+  if (has_interface<PPB_Var_1_2>())
+    str = get_interface<PPB_Var_1_2>()->VarToUtf8(var_, &len);
+  else if (has_interface<PPB_Var_1_1>())
     str = get_interface<PPB_Var_1_1>()->VarToUtf8(var_, &len);
   else if (has_interface<PPB_Var_1_0>())
     str = get_interface<PPB_Var_1_0>()->VarToUtf8(var_, &len);
@@ -242,6 +268,20 @@ std::string Var::AsString() const {
   return std::string(str, len);
 }
 
+pp::Resource Var::AsResource() const {
+  if (!is_resource()) {
+    PP_NOTREACHED();
+    return pp::Resource();
+  }
+
+  if (has_interface<PPB_Var_1_2>()) {
+    return pp::Resource(pp::PASS_REF,
+                        get_interface<PPB_Var_1_2>()->VarToResource(var_));
+  } else {
+    return pp::Resource();
+  }
+}
+
 std::string Var::DebugString() const {
   char buf[256];
   if (is_undefined()) {