WebSocket Pepper API: the second Close() should not return error code.

WebSocket Pepper API: the second Close() should not return error code.

The WebSocket API defines that call to Close() on CLOSED state must do nothing
and cause no error. Currently Pepper API returns PP_ERROR_INPROGRESS. This
error code is confusing. We should return PP_OK here.

BUG=124866
TEST=browser_tests --gtest_filter='*WebSocket*'


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

[Takashi Toyoshima, 2012-04-24 19:48:20]

Hi David,
Could you look into this change?
When user call to Close() and call it again before its callback is invoked, the
second call returns PP_ERROR_INPROGRESS. But if the second call was done after
the callback invocation, the second call return PP_ERROR_INPROGRESS.
At least, this error code is wrong because closure is not in progress.
Additionally,  if we followed the WebSocket API spec, we should do nothing and
return no error against redundant Close() calls.

This change based on a user report.

[dmichael (off chromium), 2012-04-24 20:10:13]

lgtm

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/api/ppb_websocke...
File ppapi/api/ppb_websocket.idl (right):

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/api/ppb_websocke...
ppapi/api/ppb_websocket.idl:272: * Returns <code>PP_ERROR_INPROGRESS</code> if
previous call to Close() is
if "a" previous ?

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/cpp/websocket.h
File ppapi/cpp/websocket.h (right):

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/cpp/websocket.h#...
ppapi/cpp/websocket.h:97: /// if previous call to Close() is not finished.
"a" previous ?

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/tests/test_webso...
File ppapi/tests/test_websocket.cc (right):

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/tests/test_webso...
ppapi/tests/test_websocket.cc:527: ASSERT_EQ(PP_OK, result);
optional: I have made a change to TestCompletionCallback and haven't spread it
through all the tests yet. The idea is to call a new WaitForResult that takes an
integer parameter of the result, so that it will automatically do the right
thing (wait for PP_OK_COMPLETIONPENDING, just return otherwise). You can also
use it to make sure the callback is being executed in the right way with the
"CHECK_CALLBACK_BEHAVIOR" macro. You can update this file if you like (here or
in a separate CL). If you don't, I'll do it when I update all the tests. Here's
the CL that had the update:
http://codereview.chromium.org/9937001
Here's an example of how the test code changes:
http://codereview.chromium.org/9937001/diff/12002/ppapi/tests/test_url_loader.cc

[Takashi Toyoshima, 2012-04-25 02:53:29]

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/api/ppb_websocke...
File ppapi/api/ppb_websocket.idl (right):

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/api/ppb_websocke...
ppapi/api/ppb_websocket.idl:272: * Returns <code>PP_ERROR_INPROGRESS</code> if
previous call to Close() is
On 2012/04/24 20:10:14, dmichael wrote:
> if "a" previous ?

Done.

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/cpp/websocket.h
File ppapi/cpp/websocket.h (right):

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/cpp/websocket.h#...
ppapi/cpp/websocket.h:97: /// if previous call to Close() is not finished.
On 2012/04/24 20:10:14, dmichael wrote:
> "a" previous ?

Done.

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/tests/test_webso...
File ppapi/tests/test_websocket.cc (right):

https://chromiumcodereview.appspot.com/10169036/diff/1/ppapi/tests/test_webso...
ppapi/tests/test_websocket.cc:527: ASSERT_EQ(PP_OK, result);
OK, I'll make a separate CL for this file.

[commit-bot: I haz the power, 2012-04-25 02:53:46]

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

[commit-bot: I haz the power, 2012-04-25 11:39:23]

Change committed as 133894