Make net::WebSocketChannel deletion safe and enable new IPCs

content/browser/renderer_host/websocket_dispatcher_host.cc

Index: content/browser/renderer_host/websocket_dispatcher_host.cc
diff --git a/content/browser/renderer_host/websocket_dispatcher_host.cc b/content/browser/renderer_host/websocket_dispatcher_host.cc
index 81afd61313a88c53433d67633c536caab6434848..7164f207a0f11e5dfeb65e06e82c0e3e86c252e2 100644
--- a/content/browser/renderer_host/websocket_dispatcher_host.cc
+++ b/content/browser/renderer_host/websocket_dispatcher_host.cc
@@ -14,6 +14,12 @@
 
 namespace content {
 
+namespace {
+
+typedef WebSocketDispatcherHost::WebSocketHostState WebSocketHostState;
+
+}  // namespace
+
 WebSocketDispatcherHost::WebSocketDispatcherHost(
     const GetRequestContextCallback& get_context_callback)
     : get_context_callback_(get_context_callback) {}
@@ -60,45 +66,59 @@ WebSocketHost* WebSocketDispatcherHost::GetHost(int routing_id) const {
   return it == hosts_.end() ? NULL : it->second;
 }
 
-void WebSocketDispatcherHost::SendOrDrop(IPC::Message* message) {
+WebSocketHostState WebSocketDispatcherHost::SendOrDrop(IPC::Message* message) {
   if (!Send(message)) {
     DVLOG(1) << "Sending of message type " << message->type()
              << " failed. Dropping channel.";
     DeleteWebSocketHost(message->routing_id());
+    return WEBSOCKET_HOST_DELETED;
   }
+  return WEBSOCKET_HOST_ALIVE;
 }
 
-void WebSocketDispatcherHost::SendAddChannelResponse(
+WebSocketHostState WebSocketDispatcherHost::SendAddChannelResponse(
     int routing_id,
     bool fail,
     const std::string& selected_protocol,
     const std::string& extensions) {
-  SendOrDrop(new WebSocketMsg_AddChannelResponse(
-      routing_id, fail, selected_protocol, extensions));
-  if (fail)
+  if (SendOrDrop(new WebSocketMsg_AddChannelResponse(
+          routing_id, fail, selected_protocol, extensions)) ==
+      WEBSOCKET_HOST_DELETED)
+    return WEBSOCKET_HOST_DELETED;
+  if (fail) {
     DeleteWebSocketHost(routing_id);
+    return WEBSOCKET_HOST_DELETED;
+  }
+  return WEBSOCKET_HOST_ALIVE;
 }
 
-void WebSocketDispatcherHost::SendFrame(int routing_id,
-                                        bool fin,
-                                        WebSocketMessageType type,
-                                        const std::vector<char>& data) {
-  SendOrDrop(new WebSocketMsg_SendFrame(routing_id, fin, type, data));
+WebSocketHostState WebSocketDispatcherHost::SendFrame(
+    int routing_id,
+    bool fin,
+    WebSocketMessageType type,
+    const std::vector<char>& data) {
+  return SendOrDrop(new WebSocketMsg_SendFrame(routing_id, fin, type, data));
 }
 
-void WebSocketDispatcherHost::SendFlowControl(int routing_id, int64 quota) {
-  SendOrDrop(new WebSocketMsg_FlowControl(routing_id, quota));
+WebSocketHostState WebSocketDispatcherHost::SendFlowControl(int routing_id,
+                                                            int64 quota) {
+  return SendOrDrop(new WebSocketMsg_FlowControl(routing_id, quota));
 }
 
-void WebSocketDispatcherHost::SendClosing(int routing_id) {
+WebSocketHostState WebSocketDispatcherHost::SendClosing(int routing_id) {
   // TODO(ricea): Implement the SendClosing IPC.
+  return WEBSOCKET_HOST_ALIVE;
 }
 
-void WebSocketDispatcherHost::DoDropChannel(int routing_id,
-                                            uint16 code,
-                                            const std::string& reason) {
-  SendOrDrop(new WebSocketMsg_DropChannel(routing_id, code, reason));
+WebSocketHostState WebSocketDispatcherHost::DoDropChannel(

[tyoshino (SeeGerritForStatus), 2013/10/15 07:22:15]

this method always deletes the corresponding channel. it is okay to make this
void?

[Adam Rice, 2013/10/15 08:31:42]

It would work as void. I think it is nicer to use the WebSocketHostState return
value because it makes the implementation of
WebSocketEventHandler::OnDropChannel() consistent with the other methods.

Also, as yhirano pointed out, there are three methods that always return
DELETED: WebSocketDispatcherHost::DoDropChannel(),
WebSocketEventHandler::OnDropChannel() and WebSocketChannel::FailChannel().
Whatever we choose, we should do the same thing for all of them.

+    int routing_id,
+    uint16 code,
+    const std::string& reason) {
+  if (SendOrDrop(new WebSocketMsg_DropChannel(routing_id, code, reason)) ==
+      WEBSOCKET_HOST_DELETED)
+    return WEBSOCKET_HOST_DELETED;
   DeleteWebSocketHost(routing_id);
+  return WEBSOCKET_HOST_DELETED;
 }
 
 WebSocketDispatcherHost::~WebSocketDispatcherHost() {
@@ -107,10 +127,9 @@ WebSocketDispatcherHost::~WebSocketDispatcherHost() {
 
 void WebSocketDispatcherHost::DeleteWebSocketHost(int routing_id) {
   WebSocketHostTable::iterator it = hosts_.find(routing_id);
-  if (it != hosts_.end()) {
-    delete it->second;
-    hosts_.erase(it);
-  }
+  DCHECK(it != hosts_.end());
+  delete it->second;
+  hosts_.erase(it);
 }
 
 }  // namespace content