Bug 61277 - WebSocket: Add fail() to WebSocketChannel and its family
Summary: WebSocket: Add fail() to WebSocketChannel and its family
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yuta Kitamura
URL:
Keywords:
Depends on:
Blocks: 35721 61115
  Show dependency treegraph
 
Reported: 2011-05-23 05:53 PDT by Yuta Kitamura
Modified: 2011-05-24 02:16 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.18 KB, patch)
2011-05-23 06:06 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff
Ready to commit (10.74 KB, patch)
2011-05-23 23:32 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta Kitamura 2011-05-23 05:53:55 PDT
This is first part of bug 61115.

Add WebSocketChannel::fail() (and ThreadableWebSocketChannel and WorkerThreadableWebSocketChannel as well), which logs an error message to JavaScript console and close the underlying channel.

WebSocketChannel::fail() corresponds to "fail the WebSocket connection" mentioned in the WebSocket protocol specification:
http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-00#section-6.1
   Certain algorithms require the user agent to *fail the WebSocket
   connection*.  To do so, the user agent must close the WebSocket
   connection, and may report the problem to the user (which would be
   especially useful for developers).

This is same as WebSocketChannel::close() for now (except for logging). However, this will be necessary in the near future because closing handshake will change the semantics of close().
Comment 1 Yuta Kitamura 2011-05-23 06:06:35 PDT
Created attachment 94411 [details]
Patch
Comment 2 Kent Tamura 2011-05-23 21:24:25 PDT
Comment on attachment 94411 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=94411&action=review

> Source/WebCore/websockets/ThreadableWebSocketChannel.h:56
> +    virtual void fail(const String& reason) = 0; // Log the reason text and close the connection. Will call didClose().

nit: I feel such long comment should be moved to its own line.

> Source/WebCore/websockets/WebSocketChannel.h:62
> +        virtual void fail(const String& reason); // Log the reason text and close the connection. Will call didClose().
>          virtual void disconnect(); // Will suppress didClose().

ThreadableWebSocketChannel has these comments.  So we may remove them here.

> Source/WebCore/websockets/WorkerThreadableWebSocketChannel.cpp:385
> +    m_loaderProxy.postTaskToLoader(
> +        createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadFail, AllowCrossThreadAccess(m_peer), reason));

No need to break the line.
Comment 3 Yuta Kitamura 2011-05-23 22:59:07 PDT
(In reply to comment #2)
> > Source/WebCore/websockets/WorkerThreadableWebSocketChannel.cpp:385
> > +    m_loaderProxy.postTaskToLoader(
> > +        createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadFail, AllowCrossThreadAccess(m_peer), reason));
> 
> No need to break the line.

Sure. I will update the same issues occurring in the same file.
Comment 4 Yuta Kitamura 2011-05-23 23:32:52 PDT
Created attachment 94573 [details]
Ready to commit
Comment 5 WebKit Commit Bot 2011-05-24 02:16:02 PDT
Comment on attachment 94573 [details]
Ready to commit

Clearing flags on attachment: 94573

Committed r87139: <http://trac.webkit.org/changeset/87139>
Comment 6 WebKit Commit Bot 2011-05-24 02:16:07 PDT
All reviewed patches have been landed.  Closing bug.