WebSocket Pepper API: WebArrayBuffer support

webkit/plugins/ppapi/ppb_websocket_impl.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 "webkit/plugins/ppapi/ppb_websocket_impl.h"
 
 #include <set>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_util.h"
 #include "ppapi/c/dev/ppb_var_array_buffer_dev.h"
 #include "ppapi/c/pp_completion_callback.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_var.h"
 #include "ppapi/c/ppb_var.h"
 #include "ppapi/shared_impl/var.h"
 #include "ppapi/shared_impl/var_tracker.h"
-#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebData.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebArrayBuffer.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebPluginContainer.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSocket.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebURL.h"
+#include "webkit/plugins/ppapi/host_array_buffer_var.h"
 #include "webkit/plugins/ppapi/host_globals.h"
 #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 #include "webkit/plugins/ppapi/resource_helper.h"
 
 using ppapi::ArrayBufferVar;
 using ppapi::PpapiGlobals;
 using ppapi::StringVar;
 using ppapi::thunk::PPB_WebSocket_API;
 using ppapi::TrackedCallback;
 using ppapi::VarTracker;
-using WebKit::WebData;
+using WebKit::WebArrayBuffer;
 using WebKit::WebDocument;
 using WebKit::WebString;
 using WebKit::WebSocket;
 using WebKit::WebSocketClient;
 using WebKit::WebURL;
 
 const uint32_t kMaxReasonSizeInBytes = 123;
 const size_t kHybiBaseFramingOverhead = 2;
 const size_t kHybiMaskingKeyLength = 4;
 const size_t kMinimumPayloadSizeWithTwoByteExtendedPayloadLength = 126;
@@ skipping 38 matching lines @@
       close_code_(0),
       close_was_clean_(PP_FALSE),
       empty_string_(new StringVar("", 0)),
       buffered_amount_(0),
       buffered_amount_after_close_(0) {
 }
 
 PPB_WebSocket_Impl::~PPB_WebSocket_Impl() {
   if (websocket_.get())
     websocket_->disconnect();
-
-  // Clean up received and unread messages
-  VarTracker* var_tracker = PpapiGlobals::Get()->GetVarTracker();
-  while (!received_messages_.empty()) {
-    PP_Var var = received_messages_.front();
-    received_messages_.pop();
-    var_tracker->ReleaseVar(var);
-  }
 }
 
 // static
 PP_Resource PPB_WebSocket_Impl::Create(PP_Instance instance) {
   scoped_refptr<PPB_WebSocket_Impl> ws(new PPB_WebSocket_Impl(instance));
   return ws->GetReference();
 }
 
 PPB_WebSocket_API* PPB_WebSocket_Impl::AsPPB_WebSocket_API() {
   return this;
@@ skipping 81 matching lines @@
   if (!callback.func)
     return PP_ERROR_BLOCKS_MAIN_THREAD;
 
   // Create WebKit::WebSocket object and connect.
   WebDocument document = plugin_instance->container()->element().document();
   websocket_.reset(WebSocket::create(document, this));
   DCHECK(websocket_.get());
   if (!websocket_.get())
     return PP_ERROR_NOTSUPPORTED;
 
+  // Set receiving binary object type.
+  websocket_->setBinaryType(static_cast<WebSocket::BinaryType>(binary_type_));

[dmichael (off chromium), 2012/01/13 17:36:14]

It seems like it would be a good idea to either convert explicitly with a switch
statement or to have assertions somewhere (maybe even statically) so you know
the values match up.

[Takashi Toyoshima, 2012/01/17 10:14:59]

OK, I hold this value as WebSocket::BinaryType, then explicit conversion is done
in Set/GetBinaryType().

+
   websocket_->connect(web_url, web_protocols);
   state_ = PP_WEBSOCKETREADYSTATE_CONNECTING_DEV;
 
   // Install callback.
   connect_callback_ = new TrackedCallback(this, callback);
 
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t PPB_WebSocket_Impl::Close(uint16_t code,
@@ skipping 134 matching lines @@
   // Send the message.
   if (message.type == PP_VARTYPE_STRING) {
     // Convert message to WebString.
     scoped_refptr<StringVar> message_string = StringVar::FromPPVar(message);
     if (!message_string)
       return PP_ERROR_BADARGUMENT;
     WebString web_message = WebString::fromUTF8(message_string->value());
     if (!websocket_->sendText(web_message))
       return PP_ERROR_BADARGUMENT;
   } else if (message.type == PP_VARTYPE_ARRAY_BUFFER) {
-    // Convert message to WebData.
-    // TODO(toyoshim): Must add a WebKit interface which handles WebArrayBuffer
-    // directly.
-    scoped_refptr<ArrayBufferVar> message_array_buffer =
-        ArrayBufferVar::FromPPVar(message);
-    if (!message_array_buffer)
+    // Convert message to WebArrayBuffer.
+    scoped_refptr<HostArrayBufferVar> host_message =
+        static_cast<HostArrayBufferVar*>(ArrayBufferVar::FromPPVar(message));
+    if (!host_message)
       return PP_ERROR_BADARGUMENT;
-    WebData web_message = WebData(
-        static_cast<const char*>(message_array_buffer->Map()),
-        static_cast<size_t>(message_array_buffer->ByteLength()));
-    if (!websocket_->sendBinary(web_message))
+    WebArrayBuffer& web_message = host_message->webkit_buffer();
+    if (!websocket_->sendArrayBuffer(web_message))
       return PP_ERROR_BADARGUMENT;
   } else {
     // TODO(toyoshim): Support Blob.
     return PP_ERROR_NOTSUPPORTED;
   }
 
   return PP_OK;
 }
 
 uint64_t PPB_WebSocket_Impl::GetBufferedAmount() {
@@ skipping 36 matching lines @@
 }
 
 PP_Var PPB_WebSocket_Impl::GetURL() {
   if (!url_)
     return empty_string_->GetPPVar();
   return url_->GetPPVar();
 }
 
 PP_Bool PPB_WebSocket_Impl::SetBinaryType(
     PP_WebSocketBinaryType_Dev binary_type) {
-  // TODO(toyoshim): Use WebKit new API to set the receiving binary type.
-  return PP_FALSE;
+  binary_type_ = binary_type;
+  if (websocket_.get())
+    websocket_->setBinaryType(
+        static_cast<WebSocket::BinaryType>(binary_type_));

[dmichael (off chromium), 2012/01/13 17:36:14]

Ditto, some better checking would be good here. Recall that SetBinaryType is
called by the plugin, and is free to send us bad values. It's very easy to send
us an enum to a value that's not actually valid.

[Takashi Toyoshima, 2012/01/17 10:14:59]

Done.

+  return PP_TRUE;
 }
 
 PP_WebSocketBinaryType_Dev PPB_WebSocket_Impl::GetBinaryType() {
   return binary_type_;
 }
 
 void PPB_WebSocket_Impl::didConnect() {
   DCHECK_EQ(PP_WEBSOCKETREADYSTATE_CONNECTING_DEV, state_);
   state_ = PP_WEBSOCKETREADYSTATE_OPEN_DEV;
   TrackedCallback::ClearAndRun(&connect_callback_, PP_OK);
 }
 
 void PPB_WebSocket_Impl::didReceiveMessage(const WebString& message) {
   // Dispose packets after receiving an error or in invalid state.
   if (error_was_received_ || !InValidStateToReceive(state_))
     return;
 
   // Append received data to queue.
   std::string string = message.utf8();
-  PP_Var var = StringVar::StringToPPVar(string);
-  received_messages_.push(var);
+  received_messages_.push(new StringVar(string));

[dmichael (off chromium), 2012/01/13 17:36:14]

So now the vars in your queue have no ID and no ref-count. That might be okay,
but you have to be careful about your use of the Vars and their ref-counts.

I think you probably need to at least use scoped_refptr to give it an internal
reference. I think as-written that you're leaking vars, since the queue has raw
pointers and you never increment the StringVar's ref count.

[Takashi Toyoshima, 2012/01/17 10:14:59]

Oh, sorry. I confused on managing ref counted objects in quieue.
I hold them as scoped_refptr in order to decrement reference at destruction of
queue or pop.

 
   if (!wait_for_receive_)
     return;
 
   TrackedCallback::ClearAndRun(&receive_callback_, DoReceive());
 }
 
-void PPB_WebSocket_Impl::didReceiveBinaryData(const WebData& binaryData) {
+void PPB_WebSocket_Impl::didReceiveArrayBuffer(
+    const WebArrayBuffer& binaryData) {
   // Dispose packets after receiving an error or in invalid state.
   if (error_was_received_ || !InValidStateToReceive(state_))
     return;
 
   // Append received data to queue.
-  PP_Var var = PpapiGlobals::Get()->GetVarTracker()->MakeArrayBufferPPVar(
-      binaryData.size());
-  scoped_refptr<ArrayBufferVar> arraybuffer = ArrayBufferVar::FromPPVar(var);
-  void* data = arraybuffer->Map();
-  memcpy(data, binaryData.data(), binaryData.size());
+  scoped_refptr<ArrayBufferVar> var = new HostArrayBufferVar(binaryData);
   received_messages_.push(var);

[dmichael (off chromium), 2012/01/13 17:36:14]

Does this compile, to pass a scoped_refptr when a raw pointer is expected? If
so, that seems like a big hole in scoped_refptr... I think in this case, the var
gets deleted when you exit scope, so you probably have use-after-free problems.

[Takashi Toyoshima, 2012/01/17 10:14:59]

Fixed.

 
   if (!wait_for_receive_)
     return;
 
   TrackedCallback::ClearAndRun(&receive_callback_, DoReceive());
 }
 
 void PPB_WebSocket_Impl::didReceiveMessageError() {
   // Ignore error notification in invalid state.
   if (!InValidStateToReceive(state_))
@@ skipping 62 matching lines @@
 
   // Disconnect.
   if (websocket_.get())
     websocket_->disconnect();
 }
 
 int32_t PPB_WebSocket_Impl::DoReceive() {
   if (!receive_callback_var_)
     return PP_OK;
 
-  *receive_callback_var_ = received_messages_.front();
+  *receive_callback_var_ = received_messages_.front()->GetPPVar();
   received_messages_.pop();
   receive_callback_var_ = NULL;
   wait_for_receive_ = false;
   return PP_OK;
 }
 
 }  // namespace ppapi
 }  // namespace webkit