Pepper WebSocket API: Fix memory leak issue

native_client_sdk/src/examples/websocket/websocket.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/completion_callback.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/var.h"
 #include "ppapi/cpp/var_array_buffer.h"
 #include "ppapi/cpp/websocket.h"
@@ skipping 78 matching lines @@
 }
 
 void WebSocketInstance::Send(const std::string& message) {
   if (!IsConnected())
     return;
   websocket_->SendMessage(pp::Var(message));
   PostMessage(pp::Var(std::string("log:send: ") + message));
 }
 
 void WebSocketInstance::Receive() {
+  // Release old var resource if it is reused.
+  pp::Var release(pp::PASS_REF, receive_var_.Detach());

[Takashi Toyoshima, 2012/09/07 17:18:27]

If we want to reuse the same pp::Var to receive ref-counted objects, we should
release internal PP_Var before passing it again. But I'm not sure this code is
good example for developers.
Should we release existing internal resource at ppapi/cpp/websocket.cc?

[dmichael (off chromium), 2012/09/07 18:36:58]

What if we just make ReceiveMessage release the var immediately? If plugins are
forced to do this, they'll get it wrong. I.e., we can just make ReceiveMessage
set the out-param to Undefined:
*message = Var();


The "new, cool" way to do this is with a CompletionCallbackWithOutput. It
shouldn't change the C interface, but would make the C++ interface look
different. ReceiveMessage's signature would become:
int32_t ReceiveMessage(const CompletionCallbackWithOutput<pp::Var>& callback)
And calling it would look like this:
pp::CompletionCallbackWithOutput<pp::Var> callback(OnReceiveCompletionCallback);
websocket_->ReceiveMessage(callback);
(note that you don't have to provide a Var)

OnReceiveCompletionCallback would look like this:
void OnReceiveCompletionCallback(int32_t result, const pp::Var& message);

The callback takes care of allocating & passing the var. Unfortunately, this
didn't exist when you did Websocket. But we should be able to use it now,
without breaking the ABI. I suspect we'd want to support both forms in parallel
for a while, to avoid breaking source compatibility. What do you think?

[Takashi Toyoshima, 2012/09/10 13:21:27]

OK. I'll do '*message = Var();' in ReceiveMessage internally.

Also, the CompletionCallbackWithOutput looks nice.
I want to introduce this new interface to C++ API.

But this CL might be merged to other branches.
So, I'll separate it into another CL and BUG.
https://code.google.com/p/chromium/issues/detail?id=147544

   pp::CompletionCallback callback(OnReceiveCompletionCallback, this);
   // |receive_var_| must be valid until |callback| is invoked.
   // Just use a member variable.
   websocket_->ReceiveMessage(&receive_var_, callback);
 }
 
 void WebSocketInstance::OnConnectCompletion(int32_t result) {
   if (result != PP_OK) {
     PostMessage(pp::Var("alert:connection failed"));
     return;
@@ skipping 50 matching lines @@
 };
 
 
 // Implement the required pp::CreateModule function that creates our specific
 // kind of Module.
 namespace pp {
 Module* CreateModule() {
   return new WebSocketModule();
 }
 }  // namespace pp