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

ppapi/cpp/var.cc

 // Copyright (c) 2012 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/cpp/var.h"
 
 #include <stdio.h>
 #include <string.h>
 
 #include <algorithm>
 
 #include "ppapi/c/pp_var.h"
 #include "ppapi/c/ppb_var.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/logging.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/module_impl.h"
 
 // Define equivalent to snprintf on Windows.
 #if defined(_MSC_VER)
 #  define snprintf sprintf_s
 #endif
 
 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;
 }
 template <> const char* interface_name<PPB_Var_1_0>() {
   return PPB_VAR_INTERFACE_1_0;
 }
 
 // Technically you can call AddRef and Release on any Var, but it may involve
 // cross-process calls depending on the plugin. This is an optimization so we
 // only do refcounting on the necessary objects.
 inline bool NeedsRefcounting(const PP_Var& var) {
   return var.type > PP_VARTYPE_DOUBLE;
 }
 
 // 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(),
                                                      utf8_str,
                                                      len);
   } else {
     return PP_MakeNull();
   }
 }
 
 // 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>()) {
     get_interface<PPB_Var_1_0>()->AddRef(var);
     return true;
   } else {
     return false;
   }
 }
 
 // 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>()) {
     get_interface<PPB_Var_1_0>()->Release(var);
     return true;
   } else {
     return false;
   }
 }
 
@@ skipping 37 matching lines @@
   var_ = VarFromUtf8Helper(utf8_str, len);
   is_managed_ = true;
 }
 
 Var::Var(const std::string& utf8_str) {
   var_ = VarFromUtf8Helper(utf8_str.c_str(),
                            static_cast<uint32_t>(utf8_str.size()));
   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;
   is_managed_ = true;
   if (NeedsRefcounting(var_)) {
     if (!AddRefHelper(var_))
       var_.type = PP_VARTYPE_NULL;
   }
 }
 
@@ skipping 46 matching lines @@
     case PP_VARTYPE_DOUBLE:
       return AsDouble() == other.AsDouble();
     case PP_VARTYPE_STRING:
       if (var_.value.as_id == other.var_.value.as_id)
         return true;
       return AsString() == other.AsString();
     case PP_VARTYPE_OBJECT:
     case PP_VARTYPE_ARRAY:
     case PP_VARTYPE_ARRAY_BUFFER:
     case PP_VARTYPE_DICTIONARY:
     default:  // Objects, arrays, dictionaries.

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

Please also add resource type here.

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

Done. (Oh no, this should have been there even before this CL.)

       return var_.value.as_id == other.var_.value.as_id;
   }
 }
 
 bool Var::AsBool() const {
   if (!is_bool()) {
     PP_NOTREACHED();
     return false;
   }
   return PP_ToBool(var_.value.as_bool);
@@ skipping 18 matching lines @@
 }
 
 std::string Var::AsString() const {
   if (!is_string()) {
     PP_NOTREACHED();
     return std::string();
   }
 
   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);
   else
     return std::string();
   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()) {
     snprintf(buf, sizeof(buf), "Var(UNDEFINED)");
   } else if (is_null()) {
     snprintf(buf, sizeof(buf), "Var(NULL)");
   } else if (is_bool()) {
     snprintf(buf, sizeof(buf), AsBool() ? "Var(true)" : "Var(false)");
   } else if (is_int()) {
     snprintf(buf, sizeof(buf), "Var(%d)", static_cast<int>(AsInt()));
@@ skipping 10 matching lines @@
     }
     snprintf(buf, sizeof(buf), format, str.c_str());
   } else if (is_object()) {
     snprintf(buf, sizeof(buf), "Var(OBJECT)");
   } else if (is_array()) {
     snprintf(buf, sizeof(buf), "Var(ARRAY)");
   } else if (is_dictionary()) {
     snprintf(buf, sizeof(buf), "Var(DICTIONARY)");
   } else if (is_array_buffer()) {
     snprintf(buf, sizeof(buf), "Var(ARRAY_BUFFER)");
   } else {

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

please also add resource here.

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

Done.

     buf[0] = '\0';
   }
   return buf;
 }
 
 }  // namespace pp