WebSocket Pepper API: synchronous completion support

ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_websocket_rpc_server.cc

Index: ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_websocket_rpc_server.cc
diff --git a/ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_websocket_rpc_server.cc b/ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_websocket_rpc_server.cc
index 7d19e1ae4b9802cda5203574bcac0bd19c1baf9c..37892e80834aae59f758ee7f104509463ad830b8 100644
--- a/ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_websocket_rpc_server.cc
+++ b/ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_websocket_rpc_server.cc
@@ -19,6 +19,7 @@ using ppapi_proxy::DeleteRemoteCallbackInfo;
 using ppapi_proxy::DeserializeTo;
 using ppapi_proxy::MakeRemoteCompletionCallback;
 using ppapi_proxy::PPBWebSocketInterface;
+using ppapi_proxy::Serialize;
 using ppapi_proxy::SerializeTo;
 
 void PpbWebSocketRpcServer::PPB_WebSocket_Create(
@@ -135,7 +136,9 @@ void PpbWebSocketRpcServer::PPB_WebSocket_ReceiveMessage(
     PP_Resource ws,
     int32_t callback_id,
     // outputs
-    int32_t* pp_error) {
+    int32_t* pp_error,
+    nacl_abi_size_t* async_read_size,
+    char* async_read_bytes) {
   NaClSrpcClosureRunner runner(done);
   rpc->result = NACL_SRPC_RESULT_APP_ERROR;
 
@@ -144,11 +147,6 @@ void PpbWebSocketRpcServer::PPB_WebSocket_ReceiveMessage(
       MakeRemoteCompletionCallback(rpc->channel, callback_id, &callback_var);
   if (NULL == remote_callback.func)
     return;
-  // TODO(toyoshim): Removing optional flag is easy way to expect asynchronous
-  // completion on the following PPBWebSocketInterface()->ReceiveMessage(). But
-  // from the viewpoint of performance, we should handle synchronous
-  // completion correctly.
-  remote_callback.flags &= ~PP_COMPLETIONCALLBACK_FLAG_OPTIONAL;
 
   // The callback is always invoked asynchronously for now, so it doesn't care
   // about re-entrancy.

[dmichael (off chromium), 2012/04/02 03:28:01]

This comment's not valid anymore, right?

[Takashi Toyoshima, 2012/04/05 06:44:00]

Right. I remove this comment.

@@ -156,10 +154,23 @@ void PpbWebSocketRpcServer::PPB_WebSocket_ReceiveMessage(
       ws, callback_var, remote_callback);
   DebugPrintf("PPB_WebSocket::ReceiveMessage: pp_error=%"NACL_PRId32"\n",
       *pp_error);
-  CHECK(*pp_error != PP_OK);  // Should not complete synchronously
-  if (*pp_error != PP_OK_COMPLETIONPENDING)
-    DeleteRemoteCallbackInfo(remote_callback);
   rpc->result = NACL_SRPC_RESULT_OK;
+
+  // No callback scheduled. Handles synchronous completion here.
+  if (*pp_error != PP_OK_COMPLETIONPENDING) {
+    if (*pp_error == PP_OK) {
+      // Try serialization from callback_var to async_read_bytes. It could fail
+      // if serialized callback_var is larger than async_read_size.
+      if (!SerializeTo(callback_var, async_read_bytes, async_read_size)) {
+        // Buffer for asynchronous completion is not enough. Uses asynchronous
+        // completion callback.
+        *pp_error = PP_OK_COMPLETIONPENDING;
+        // TODO(toyoshim): Schedule to invoke callback_id here.

[Takashi Toyoshima, 2012/03/28 12:05:47]

I could not find out how can I schedule remote_callback to be invoked later.

[dmichael (off chromium), 2012/04/02 03:28:01]

Could you maybe use PPB_Core.CallOnMainThread? It might be tricky to make sure
it won't misbehave if the WebSocket gets deleted before that call. You can
actually use CompletionCallbackFactory here, though, so maybe that would work.

Alternatively...  I think URLLoader has the same 64k limitation, right? Is it
perhaps acceptable to just make that limitation for now? The  Chrome IPC proxy
won't have that limitation, so it would be temporary. Just a thought.

[Takashi Toyoshima, 2012/04/05 06:44:00]

Thanks!
If I can use CallIOnMainThread here, it looks not so difficult.
I believe I can reuse remote_callback for CallOnMainThread... and looks working
fine to me:)

For test purpose, I removed SerializeTo() and called CallOnMainThread(0,
remote_callback, PP_OK) every time. It works fine. So I'd like to use it in case
of SerializeTo() failures.

+        return;
+      }
+    }
+    DeleteRemoteCallbackInfo(remote_callback);
+  }
 }
 
 void PpbWebSocketRpcServer::PPB_WebSocket_SendMessage(