WebSocket Pepper API: fix data corruption at ReceiveMessage in NaCl

PpbWebSocketRpcServer::PPB_WebSocket_ReceiveMessage sometime gets unexpected
synchronous PP_OK result on PPBWebSocketInterface calling. Receiving data is
passed only in completion callbacks. Then, it causes data corruption.

This CL fix this issue and in order to reproduce this synchronous PP_OK situation,
add one stress test, StressedSendReceive.

This CL also fix CcInterface test flakiness on Mac.

BUG=111636
TEST=browser_tests --test_filter='PPAPI*Test.WebSocket_CcInterfaces'; browser_tests --test_filter='PPAPI*Test.WebSocket_StressedSendReceive'

Review URL: http://codereview.chromium.org/9724007

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

4 files changed, 60 insertions, 10 deletions

diff --git a/chrome/test/ui/ppapi_uitest.cc b/chrome/test/ui/ppapi_uitest.cc
index 83ff2514..4d90ac9 100644
--- a/chrome/test/ui/ppapi_uitest.cc
+++ b/chrome/test/ui/ppapi_uitest.cc
@@ -881,13 +881,6 @@ TEST_PPAPI_OUT_OF_PROCESS(Flash_MessageLoop)
 TEST_PPAPI_OUT_OF_PROCESS(Flash_GetLocalTimeZoneOffset)
 TEST_PPAPI_OUT_OF_PROCESS(Flash_GetCommandLineArgs)
 
-// Intermittently fails on OSX. http://crbug.com/111636
-#if defined(OS_MACOSX)
-#define MAYBE_WebSocket_CcInterfaces DISABLED_WebSocket_CcInterfaces
-#else
-#define MAYBE_WebSocket_CcInterfaces WebSocket_CcInterfaces
-#endif
-
 TEST_PPAPI_IN_PROCESS(WebSocket_IsWebSocket)
 TEST_PPAPI_IN_PROCESS(WebSocket_UninitializedPropertiesAccess)
 TEST_PPAPI_IN_PROCESS(WebSocket_InvalidConnect)
@@ -899,8 +892,9 @@ TEST_PPAPI_IN_PROCESS_WITH_WS(WebSocket_ValidClose)
 TEST_PPAPI_IN_PROCESS_WITH_WS(WebSocket_GetProtocol)
 TEST_PPAPI_IN_PROCESS_WITH_WS(WebSocket_TextSendReceive)
 TEST_PPAPI_IN_PROCESS_WITH_WS(WebSocket_BinarySendReceive)
+TEST_PPAPI_IN_PROCESS_WITH_WS(WebSocket_StressedSendReceive)
 TEST_PPAPI_IN_PROCESS_WITH_WS(WebSocket_BufferedAmount)
-TEST_PPAPI_IN_PROCESS_WITH_WS(MAYBE_WebSocket_CcInterfaces)
+TEST_PPAPI_IN_PROCESS_WITH_WS(WebSocket_CcInterfaces)
 TEST_PPAPI_IN_PROCESS(WebSocket_UtilityInvalidConnect)
 TEST_PPAPI_IN_PROCESS(WebSocket_UtilityProtocols)
 TEST_PPAPI_IN_PROCESS(WebSocket_UtilityGetURL)
@@ -922,8 +916,9 @@ TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(WebSocket_ValidClose)
 TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(WebSocket_GetProtocol)
 TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(WebSocket_TextSendReceive)
 TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(WebSocket_BinarySendReceive)
+TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(WebSocket_StressedSendReceive)
 TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(WebSocket_BufferedAmount)
-TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(MAYBE_WebSocket_CcInterfaces)
+TEST_PPAPI_NACL_VIA_HTTP_WITH_WS(WebSocket_CcInterfaces)
 TEST_PPAPI_NACL_VIA_HTTP(WebSocket_UtilityInvalidConnect)
 TEST_PPAPI_NACL_VIA_HTTP(WebSocket_UtilityProtocols)
 TEST_PPAPI_NACL_VIA_HTTP(WebSocket_UtilityGetURL)
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 65c0877..7d19e1a 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
@@ -144,6 +144,11 @@ 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.
@@ -151,7 +156,7 @@ 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;
diff --git a/ppapi/tests/test_websocket.cc b/ppapi/tests/test_websocket.cc
index 1992e3e..69502a1 100644
--- a/ppapi/tests/test_websocket.cc
+++ b/ppapi/tests/test_websocket.cc
@@ -196,6 +196,7 @@ void TestWebSocket::RunTests(const std::string& filter) {
   RUN_TEST_WITH_REFERENCE_CHECK(GetProtocol, filter);
   RUN_TEST_WITH_REFERENCE_CHECK(TextSendReceive, filter);
   RUN_TEST_WITH_REFERENCE_CHECK(BinarySendReceive, filter);
+  RUN_TEST_WITH_REFERENCE_CHECK(StressedSendReceive, filter);
   RUN_TEST_WITH_REFERENCE_CHECK(BufferedAmount, filter);
 
   RUN_TEST_WITH_REFERENCE_CHECK(CcInterfaces, filter);
@@ -637,6 +638,54 @@ std::string TestWebSocket::TestBinarySendReceive() {
   PASS();
 }
 
+std::string TestWebSocket::TestStressedSendReceive() {
+  // Connect to test echo server.
+  int32_t connect_result;
+  PP_Resource ws = Connect(GetFullURL(kEchoServerURL), &connect_result, "");
+  ASSERT_TRUE(ws);
+  ASSERT_EQ(PP_OK, connect_result);
+
+  // Prepare PP_Var objects to send.
+  const char* text = "hello pepper";
+  PP_Var text_var = CreateVarString(text);
+  std::vector<uint8_t> binary(256);
+  for (uint32_t i = 0; i < binary.size(); ++i)
+    binary[i] = i;
+  PP_Var binary_var = CreateVarBinary(binary);
+
+  // Send many messages.
+  for (int i = 0; i < 256; ++i) {
+    int32_t result = websocket_interface_->SendMessage(ws, text_var);
+    ASSERT_EQ(PP_OK, result);
+    result = websocket_interface_->SendMessage(ws, binary_var);
+    ASSERT_EQ(PP_OK, result);
+  }
+  ReleaseVar(text_var);
+  ReleaseVar(binary_var);
+
+  // Receive echoed data.
+  for (int i = 0; i < 512; ++i) {
+    TestCompletionCallback callback(instance_->pp_instance(), force_async_);
+    PP_Var received_message;
+    int32_t result = websocket_interface_->ReceiveMessage(
+        ws, &received_message, static_cast<pp::CompletionCallback>(
+            callback).pp_completion_callback());
+    ASSERT_TRUE(result == PP_OK || result == PP_OK_COMPLETIONPENDING);
+    if (result == PP_OK_COMPLETIONPENDING)
+      result = callback.WaitForResult();
+    ASSERT_EQ(PP_OK, result);
+    if (i & 1) {
+      ASSERT_TRUE(AreEqualWithBinary(received_message, binary));
+    } else {
+      ASSERT_TRUE(AreEqualWithString(received_message, text));
+    }
+    ReleaseVar(received_message);
+  }
+  core_interface_->ReleaseResource(ws);
+
+  PASS();
+}
+
 std::string TestWebSocket::TestBufferedAmount() {
   // Connect to test echo server.
   int32_t connect_result;
diff --git a/ppapi/tests/test_websocket.h b/ppapi/tests/test_websocket.h
index 3b8f38d..fb6ea9bb 100644
--- a/ppapi/tests/test_websocket.h
+++ b/ppapi/tests/test_websocket.h
@@ -45,6 +45,7 @@ class TestWebSocket : public TestCase {
   std::string TestGetProtocol();
   std::string TestTextSendReceive();
   std::string TestBinarySendReceive();
+  std::string TestStressedSendReceive();
   std::string TestBufferedAmount();
 
   std::string TestCcInterfaces();