Websocket should fire 'error' event if no server available

content/browser/renderer_host/socket_stream_dispatcher_host.cc

Index: content/browser/renderer_host/socket_stream_dispatcher_host.cc
diff --git a/content/browser/renderer_host/socket_stream_dispatcher_host.cc b/content/browser/renderer_host/socket_stream_dispatcher_host.cc
index 84e5c8e8a98abd7bf575dcfcc1e7c693eb1f1f8b..965cda534b7231c8755db47d64621bd3dc47ae69 100644
--- a/content/browser/renderer_host/socket_stream_dispatcher_host.cc
+++ b/content/browser/renderer_host/socket_stream_dispatcher_host.cc
@@ -99,6 +99,19 @@ void SocketStreamDispatcherHost::OnClose(net::SocketStream* socket) {
   DeleteSocketStreamHost(socket_id);
 }
 
+void SocketStreamDispatcherHost::OnError(
+    const net::SocketStream* socket, int error) {

[Yuta Kitamura, 2012/06/11 08:24:16]

Function arguments should usually be placed in different lines.

Search for "BadWrappingStyle" in
http://dev.chromium.org/developers/coding-style#TOC-Code-formatting

+  int socket_id = SocketStreamHost::SocketIdFromSocketStream(socket);
+  DVLOG(1) << "SocketStreamDispatcherHost::OnError socket_id=" << socket_id;
+  if (socket_id == content::kNoSocketId) {
+    LOG(ERROR) << "NoSocketId in OnError";
+    return;
+  }
+  if (!Send(new SocketStreamMsg_Failed(socket_id, error))) {

[Yuta Kitamura, 2012/06/11 08:24:16]

I suggest getting a reason message for |error| here and sending it via IPC. IIRC
WebKit has the interface to receive an error message in addition to error code.

|error| is a "net error" (error code used in net/ module), so you can convert an
error code to a text form by using ErrorToString() function located in
net_errors.h.

[Li Yin, 2012/06/12 08:54:20]

Sending the error message sounds a good idea.
Maybe it will be more better that I keep the IPC unchanged,
and get the error message from error code in
WebSocketStreamHandleImpl::Context::DidFail function.

And we need design WebSocketStreamHandle class.

[Yuta Kitamura, 2012/06/12 09:49:54]

Yeah, it's perhaps better to do conversion after IPC to reduce
IPC load, but WebSocketStreamHandleImpl isn't the right place
because webkit/ module should not touch anything in net/
(as far as I understand).

So I suggest doing the conversion within
IPCWebSocketStreamHandleBridge::OnFailed(), which receives
the IPC message from the browser process, and pass the error
text (along with the error code) to the webkit/ module.

I think you are new to the project so let me explain about
modules further: the root directories like net/, content/ and
webkit/ are called "modules", and each module has a set of
"allowed" modules it can directly talk to. See the drawing in
<http://dev.chromium.org/developers/content-module> for
relationships between core modules, and look at "include_rules"
section of DEPS files in each module if you are interested in
more details.

In this case, error codes are generated in net/ module, and
handed off to content/ module. content/ module is responsible
for interpreting the value as what webkit/ module understands,
because webkit/ doesn't know anything about net/. So the
conversion to the text form should be done within content/.

I think modules are hard part for new folks but I hope you
get some ideas about these.
Yep, but that should be mostly straightforward.

+    LOG(ERROR) << "SocketStreamMsg_Failed failed.";
+  }
+}
+
 void SocketStreamDispatcherHost::OnSSLCertificateError(
     net::SocketStream* socket, const net::SSLInfo& ssl_info, bool fatal) {
   int socket_id = SocketStreamHost::SocketIdFromSocketStream(socket);