Websocket should fire 'error' event if no server available

content/browser/renderer_host/socket_stream_dispatcher_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/socket_stream_dispatcher_host.h"
 
 #include "base/logging.h"
 #include "content/browser/renderer_host/socket_stream_host.h"
 #include "content/browser/ssl/ssl_manager.h"
 #include "content/common/resource_messages.h"
@@ skipping 81 matching lines @@
 void SocketStreamDispatcherHost::OnClose(net::SocketStream* socket) {
   int socket_id = SocketStreamHost::SocketIdFromSocketStream(socket);
   DVLOG(1) << "SocketStreamDispatcherHost::OnClosed socket_id=" << socket_id;
   if (socket_id == content::kNoSocketId) {
     LOG(ERROR) << "NoSocketId in OnClose";
     return;
   }
   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);
   DVLOG(1) << "SocketStreamDispatcherHost::OnSSLCertificateError socket_id="
            << socket_id;
   if (socket_id == content::kNoSocketId) {
     LOG(ERROR) << "NoSocketId in OnSSLCertificateError";
     return;
   }
   SocketStreamHost* socket_stream_host = hosts_.Lookup(socket_id);
@@ skipping 107 matching lines @@
   hosts_.Remove(socket_id);
   if (!Send(new SocketStreamMsg_Closed(socket_id))) {
     LOG(ERROR) << "SocketStreamMsg_Closed failed.";
   }
 }
 
 net::URLRequestContext* SocketStreamDispatcherHost::GetURLRequestContext() {
   return url_request_context_selector_->GetRequestContext(
       ResourceType::SUB_RESOURCE);
 }