Permalink
Browse files

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
  • Loading branch information...
1 parent 146180a commit 3f915fae62435f901f520286c7afeddb26644549 toyoshim@chromium.org committed Mar 26, 2012
@@ -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)
@@ -144,14 +144,19 @@ 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.
*pp_error = PPBWebSocketInterface()->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;
@@ -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;
@@ -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();

0 comments on commit 3f915fa

Please sign in to comment.