Bug 61277

Summary: WebSocket: Add fail() to WebSocketChannel and its family
Product: WebKit Reporter: Yuta Kitamura <yutak>
Component: WebCore Misc.Assignee: Yuta Kitamura <yutak>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, tkent, ukai
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 35721, 61115    
Attachments:
Description Flags
Patch
none
Ready to commit none

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.