WebSocket Pepper API: in process API implementation (reland)

WebSocket Pepper API: in process API implementation

This change enable in process Websocket Pepper API.
For now, some unit tests are disabled because they need external a
WebSocket server.

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

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

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

[Yuta Kitamura, 2011-11-21 05:49:21]

WebSocket stuff lgtm with nits.

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

http://codereview.chromium.org/8558017/diff/1007/ppapi/tests/test_websocket.c...
ppapi/tests/test_websocket.cc:70: // TODO(toyoshim): For now, following
WebSocket server set up manually.
nit: "following WebSocket server must be started manually"?

http://codereview.chromium.org/8558017/diff/1007/ppapi/tests/test_websocket.h
File ppapi/tests/test_websocket.h (right):

http://codereview.chromium.org/8558017/diff/1007/ppapi/tests/test_websocket.h...
ppapi/tests/test_websocket.h:26: void ReleaseVar(PP_Var& var);
Use of non-const reference is not allowed; you should use const reference or
non-const pointer.

http://codereview.chromium.org/8558017/diff/1007/webkit/plugins/ppapi/ppb_web...
File webkit/plugins/ppapi/ppb_websocket_impl.cc (right):

http://codereview.chromium.org/8558017/diff/1007/webkit/plugins/ppapi/ppb_web...
webkit/plugins/ppapi/ppb_websocket_impl.cc:200: // unpaired surrogates.
WebSocket API spec mentions unpaired surrogates because DOMString is UTF-16; in
theory, UTF-8 strings don't have any surrogates, so we may reject a string
containing any surrogate (paired or unpaired) in it.

cf. http://unicode.org/faq/utf_bom.html#utf8-4

[Takashi Toyoshima, 2011-11-21 09:00:45]

Thanks, Yuta.

Could anyone being familiar with pepper review on this?
Actually, this change lack some minor features or checker implementation.
But, I'd like to land this partially after WebKit roll of r100849.

After that, I'll enable WebSocket server launcher, then refine non-proxy
implementation and add unit tests more.

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

http://codereview.chromium.org/8558017/diff/1007/ppapi/tests/test_websocket.c...
ppapi/tests/test_websocket.cc:70: // TODO(toyoshim): For now, following
WebSocket server set up manually.
On 2011/11/21 05:49:22, Yuta Kitamura wrote:
> nit: "following WebSocket server must be started manually"?

Done.

http://codereview.chromium.org/8558017/diff/1007/ppapi/tests/test_websocket.h
File ppapi/tests/test_websocket.h (right):

http://codereview.chromium.org/8558017/diff/1007/ppapi/tests/test_websocket.h...
ppapi/tests/test_websocket.h:26: void ReleaseVar(PP_Var& var);
I fixed it as not pointer but const reference because compiler allow to use
const PP_Var& here.
But, I don't have a confidence that releasing binding context really could be
handled as const.

I'd like to know your idea.

http://codereview.chromium.org/8558017/diff/1007/webkit/plugins/ppapi/ppb_web...
File webkit/plugins/ppapi/ppb_websocket_impl.cc (right):

http://codereview.chromium.org/8558017/diff/1007/webkit/plugins/ppapi/ppb_web...
webkit/plugins/ppapi/ppb_websocket_impl.cc:200: // unpaired surrogates.
On 2011/11/21 05:49:22, Yuta Kitamura wrote:
> WebSocket API spec mentions unpaired surrogates because DOMString is UTF-16;
in
> theory, UTF-8 strings don't have any surrogates, so we may reject a string
> containing any surrogate (paired or unpaired) in it.
> 
> cf. http://unicode.org/faq/utf_bom.html#utf8-4

Done.

[Takashi Toyoshima, 2011-11-23 05:10:58]

Hi Dave,

Could you review this change especially on pepper related stuff?
I landed a change to launch WebSocket server in ppapi ui tests.
So, I use it for connection tests in this change.

These tests are still disabled because of
http://codereview.chromium.org/8633004/.
But, it will be landed and enable all related tests soon.

[dmichael (off chromium), 2011-11-23 17:41:24]

Since I'm already so late getting comments to you, I'll send a batch now. I'm
still looking, though.

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

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:55: bool TestWebSocket::Compare(const PP_Var& var,
const char* string) {
nit: Maybe AreEqual would be a better name, so it's clear what the return value
means.

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:116: ws, PP_MakeUndefined(), undefined_protocols,
1, PP_BlockUntilComplete());
PP_BlockUntilComplete is not allowed on the main thread, so using it should
suffice to make result be. You might want to use a non-blocking
TestCompletionCallback callback, since as-is, it might be equally valid for
Connect to return PP_ERROR_BLOCKS_MAIN_THREAD.

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:121: ASSERT_EQ(PP_ERROR_INPROGRESS, result);
Should it really return 'INPROGRESS'? I would have expected the previous call to
Connect to have taken no action, so I might have expected |ws| to be in the same
state as before.

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:174: PP_Var message_var = CreateVar(message);
Optional: You might save a little bit of coding if you used pp::Var from
ppapi/cpp/var.h. It has an appropriate constructor for string literals and will
release the var automatically when it goes out of scope.

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

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:52:
pp::Module::Get()->GetBrowserInterface(PPB_VAR_INTERFACE));
In implementation code, we should access the VarTracker directly (e.g.,
PpapiGlobals::Get()->GetVarTracker()) rather than using the var interface.

Eventually, all the implementation code will be behind a lock, and attempting to
use the interface inside the implementation would lock again (and we have no
recursive locks).

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:55: empty_var_ = PP_MakeUndefined();
And maybe empty_var_ should be a scoped_refptr<StringVar>. I don't think you
should need a case where you can't create an empty string. Just use
StringVar::StringToPPVar

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:75: var_interface_->Release(var);
*if* each received_message is really just a string, you should make the queue
contain scoped_refptr<StringVar> (then this loop would be unnecessary and some
other code should get cleaner too).

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:338: pp::Module::Get()->pp_module(),
string.data(), string.length());
Does the message really just represent a string? Or is it really meant to be
interpreted as a SerializedScriptValue?

If it's really just a String, you should use StringVar.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
File webkit/plugins/ppapi/ppb_websocket_impl.h (right):

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.h:58: // WebSocketClient
nit: "WebSocketClient implementation." ?

[dmichael (off chromium), 2011-11-23 18:29:57]

Okay, done reviewing for now. Sorry again for the delay.

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

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:92: PP_CompletionCallback callback) {
If callback is a blocking completion callback, you should return
PP_ERROR_BLOCKS_MAIN_THREAD and do nothing.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:183: PP_CompletionCallback callback)
{
Same as above, you should not allow a blocking completion callback.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:219: "WebSocket is closed before the
connection is established.");
is->was, is->was ?

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:247: // Or retain |message| as buffer
to store and install |callbak|.
nit: callbak->callback

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:250: receive_callback_ = callback;
I don't see where these 3 get cleared. Should that happen in DoReceive?

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:354: puts("didReceiveMessageError");
please don't use stdio directly. DLOG(INFO)?

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:380: state_ =
PP_WEBSOCKETREADYSTATE_CLOSED_DEV;
I don't see where you ever invoke close_callback_? Should that happen in this
method?

[Takashi Toyoshima, 2011-11-24 03:55:48]

Thanks.

I fixed commented points.
And now we can safely enabled tests using WebSocket server.

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

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:55: bool TestWebSocket::Compare(const PP_Var& var,
const char* string) {
On 2011/11/23 17:41:24, dmichael wrote:
> nit: Maybe AreEqual would be a better name, so it's clear what the return
value
> means.

Done.

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:116: ws, PP_MakeUndefined(), undefined_protocols,
1, PP_BlockUntilComplete());
This callback is totally dummy and never used in websocket_interface_.
That's why I just use PP_BlockUntilComplete().
Anyway, I use TestCompletionCallback for safe.

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:121: ASSERT_EQ(PP_ERROR_INPROGRESS, result);
We define that Connect must be called at most once.
So, I intendedly return PP_ERROR_INPROGRESS for the second and later call.

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:174: PP_Var message_var = CreateVar(message);
Thank you for good advice.
I'm planning to prepare C++ interface for WebSocket API.
So later, I use cpp/var.h in tests for C++ interface.

These tests check C interface.
If these tests use only C interfaces for other modules, it could be good example
for C programmer.

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

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:52:
pp::Module::Get()->GetBrowserInterface(PPB_VAR_INTERFACE));
On 2011/11/23 17:41:24, dmichael wrote:
> In implementation code, we should access the VarTracker directly (e.g.,
> PpapiGlobals::Get()->GetVarTracker()) rather than using the var interface.
> 
> Eventually, all the implementation code will be behind a lock, and attempting
to
> use the interface inside the implementation would lock again (and we have no
> recursive locks).

Done.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:55: empty_var_ = PP_MakeUndefined();
On 2011/11/23 17:41:24, dmichael wrote:
> And maybe empty_var_ should be a scoped_refptr<StringVar>. I don't think you
> should need a case where you can't create an empty string. Just use
> StringVar::StringToPPVar

Done.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:75: var_interface_->Release(var);
For now, it is true.
But, we must handle binary data in the future.
So, I'd like to keep this clean up code untouched.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:92: PP_CompletionCallback callback) {
I'd like to valid after changing state_, because I want to ensure that the
second and later call must be failed with PP_ERROR_INPROGRESS.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:183: PP_CompletionCallback callback)
{
On 2011/11/23 18:29:57, dmichael wrote:
> Same as above, you should not allow a blocking completion callback.

Done.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:219: "WebSocket is closed before the
connection is established.");
OK, I fixed.

Yuta,
I think WebCore::WebSocket has the same message for the equivalent case.
Should we fix it, too?

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:247: // Or retain |message| as buffer
to store and install |callbak|.
On 2011/11/23 18:29:57, dmichael wrote:
> nit: callbak->callback

Done.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:250: receive_callback_ = callback;
On 2011/11/23 18:29:57, dmichael wrote:
> I don't see where these 3 get cleared. Should that happen in DoReceive?

Done.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:338: pp::Module::Get()->pp_module(),
string.data(), string.length());
In the future, we must handle binary data in didReceiveBinaryData().
And receiving order between text and binary must be reserved.
It mean we must hold these data in the same queue, or received_message_.

That's why I decide to hold these data as PP_Var.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:354: puts("didReceiveMessageError");
Sorry, I miss removing them.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:380: state_ =
PP_WEBSOCKETREADYSTATE_CLOSED_DEV;
Oh, sorry.
I must invoke around here.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
File webkit/plugins/ppapi/ppb_websocket_impl.h (right):

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.h:58: // WebSocketClient
On 2011/11/23 17:41:24, dmichael wrote:
> nit: "WebSocketClient implementation." ?

Done.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.h:59: // TODO(toyoshim): Check if
OVERRIDE is needed for following methods.
Oh, I forget to remove this TODO.
I think following virtual functions need OVERRIDE, right?

[dmichael (off chromium), 2011-11-24 04:32:36]

I think you need to validate protocols[] vars, but otherwise LGTM

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

http://codereview.chromium.org/8558017/diff/16001/ppapi/tests/test_websocket....
ppapi/tests/test_websocket.cc:116: ws, PP_MakeUndefined(), undefined_protocols,
1, PP_BlockUntilComplete());
On 2011/11/24 03:55:49, toyoshim wrote:
> This callback is totally dummy and never used in websocket_interface_.
> That's why I just use PP_BlockUntilComplete().
> Anyway, I use TestCompletionCallback for safe.
I understand it's a dummy, but the blocking completion callback would mean you
have at least 2 different ways the call can fail with different error codes, so
you wouldn't be guaranteed to get PP_ERROR_BADARGUMENT below, even with a valid
implementation.

Thanks for fixing.

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

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:75: var_interface_->Release(var);
On 2011/11/24 03:55:49, toyoshim wrote:
> For now, it is true.
> But, we must handle binary data in the future.
> So, I'd like to keep this clean up code untouched.
The binary data will be ArrayBuffer and Blob, and they'll also inherit from Var.
I'm working on ArrayBuffer support now. (By the way, do you think ArrayBuffer
would suffice for the first release of the interface? I'm guessing it would be
good enough).

Anyway, it would probably make sense to use std::queue<scoped_refptr<Var> >, but
I guess it's not a big deal if you want to use PP_Var.

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
File webkit/plugins/ppapi/ppb_websocket_impl.h (right):

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.h:59: // TODO(toyoshim): Check if
OVERRIDE is needed for following methods.
On 2011/11/24 03:55:49, toyoshim wrote:
> Oh, I forget to remove this TODO.
> I think following virtual functions need OVERRIDE, right?
Since they're overriding WebKit functions it's optional, but preferred to leave
them if you don't expect to change the WebKit API in the near future.

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

http://codereview.chromium.org/8558017/diff/21001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:125: return PP_ERROR_BADARGUMENT;
Shouldn't we also check that string_var is valid (that is, the protocol was a
string var) before this?

[Takashi Toyoshima, 2011-11-25 05:42:48]

Thanks.
I'll land with minor update, so I added you as a TBR.

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

http://codereview.chromium.org/8558017/diff/16001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:75: var_interface_->Release(var);
Oh, it sounds very nice that ArrayBuffer and Blob inherit from Var.

For now, I'll commit with current clean up code, but I'll fix it as Var at the
time I update it to handle binary.

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

http://codereview.chromium.org/8558017/diff/21001/webkit/plugins/ppapi/ppb_we...
webkit/plugins/ppapi/ppb_websocket_impl.cc:125: return PP_ERROR_BADARGUMENT;
On 2011/11/24 04:32:36, dmichael wrote:
> Shouldn't we also check that string_var is valid (that is, the protocol was a
> string var) before this?

Done.

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

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

[commit-bot: I haz the power, 2011-11-25 06:08:42]

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

[commit-bot: I haz the power, 2011-11-25 06:21:22]

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

[Takashi Toyoshima, 2011-11-25 07:37:31]

Hum.., build on bots fails.
But, I could not reproduce it in my local machine.
Should I use PpapiGlobals::GetModuleForInstance() to get PP_MODULE?

[commit-bot: I haz the power, 2011-11-25 07:47:54]

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

[commit-bot: I haz the power, 2011-11-25 10:09:03]

Try job failure for 8558017-27002 (retry) (retry) on linux_rel for steps
"check_deps, interactive_ui_tests".
It's a second try, previously, steps "check_deps, interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

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

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

[commit-bot: I haz the power, 2011-11-25 11:40:44]

Try job failure for 8558017-27002 (retry) on linux_rel for steps "check_deps,
interactive_ui_tests".
It's a second try, previously, steps "check_deps, interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[Takashi Toyoshima, 2011-11-25 14:53:17]

The last problems seem not to be related to this change.
I used dcommit to land.

[Takashi Toyoshima, 2011-11-25 15:17:24]

Change was reverted, so I reopen this review.

[Takashi Toyoshima, 2011-11-25 15:40:19]

This revert related to build with shared library.
I try to reproduce the problem on landing by using git try -b linux_shared.

Also this change has a problem on check_deps.
http://build.chromium.org/p/chromium/builders/XP%20Tests%20%28dbg%29%281%29/b...
Shouldn't I include ppapi/cpp/module.h ? These headers in ppapi/cpp exist only
for nacl-g++?

[dmichael (off chromium), 2011-11-25 16:19:11]

Oops, should've caught that in review. You can't use anything from
ppapi/cpp, except in ppapi/tests. You probably just want
ppapi/c/pp_module.h.

On Fri, Nov 25, 2011 at 8:40 AM, <toyoshim@chromium.org> wrote:

> This revert related to build with shared library.
> I try to reproduce the problem on landing by using git try -b linux_shared.
>
> Also this change has a problem on check_deps.
> http://build.chromium.org/p/**chromium/builders/XP%20Tests%**
>
20%28dbg%29%281%29/builds/**16005/steps/check_deps/logs/**stdio<http://build.chromium.org/p/chromium/builders/XP%20Tests%20%28dbg%29%281%29/builds/16005/steps/check_deps/logs/stdio>
> Shouldn't I include ppapi/cpp/module.h ? These headers in ppapi/cpp exist
> only
> for nacl-g++?
>
>
http://codereview.chromium.**org/8558017/<http://codereview.chromium.org/8558...
>

[commit-bot: I haz the power, 2011-11-26 01:08:02]

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

[commit-bot: I haz the power, 2011-11-26 01:08:06]

Presubmit check for 8558017-27003 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

Presubmit checks took 1.8s to calculate.