WebSocket Pepper API: in process API base implementation

- Implement internal API and thunk
- Implement base frame for in process API
- Add basic unit tests for in process API

BUG=87310
TEST=ui_tests --gtest_filter="PPAPITest.WebSocket*"

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

[Takashi Toyoshima, 2011-11-16 08:07:05]

Hi,
First, I'd like to land the basic implementation for in process use.

This change includes internal API definition, thunk implementation,
basic plugins ppapi implementation (lack to use WebWebSocketChannel yet),
and some basic unit tests for them.

After this change and WebKit API landing, I push subsequent change
which implement in process interfaces to review.

Thanks!

[dmichael (off chromium), 2011-11-17 18:50:57]

Just a couple of nits. LGTM

http://codereview.chromium.org/8571002/diff/1029/chrome/test/ui/ppapi_uitest.cc
File chrome/test/ui/ppapi_uitest.cc (right):

http://codereview.chromium.org/8571002/diff/1029/chrome/test/ui/ppapi_uitest....
chrome/test/ui/ppapi_uitest.cc:390: TEST_PPAPI_IN_PROCESS(WebSocket)
I just sent an e-mail about the new way of putting tests in ppapi_uitest.cc... 
please list all your tests here before landing.

http://codereview.chromium.org/8571002/diff/1029/webkit/plugins/ppapi/ppb_web...
File webkit/plugins/ppapi/ppb_websocket_impl.h (right):

http://codereview.chromium.org/8571002/diff/1029/webkit/plugins/ppapi/ppb_web...
webkit/plugins/ppapi/ppb_websocket_impl.h:25: virtual
::ppapi::thunk::PPB_WebSocket_API* AsPPB_WebSocket_API();
Please use OVERRIDE on all these virtual function overrides.

[Takashi Toyoshima, 2011-11-18 05:01:19]

Thanks!

http://codereview.chromium.org/8571002/diff/1029/chrome/test/ui/ppapi_uitest.cc
File chrome/test/ui/ppapi_uitest.cc (right):

http://codereview.chromium.org/8571002/diff/1029/chrome/test/ui/ppapi_uitest....
chrome/test/ui/ppapi_uitest.cc:390: TEST_PPAPI_IN_PROCESS(WebSocket)
On 2011/11/17 18:50:57, dmichael wrote:
> I just sent an e-mail about the new way of putting tests in ppapi_uitest.cc...

> please list all your tests here before landing.

Done.

http://codereview.chromium.org/8571002/diff/1029/webkit/plugins/ppapi/ppb_web...
File webkit/plugins/ppapi/ppb_websocket_impl.h (right):

http://codereview.chromium.org/8571002/diff/1029/webkit/plugins/ppapi/ppb_web...
webkit/plugins/ppapi/ppb_websocket_impl.h:25: virtual
::ppapi::thunk::PPB_WebSocket_API* AsPPB_WebSocket_API();
On 2011/11/17 18:50:57, dmichael wrote:
> Please use OVERRIDE on all these virtual function overrides.

Done.

[commit-bot: I haz the power, 2011-11-18 05:09:58]

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

[commit-bot: I haz the power, 2011-11-18 05:10:25]

Presubmit check for 8571002-13001 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,webkit/plugins/ppapi/ppb_websocket_impl.h,webkit/plugins/ppapi/resource_creation_impl.cc,webkit/plugins/ppapi/resource_creation_impl.h,webkit/glue/webkit_glue.gypi

Presubmit checks took 13.1s to calculate.

[Takashi Toyoshima, 2011-11-18 11:21:36]

I notice that unit test registration is still wrong.
I forget to replace instance_->LogTest to RUN_TEST in TestWebSocket::RunTests.
As a result, test selector doesn't work and all test items run for each test
target.

I think it's not so harmfull, so I'd like to fix them in next change.

[dmichael (off chromium), 2011-11-18 16:22:21]

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

http://codereview.chromium.org/8571002/diff/15003/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:15: websocket_interface_ =
reinterpret_cast<PPB_WebSocket_Dev const*>(
static_cast (okay to do in next CL)

http://codereview.chromium.org/8571002/diff/15003/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:28: return "Could not create websocket via C
interface";
For what it's worth, we usually just user the 'ASSERT' macros that are in
test_case.h.

I had assumed you did this to get more meaningful error messages, but now I'm
guessing you just copied from an older test. If it's important to you to get
this error instead of a file and line number, I guess it's okay. But it might be
nice to prefer ASSERTs for consistency (and to make it more gtesty).

(Okay to do in next CL).

[dmichael (off chromium), 2011-11-18 16:22:46]

Oh yeah, still LGTM

[Takashi Toyoshima, 2011-11-19 00:01:19]

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

http://codereview.chromium.org/8571002/diff/15003/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:15: websocket_interface_ =
reinterpret_cast<PPB_WebSocket_Dev const*>(
Fixed in http://codereview.chromium.org/8558017/.

http://codereview.chromium.org/8571002/diff/15003/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:28: return "Could not create websocket via C
interface";
Also fixed in http://codereview.chromium.org/8558017/.
That change is not finished. But I'm happy if you could look into quickly.
I'm not sure on pepper related stuff, PP_Var's reference count handling, and
object conversion to WebKit specific objects.

I could take review on WebSocket specific code by yutak@.
He wrote WebCore version of WebSocket and one of OWNERS on net/websockets.
He works in the same timezone with me. So, I'd work with him at next Japanese
Monday.