WebSocket Pepper API: ArrayBuffer support

WebSocket Pepper API: ArrayBuffer support

This CL supports basic binary communication.
For now, communication between Chrome and WebKit uses WebData structure.
It must use WebArrayBuffer directly after WebKit API revision.

BUG=87310
TEST=ui_tests --gtest_filter='PPAPITest.WebSocket_BinarySendReceive'


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=115871

[Takashi Toyoshima, 2011-12-02 13:27:18]

This change depends following patch for WebKit.

diff --git a/LayoutTests/http/tests/websocket/tests/hybi/echo_wsh.py
b/LayoutTests/http/tests/websocket/tests/hybi/echo_wsh.py
index 79c3f10..9851d26 100644
--- a/LayoutTests/http/tests/websocket/tests/hybi/echo_wsh.py
+++ b/LayoutTests/http/tests/websocket/tests/hybi/echo_wsh.py
@@ -1,4 +1,4 @@
-# Copyright 2009, Google Inc.
+# Copyright 2009, 2011, Google Inc.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -28,10 +28,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
-from mod_pywebsocket import msgutil
-
-
-_GOODBYE_MESSAGE = 'Goodbye'
+_GOODBYE_MESSAGE = u'Goodbye'
 
 
 def web_socket_do_extra_handshake(request):
@@ -40,7 +37,12 @@ def web_socket_do_extra_handshake(request):
 
 def web_socket_transfer_data(request):
     while True:
-        line = msgutil.receive_message(request)
-        msgutil.send_message(request, line)
-        if line == _GOODBYE_MESSAGE:
+        line = request.ws_stream.receive_message()
+        if line is None:
             return
+        if isinstance(line, unicode):
+            request.ws_stream.send_message(line, binary=False)
+            if line == _GOODBYE_MESSAGE:
+                return
+        else:
+            request.ws_stream.send_message(line, binary=True)

[Takashi Toyoshima, 2011-12-05 08:08:26]

Comment #1 patch is already landed in WebKit side.

[Takashi Toyoshima, 2011-12-16 08:41:52]

Copy of memo on Patch Set 2:

For now, this change depends on following ongoing changes.
 - http://codereview.chromium.org/8502030/ (Patch Set 5 + Comment #3 Patch)
 - https://bugs.webkit.org/show_bug.cgi?id=73593 [landed]
 - https://bugs.webkit.org/show_bug.cgi?id=73649, or Comment #1 of this Issue
[landed]

And it doesn't depends on following change, but it's better to be landed after
them.
 - http://codereview.chromium.org/8772001/ [landed]

[Takashi Toyoshima, 2011-12-20 04:40:21]

I update this change by using landed ArrayBuffer support.
Now, I'm updating WebKit side API in order to use various binary type like
WebData, and WebArrayBuffer.
First, I'll land this change to support binary data in pepper bindings.

[dmichael (off chromium), 2011-12-20 17:11:04]

lgtm

http://codereview.chromium.org/8774042/diff/15001/ppapi/tests/test_websocket.cc
File ppapi/tests/test_websocket.cc (right):

http://codereview.chromium.org/8774042/diff/15001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:468: ASSERT_FALSE(result != PP_OK && result !=
PP_OK_COMPLETIONPENDING);
optional: I think:
ASSERT_TRUE(result == PP_OK || result == PP_OK_COMPLETIONPENDING);
would be more readable.

http://codereview.chromium.org/8774042/diff/15001/webkit/plugins/ppapi/ppb_we...
File webkit/plugins/ppapi/ppb_websocket_impl.cc (right):

http://codereview.chromium.org/8774042/diff/15001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:350: // directory.
directory->directly?

[Takashi Toyoshima, 2011-12-21 07:45:57]

Thank you!

http://codereview.chromium.org/8774042/diff/15001/ppapi/tests/test_websocket.cc
File ppapi/tests/test_websocket.cc (right):

http://codereview.chromium.org/8774042/diff/15001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:468: ASSERT_FALSE(result != PP_OK && result !=
PP_OK_COMPLETIONPENDING);
On 2011/12/20 17:11:06, dmichael wrote:
> optional: I think:
> ASSERT_TRUE(result == PP_OK || result == PP_OK_COMPLETIONPENDING);
> would be more readable.

Done.

http://codereview.chromium.org/8774042/diff/15001/webkit/plugins/ppapi/ppb_we...
File webkit/plugins/ppapi/ppb_websocket_impl.cc (right):

http://codereview.chromium.org/8774042/diff/15001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:350: // directory.
On 2011/12/20 17:11:06, dmichael wrote:
> directory->directly?

Done.

[commit-bot: I haz the power, 2011-12-21 07:46:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/8774042/23001

[commit-bot: I haz the power, 2011-12-21 07:46:24]

Presubmit check for 8774042-23001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for: webkit/plugins/ppapi/ppb_websocket_impl.cc

Presubmit checks took 1.2s to calculate.

[Takashi Toyoshima, 2011-12-21 07:49:36]

Brett and Piman,
Could you give me a LGTM stamp for webkit/plugins/ppapi ?

[brettw, 2011-12-21 17:55:22]

lgtm

[commit-bot: I haz the power, 2011-12-22 03:34:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/8774042/23001

[commit-bot: I haz the power, 2011-12-22 05:36:41]

Try job failure for 8774042-23001 (retry) on mac_rel for steps "ui_tests,
webkit_unit_tests" (clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Takashi Toyoshima, 2011-12-22 08:15:20]

Binary communication via C++ interface seems to fail constantly in mac.
I'll investigate it.

[Takashi Toyoshima, 2011-12-22 11:33:47]

I could not reproduce this issue in my local environment, yet.
I try it in trybots again.

[commit-bot: I haz the power, 2011-12-22 13:13:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/8774042/35001

[commit-bot: I haz the power, 2011-12-22 14:56:43]

Try job failure for 8774042-35001 (retry) on win_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2011-12-22 15:12:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/8774042/35001

[Takashi Toyoshima, 2011-12-23 02:10:54]

Only bots's mac_rel build cause error on binary data communication via
C++interface at high rate. On the other hand, local release build could not
reproduce this failure at all.

In the bots, receiving binary data contents don't match with the expectation.
Local valgrind running could not find any memory usage violations.
The problem seems to be a little complex.

How about landing this change with disabling CcInterfaces test in Mac port?

[Takashi Toyoshima, 2011-12-26 13:53:42]

I caught the issue.
ReceiveMessage had a bug on asynchronous returning path.
I must set receiving buffer 'message' to receive_callback_var_ before calling
DoReceive().

I'm sorry for the bug, but could you give me a LGTM again?

Thanks and happy holidays!!

[dmichael (off chromium), 2011-12-27 16:45:29]

lgtm

[commit-bot: I haz the power, 2011-12-28 02:16:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/8774042/38006

[commit-bot: I haz the power, 2011-12-28 02:52:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/8774042/38006

[commit-bot: I haz the power, 2011-12-28 04:36:03]

Try job failure for 8774042-38006 (retry) on win_rel for steps "browser_tests,
ui_tests".
It's a second try, previously, steps "browser_tests, ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2011-12-28 04:38:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/8774042/38006

[Takashi Toyoshima, 2011-12-28 04:39:51]

win_rel got the known issue on bots.
http://code.google.com/p/chromium/issues/detail?id=107836
I'll retry.

[commit-bot: I haz the power, 2011-12-28 05:51:33]

Change committed as 115871