PPAPI: Release renderer Var refcount when returning to plugin

BUG=451624
R=raymes@chromium.org

Review URL: https://codereview.chromium.org/874703003

Cr-Commit-Position: refs/heads/master@{#313355}

ppapi/proxy/host_var_serialization_rules.cc

@@ -45,9 +45,18 @@ PP_Var HostVarSerializationRules::BeginSendPassRef(const PP_Var& var) {
   return var;
 }
 
-void HostVarSerializationRules::EndSendPassRef(const PP_Var& /* var */) {
-  // See PluginVarSerialization::ReceivePassRef for an example. We don't need
-  // to do anything here.
+void HostVarSerializationRules::EndSendPassRef(const PP_Var& var) {
+  // See PluginVarSerializationRules::ReceivePassRef for an example. We don't
+  // need to do anything here for "Object" vars; we continue holding one ref on
+  // behalf of the plugin.
+  if (var.type != PP_VARTYPE_OBJECT) {
+    // But for other ref-counted types (like String, Array, and Dictionary),
+    // the value will be re-constituted on the other side as a new Var with no
+    // connection to the host-side reference counting. We must therefore release
+    // our reference count; this is roughly equivalent to passing the ref to the
+    // plugin.
+    PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(var);
+  }
 }
 
 void HostVarSerializationRules::ReleaseObjectRef(const PP_Var& var) {

ppapi/proxy/serialized_var_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "ppapi/proxy/ppapi_proxy_test.h"
 
+#include "ppapi/proxy/proxy_object_var.h"
 #include "ppapi/proxy/serialized_var.h"
 #include "ppapi/shared_impl/proxy_lock.h"
 
@@ -272,6 +273,38 @@ TEST_F(SerializedVarTest, PluginVectorReceiveInput) {
   EXPECT_EQ(-1, var_tracker().GetRefCountForObject(plugin_objects2[1]));
 }
 
+// Tests the browser sending a String var as a return value to make sure we
+// ref-count the host side properly.
+typedef HostProxyTest HostSerializedVarTest;
+TEST_F(HostSerializedVarTest, PluginReceiveStringReturn) {
+  {
+    PP_Var string_var = StringVar::StringToPPVar("Hello");
+    EXPECT_EQ(1, var_tracker().GetRefCountForObject(string_var));
+    GetDispatcher()->serialization_rules()->BeginSendPassRef(string_var);
+    GetDispatcher()->serialization_rules()->EndSendPassRef(string_var);
+    // It should be gone, so we should get -1 to indicate that.
+    EXPECT_EQ(-1, var_tracker().GetRefCountForObject(string_var));
+  }
+
+  {
+    // Note this is as little weird; we're testing the behavior of the host-
+    // side of the proxy, but we use ProxyObjectVar, because this unit test
+    // doesn't have access to stuff in content/renderer/pepper. The ref-counting
+    // behavior should be the same, however. All we're really testing
+    // is the code in ppapi/proxy (HostVarSerializationRules).
+    scoped_refptr<Var> obj_var = new ProxyObjectVar(NULL, 1234);
+    PP_Var obj_pp_var = obj_var->GetPPVar();
+    EXPECT_EQ(1, var_tracker().GetRefCountForObject(obj_pp_var));
+    GetDispatcher()->serialization_rules()->BeginSendPassRef(obj_pp_var);
+    GetDispatcher()->serialization_rules()->EndSendPassRef(obj_pp_var);
+    // The host side for object vars always keeps 1 ref on behalf of the plugin.
+    // See HostVarSerializationRules and PluginVarSerializationRules for an
+    // explanation.
+    EXPECT_EQ(1, var_tracker().GetRefCountForObject(obj_pp_var));
+    var_tracker().ReleaseVar(obj_pp_var);
+  }
+}
+
 // Tests the plugin receiving a var as a return value from the browser
 // two different times (passing ownership).
 TEST_F(SerializedVarTest, PluginReceiveReturn) {