Pepper WebSocket API: Fix memory leak issue

 - Browser callback should release PP_Var resource after serialization.
 - Fix pp::Var to release injected refcounted object.
 - Fix example to release resources before reusing it.

BUG=146304

Review URL: https://chromiumcodereview.appspot.com/10905128

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@156235 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 31 insertions, 28 deletions

diff --git a/ppapi/cpp/private/var_private.cc b/ppapi/cpp/private/var_private.cc
index 27e418a..ff61226 100644
--- a/ppapi/cpp/private/var_private.cc
+++ b/ppapi/cpp/private/var_private.cc
@@ -29,12 +29,11 @@ VarPrivate::VarPrivate(const InstanceHandle& instance,
   if (has_interface<PPB_Var_Deprecated>()) {
     var_ = get_interface<PPB_Var_Deprecated>()->CreateObject(
         instance.pp_instance(), object->GetClass(), object);
-    needs_release_ = true;
   } else {
     var_.type = PP_VARTYPE_NULL;
     var_.padding = 0;
-    needs_release_ = false;
   }
+  is_managed_ = true;
 }
 
 ScriptableObject* VarPrivate::AsScriptableObject() const {
diff --git a/ppapi/cpp/var.cc b/ppapi/cpp/var.cc
index 3e76593..7fc67f5 100644
--- a/ppapi/cpp/var.cc
+++ b/ppapi/cpp/var.cc
@@ -59,65 +59,63 @@ PP_Var VarFromUtf8Helper(const char* utf8_str, uint32_t len) {
 Var::Var() {
   memset(&var_, 0, sizeof(var_));
   var_.type = PP_VARTYPE_UNDEFINED;
-  needs_release_ = false;
+  is_managed_ = true;
 }
 
 Var::Var(Null) {
   memset(&var_, 0, sizeof(var_));
   var_.type = PP_VARTYPE_NULL;
-  needs_release_ = false;
+  is_managed_ = true;
 }
 
 Var::Var(bool b) {
   var_.type = PP_VARTYPE_BOOL;
   var_.padding = 0;
   var_.value.as_bool = PP_FromBool(b);
-  needs_release_ = false;
+  is_managed_ = true;
 }
 
 Var::Var(int32_t i) {
   var_.type = PP_VARTYPE_INT32;
   var_.padding = 0;
   var_.value.as_int = i;
-  needs_release_ = false;
+  is_managed_ = true;
 }
 
 Var::Var(double d) {
   var_.type = PP_VARTYPE_DOUBLE;
   var_.padding = 0;
   var_.value.as_double = d;
-  needs_release_ = false;
+  is_managed_ = true;
 }
 
 Var::Var(const char* utf8_str) {
   uint32_t len = utf8_str ? static_cast<uint32_t>(strlen(utf8_str)) : 0;
   var_ = VarFromUtf8Helper(utf8_str, len);
-  needs_release_ = (var_.type == PP_VARTYPE_STRING);
+  is_managed_ = true;
 }
 
 Var::Var(const std::string& utf8_str) {
   var_ = VarFromUtf8Helper(utf8_str.c_str(),
-                            static_cast<uint32_t>(utf8_str.size()));
-  needs_release_ = (var_.type == PP_VARTYPE_STRING);
+                           static_cast<uint32_t>(utf8_str.size()));
+  is_managed_ = true;
 }
 
 Var::Var(const Var& other) {
   var_ = other.var_;
+  is_managed_ = true;
   if (NeedsRefcounting(var_)) {
-    if (has_interface<PPB_Var_1_0>()) {
-      needs_release_ = true;
+    if (has_interface<PPB_Var_1_0>())
       get_interface<PPB_Var_1_0>()->AddRef(var_);
-    } else {
+    else
       var_.type = PP_VARTYPE_NULL;
-      needs_release_ = false;
-    }
-  } else {
-    needs_release_ = false;
   }
 }
 
 Var::~Var() {
-  if (needs_release_ && has_interface<PPB_Var_1_0>())
+  if (NeedsRefcounting(var_) &&
+      is_managed_ &&
+      has_interface<PPB_Var_1_0>())
     get_interface<PPB_Var_1_0>()->Release(var_);
 }
 
@@ -130,16 +128,14 @@ Var& Var::operator=(const Var& other) {
 
   // Be careful to keep the ref alive for cases where we're assigning an
   // object to itself by addrefing the new one before releasing the old one.
-  bool old_needs_release = needs_release_;
+  bool old_is_managed = is_managed_;
+  is_managed_ = true;
   if (NeedsRefcounting(other.var_)) {
     // Assume we already has_interface<PPB_Var_1_0> for refcounted vars or else
     // we couldn't have created them in the first place.
-    needs_release_ = true;
     get_interface<PPB_Var_1_0>()->AddRef(other.var_);
-  } else {
-    needs_release_ = false;
   }
-  if (old_needs_release)
+  if (NeedsRefcounting(var_) && old_is_managed)
     get_interface<PPB_Var_1_0>()->Release(var_);
 
   var_ = other.var_;
diff --git a/ppapi/cpp/var.h b/ppapi/cpp/var.h
index b0914be..437eed9 100644
--- a/ppapi/cpp/var.h
+++ b/ppapi/cpp/var.h
@@ -57,7 +57,7 @@ class Var {
   /// the reference count will not normally be incremented for you.
   Var(PassRef, PP_Var var) {
     var_ = var;
-    needs_release_ = true;
+    is_managed_ = true;
   }
 
   struct DontManage {};
@@ -72,7 +72,7 @@ class Var {
   /// @param[in] var A <code>Var</code>.
   Var(DontManage, PP_Var var) {
     var_ = var;
-    needs_release_ = false;
+    is_managed_ = false;
   }
 
   /// A constructor for copying a <code>Var</code>.
@@ -211,7 +211,7 @@ class Var {
   PP_Var Detach() {
     PP_Var ret = var_;
     var_ = PP_MakeUndefined();
-    needs_release_ = false;
+    is_managed_ = true;
     return ret;
   }
 
@@ -280,7 +280,10 @@ class Var {
 
  protected:
   PP_Var var_;
-  bool needs_release_;
+
+  // |is_managed_| indicates if the instance manages |var_|.
+  // You need to check if |var_| is refcounted to call Release().
+  bool is_managed_;
 
  private:
   // Prevent an arbitrary pointer argument from being implicitly converted to
diff --git a/ppapi/cpp/var_array_buffer.cc b/ppapi/cpp/var_array_buffer.cc
index 358b2c9..c05a8eb 100644
--- a/ppapi/cpp/var_array_buffer.cc
+++ b/ppapi/cpp/var_array_buffer.cc
@@ -30,11 +30,11 @@ VarArrayBuffer::VarArrayBuffer(const Var& var) : Var(var) {
 VarArrayBuffer::VarArrayBuffer(uint32_t size_in_bytes) {
   if (has_interface<PPB_VarArrayBuffer_1_0>()) {
     var_ = get_interface<PPB_VarArrayBuffer_1_0>()->Create(size_in_bytes);
-    needs_release_ = true;
   } else {
     PP_NOTREACHED();
     var_ = PP_MakeNull();
   }
+  is_managed_ = true;
 }
 
 pp::VarArrayBuffer& VarArrayBuffer::operator=(const VarArrayBuffer& other) {
diff --git a/ppapi/cpp/websocket.cc b/ppapi/cpp/websocket.cc
index 1fbcaa4..f0ae5fa 100644
--- a/ppapi/cpp/websocket.cc
+++ b/ppapi/cpp/websocket.cc
@@ -70,6 +70,10 @@ int32_t WebSocket::ReceiveMessage(Var* message,
   if (!has_interface<PPB_WebSocket_1_0>())
     return PP_ERROR_BADRESOURCE;
 
+  // Initialize |message| to release old internal PP_Var of reused |message|.
+  if (message)
+    *message = Var();
+
   return get_interface<PPB_WebSocket_1_0>()->ReceiveMessage(
       pp_resource(), const_cast<PP_Var*>(&message->pp_var()),
       callback.pp_completion_callback());
diff --git a/ppapi/native_client/src/shared/ppapi_proxy/browser_callback.cc b/ppapi/native_client/src/shared/ppapi_proxy/browser_callback.cc
index 5f5e7c3..debb0bf 100644
--- a/ppapi/native_client/src/shared/ppapi_proxy/browser_callback.cc
+++ b/ppapi/native_client/src/shared/ppapi_proxy/browser_callback.cc
@@ -89,6 +89,7 @@ void RunRemoteCallback(void* user_data, int32_t result) {
     read_buffer_size = kMaxReturnVarSize;
     read_buffer.reset(
         Serialize(&remote_callback->read_var, 1, &read_buffer_size));
+    PPBVarInterface()->Release(remote_callback->read_var);
   }
 
   NaClSrpcError srpc_result =