[WebSocket] Send a close frame when the renderer process is gone.

Renderer processes may die without clearing its |WebSocketBridge|s.
It is helpful to send a close frame with code = 1001 in such a case.

BUG=392534
R=ricea@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285557

Review URL: https://codereview.chromium.org/390773002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285884 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/net/websocket_browsertest.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
+
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/browser.h"
@@ -11,21 +13,39 @@
 #include "content/public/test/browser_test_utils.h"
 #include "net/base/test_data_directory.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
+#include "url/gurl.h"
 
 namespace {
 
 class WebSocketBrowserTest : public InProcessBrowserTest {
  public:
   WebSocketBrowserTest()
-    : ws_server_(net::SpawnedTestServer::TYPE_WS,
-                 net::SpawnedTestServer::kLocalhost,
-                 net::GetWebSocketTestDataDirectory()),
-      wss_server_(net::SpawnedTestServer::TYPE_WSS,
-                  SSLOptions(SSLOptions::CERT_OK),
-                  net::GetWebSocketTestDataDirectory()) {
-  }
+      : ws_server_(net::SpawnedTestServer::TYPE_WS,
+                   net::SpawnedTestServer::kLocalhost,
+                   net::GetWebSocketTestDataDirectory()),
+        wss_server_(net::SpawnedTestServer::TYPE_WSS,
+                    SSLOptions(SSLOptions::CERT_OK),
+                    net::GetWebSocketTestDataDirectory()) {}
 
  protected:
+  void NavigateToHTTP(const std::string& path) {
+    // Visit a HTTP page for testing.
+    std::string scheme("http");
+    GURL::Replacements replacements;
+    replacements.SetSchemeStr(scheme);
+    ui_test_utils::NavigateToURL(
+        browser(), ws_server_.GetURL(path).ReplaceComponents(replacements));
+  }
+
+  void NavigateToHTTPS(const std::string& path) {
+    // Visit a HTTPS page for testing.
+    std::string scheme("https");
+    GURL::Replacements replacements;
+    replacements.SetSchemeStr(scheme);
+    ui_test_utils::NavigateToURL(
+        browser(), wss_server_.GetURL(path).ReplaceComponents(replacements));
+  }
+
   net::SpawnedTestServer ws_server_;
   net::SpawnedTestServer wss_server_;
 
@@ -47,17 +67,10 @@ IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, WebSocketSplitSegments) {
   content::TitleWatcher watcher(tab, base::ASCIIToUTF16("PASS"));
   watcher.AlsoWaitForTitle(base::ASCIIToUTF16("FAIL"));
 
-  // Visit a HTTP page for testing.
-  std::string scheme("http");
-  GURL::Replacements replacements;
-  replacements.SetSchemeStr(scheme);
-  ui_test_utils::NavigateToURL(
-      browser(),
-      ws_server_.GetURL(
-          "split_packet_check.html").ReplaceComponents(replacements));
+  NavigateToHTTP("split_packet_check.html");
 
   const base::string16 result = watcher.WaitAndGetTitle();
-  EXPECT_TRUE(EqualsASCII(result, "PASS"));
+  EXPECT_EQ(base::ASCIIToUTF16("PASS"), result);
 }
 
 IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, SecureWebSocketSplitRecords) {
@@ -70,17 +83,43 @@ IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, SecureWebSocketSplitRecords) {
   content::TitleWatcher watcher(tab, base::ASCIIToUTF16("PASS"));
   watcher.AlsoWaitForTitle(base::ASCIIToUTF16("FAIL"));
 
-  // Visit a HTTPS page for testing.
-  std::string scheme("https");
-  GURL::Replacements replacements;
-  replacements.SetSchemeStr(scheme);
-  ui_test_utils::NavigateToURL(
-      browser(),
-      wss_server_.GetURL(
-          "split_packet_check.html").ReplaceComponents(replacements));
+  NavigateToHTTPS("split_packet_check.html");
 
   const base::string16 result = watcher.WaitAndGetTitle();
-  EXPECT_TRUE(EqualsASCII(result, "PASS"));
+  EXPECT_EQ(base::ASCIIToUTF16("PASS"), result);
+}
+
+IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, SendCloseFrameWhenTabIsClosed) {
+  // Launch a WebSocket server.
+  ASSERT_TRUE(ws_server_.Start());
+
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  {
+    // Create a new tab, establish a WebSocket connection and close the tab.
+    content::WebContents* new_tab = content::WebContents::Create(
+        content::WebContents::CreateParams(tab->GetBrowserContext()));
+    browser()->tab_strip_model()->AppendWebContents(new_tab, true);
+    ASSERT_EQ(new_tab, browser()->tab_strip_model()->GetWebContentsAt(1));
+
+    content::TitleWatcher connected_title_watcher(
+        new_tab, base::ASCIIToUTF16("CONNECTED"));
+    connected_title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("CLOSED"));
+    NavigateToHTTP("counted_connection.html");
+    const base::string16 result = connected_title_watcher.WaitAndGetTitle();
+    EXPECT_TRUE(EqualsASCII(result, "CONNECTED"));
+
+    content::WebContentsDestroyedWatcher destroyed_watcher(new_tab);
+    browser()->tab_strip_model()->CloseWebContentsAt(1, 0);
+    destroyed_watcher.Wait();
+  }
+
+  content::TitleWatcher title_watcher(tab, base::ASCIIToUTF16("PASS"));
+  title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("FAIL"));
+
+  NavigateToHTTP("count_connection.html");
+  const base::string16 result = title_watcher.WaitAndGetTitle();
+  EXPECT_EQ(base::ASCIIToUTF16("PASS"), result);
 }
 
 }  // namespace

content/browser/renderer_host/websocket_dispatcher_host.cc

@@ -5,6 +5,7 @@
 #include "content/browser/renderer_host/websocket_dispatcher_host.h"
 
 #include <string>
+#include <vector>
 
 #include "base/callback.h"
 #include "base/logging.h"
@@ -178,6 +179,21 @@ WebSocketHostState WebSocketDispatcherHost::DoDropChannel(
 }
 
 WebSocketDispatcherHost::~WebSocketDispatcherHost() {
+  std::vector<WebSocketHost*> hosts;
+  for (base::hash_map<int, WebSocketHost*>::const_iterator i = hosts_.begin();
+       i != hosts_.end(); ++i) {
+    // In order to avoid changing the container while iterating, we copy
+    // the hosts.
+    hosts.push_back(i->second);
+  }
+
+  for (size_t i = 0; i < hosts.size(); ++i) {
+    // Note that some calls to GoAway could fail. In that case hosts[i] will be
+    // deleted and removed from |hosts_| in |DoDropChannel|.
+    hosts[i]->GoAway();
+    hosts[i] = NULL;
+  }
+
   STLDeleteContainerPairSecondPointers(hosts_.begin(), hosts_.end());
 }
 

content/browser/renderer_host/websocket_dispatcher_host_unittest.cc

@@ -4,11 +4,13 @@
 
 #include "content/browser/renderer_host/websocket_dispatcher_host.h"
 
+#include <algorithm>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "content/browser/renderer_host/websocket_host.h"
 #include "content/common/websocket.h"
 #include "content/common/websocket_messages.h"
@@ -23,28 +25,33 @@ namespace {
 // This number is unlikely to occur by chance.
 static const int kMagicRenderProcessId = 506116062;
 
+class WebSocketDispatcherHostTest;
+
 // A mock of WebsocketHost which records received messages.
 class MockWebSocketHost : public WebSocketHost {
  public:
   MockWebSocketHost(int routing_id,
                     WebSocketDispatcherHost* dispatcher,
-                    net::URLRequestContext* url_request_context)
-      : WebSocketHost(routing_id, dispatcher, url_request_context) {
-  }
+                    net::URLRequestContext* url_request_context,
+                    WebSocketDispatcherHostTest* owner);
 
   virtual ~MockWebSocketHost() {}
 
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE{
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
     received_messages_.push_back(message);
     return true;
   }
 
+  virtual void GoAway() OVERRIDE;
+
   std::vector<IPC::Message> received_messages_;
+  base::WeakPtr<WebSocketDispatcherHostTest> owner_;
 };
 
 class WebSocketDispatcherHostTest : public ::testing::Test {
  public:
-  WebSocketDispatcherHostTest() {
+  WebSocketDispatcherHostTest()
+      : weak_ptr_factory_(this) {
     dispatcher_host_ = new WebSocketDispatcherHost(
         kMagicRenderProcessId,
         base::Bind(&WebSocketDispatcherHostTest::OnGetRequestContext,
@@ -53,7 +60,19 @@ class WebSocketDispatcherHostTest : public ::testing::Test {
                    base::Unretained(this)));
   }
 
-  virtual ~WebSocketDispatcherHostTest() {}
+  virtual ~WebSocketDispatcherHostTest() {
+    // We need to invalidate the issued WeakPtrs at the beginning of the
+    // destructor in order not to access destructed member variables.
+    weak_ptr_factory_.InvalidateWeakPtrs();
+  }
+
+  void GoAway(int routing_id) {
+    gone_hosts_.push_back(routing_id);
+  }
+
+  base::WeakPtr<WebSocketDispatcherHostTest> GetWeakPtr() {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
 
  protected:
   scoped_refptr<WebSocketDispatcherHost> dispatcher_host_;
@@ -61,6 +80,9 @@ class WebSocketDispatcherHostTest : public ::testing::Test {
   // Stores allocated MockWebSocketHost instances. Doesn't take ownership of
   // them.
   std::vector<MockWebSocketHost*> mock_hosts_;
+  std::vector<int> gone_hosts_;
+
+  base::WeakPtrFactory<WebSocketDispatcherHostTest> weak_ptr_factory_;
 
  private:
   net::URLRequestContext* OnGetRequestContext() {
@@ -69,12 +91,25 @@ class WebSocketDispatcherHostTest : public ::testing::Test {
 
   WebSocketHost* CreateWebSocketHost(int routing_id) {
     MockWebSocketHost* host =
-        new MockWebSocketHost(routing_id, dispatcher_host_.get(), NULL);
+        new MockWebSocketHost(routing_id, dispatcher_host_.get(), NULL, this);
     mock_hosts_.push_back(host);
     return host;
   }
 };
 
+MockWebSocketHost::MockWebSocketHost(
+    int routing_id,
+    WebSocketDispatcherHost* dispatcher,
+    net::URLRequestContext* url_request_context,
+    WebSocketDispatcherHostTest* owner)
+    : WebSocketHost(routing_id, dispatcher, url_request_context),
+      owner_(owner->GetWeakPtr()) {}
+
+void MockWebSocketHost::GoAway() {
+  if (owner_)
+    owner_->GoAway(routing_id());
+}
+
 TEST_F(WebSocketDispatcherHostTest, Construct) {
   // Do nothing.
 }
@@ -156,5 +191,29 @@ TEST_F(WebSocketDispatcherHostTest, SendFrame) {
   }
 }
 
+TEST_F(WebSocketDispatcherHostTest, Destruct) {
+  WebSocketHostMsg_AddChannelRequest message1(
+      123, GURL("ws://example.com/test"), std::vector<std::string>(),
+      url::Origin("http://example.com"), -1);
+  WebSocketHostMsg_AddChannelRequest message2(
+      456, GURL("ws://example.com/test2"), std::vector<std::string>(),
+      url::Origin("http://example.com"), -1);
+
+  ASSERT_TRUE(dispatcher_host_->OnMessageReceived(message1));
+  ASSERT_TRUE(dispatcher_host_->OnMessageReceived(message2));
+
+  ASSERT_EQ(2u, mock_hosts_.size());
+
+  mock_hosts_.clear();
+  dispatcher_host_ = NULL;
+
+  ASSERT_EQ(2u, gone_hosts_.size());
+  // The gone_hosts_ ordering is not predictable because it depends on the
+  // hash_map ordering.
+  std::sort(gone_hosts_.begin(), gone_hosts_.end());
+  EXPECT_EQ(123, gone_hosts_[0]);
+  EXPECT_EQ(456, gone_hosts_[1]);
+}
+
 }  // namespace
 }  // namespace content

content/browser/renderer_host/websocket_host.cc

@@ -17,6 +17,7 @@
 #include "net/http/http_util.h"
 #include "net/ssl/ssl_info.h"
 #include "net/websockets/websocket_channel.h"
+#include "net/websockets/websocket_errors.h"
 #include "net/websockets/websocket_event_interface.h"
 #include "net/websockets/websocket_frame.h"  // for WebSocketFrameHeader::OpCode
 #include "net/websockets/websocket_handshake_request_info.h"
@@ -330,6 +331,10 @@ WebSocketHost::WebSocketHost(int routing_id,
 
 WebSocketHost::~WebSocketHost() {}
 
+void WebSocketHost::GoAway() {
+  OnDropChannel(false, static_cast<uint16>(net::kWebSocketErrorGoingAway), "");
+}
+
 bool WebSocketHost::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(WebSocketHost, message)

content/browser/renderer_host/websocket_host.h

@@ -40,10 +40,16 @@ class CONTENT_EXPORT WebSocketHost {
                 net::URLRequestContext* url_request_context);
   virtual ~WebSocketHost();
 
+  // The renderer process is going away.
+  // This function is virtual for testing.
+  virtual void GoAway();
+
   // General message dispatch. WebSocketDispatcherHost::OnMessageReceived
   // delegates to this method after looking up the |routing_id|.
   virtual bool OnMessageReceived(const IPC::Message& message);
 
+  int routing_id() const { return routing_id_; }
+
  private:
   // Handlers for each message type, dispatched by OnMessageReceived(), as
   // defined in content/common/websocket_messages.h

net/data/websocket/README

@@ -19,6 +19,16 @@ Multiple tests may share one resource, or URI handler.
      content::TitleWatcher.
      Used by WorkerTest.WebSocketSharedWorker.
 
+- counted_connection.html : A page that creates a WebSocket connection
+     to count-connection_wsh.
+     This file does NOT close the established connection.
+     Used by SendCloseFrameWhenTabIsClosed.
+
+- count_connection.html : A page that creates a WebSocket connection
+     to count-connection_wsh and checks the number of open and closed
+     websocket connections to the handler.
+     Used by SendCloseFrameWhenTabIsClosed.
+
 - websocket_worker_simple.js : A JavaScript runs on Workers created from the
      websocket_shared_worker.html.
      Used by WorkerTest.WebSocketSharedWorker.
@@ -44,6 +54,11 @@ Multiple tests may share one resource, or URI handler.
      Used by WebSocketBrowserTest.WebSocketSplitSegments and
      WebSocketBrowserTest.SecureWebSocketSplitRecords.
 
+- count-connection_wsh.py : counts the number of open and closed websocket
+     connections to this handler.
+     It sends the numbers after each connection establishment.
+     Used by SendCloseFrameWhenTabIsClosed.
+
 - protocol-test_wsh.py : A WebSocket URL handler for testing outgoing opening
      handshake protocol.
      Used by kinds of PPAPI tests for WebSocket.

net/data/websocket/connect_check.html

@@ -3,17 +3,8 @@
 <head>
 <title>test ws connection</title>
 <script type="text/javascript">
-
-var href = window.location.href;
-var hostBegin = href.indexOf('/') + 2;
-var hostEnd = href.lastIndexOf(':');
-var host = href.slice(hostBegin, hostEnd);
-var portBegin = hostEnd + 1;
-var portEnd = href.lastIndexOf('/');
-var port = href.slice(portBegin, portEnd);
-var scheme = href.indexOf('https') >= 0 ? 'wss' : 'ws';
-var url = scheme + '://' + host + ':' + port + '/echo-with-no-extension';
-
+var protocol = location.protocol.replace('http', 'ws');
+var url = protocol + '//' + location.host + '/echo-with-no-extension';
 // Do connection test.
 var ws = new WebSocket(url);
 

net/data/websocket/count-connection_wsh.py

+# Copyright 2014 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.
+
+numOpenConnections = 0
+numClosedConnections = 0
+
+
+def web_socket_do_extra_handshake(request):
+  global numOpenConnections
+  numOpenConnections += 1
+
+
+def web_socket_transfer_data(request):
+  request.ws_stream.send_message('open: %d, closed: %d' %
+      (numOpenConnections, numClosedConnections), binary=False)
+  # Just waiting...
+  request.ws_stream.receive_message()
+
+
+def web_socket_passive_closing_handshake(request):
+  global numOpenConnections
+  global numClosedConnections
+  numOpenConnections -= 1
+  numClosedConnections += 1
+  return (1000, '')

net/data/websocket/count_connection.html

+<!doctype html>
+<html>
+<head>
+<title>Connect to a WebSocket server</title>
+<script type="text/javascript">
+var protocol = location.protocol.replace('http', 'ws');
+var url = protocol + '//' + location.host + '/count-connection';
+
+// Do connection test.
+var ws = new WebSocket(url);
+
+ws.onmessage = function(e) {
+    document.title = (e.data === 'open: 1, closed: 1' ? 'PASS' : 'FAIL');
+}
+
+ws.onclose = function() {
+    document.title = 'FAIL';
+};
+
+</script>
+</head>
+</html>

net/data/websocket/counted_connection.html

+<!doctype html>
+<html>
+<head>
+<title>Connect to a WebSocket server</title>
+<script type="text/javascript">
+var protocol = location.protocol.replace('http', 'ws');
+var url = protocol + '//' + location.host + '/count-connection';
+var ws = new WebSocket(url);
+
+ws.onopen = function() {
+    document.title = 'CONNECTED';
+};
+
+ws.onclose = function() {
+    document.title = 'CLOSED';
+};
+</script>
+</head>
+</html>

net/data/websocket/split_packet_check.html

@@ -3,16 +3,8 @@
 <head>
 <title>test ws split packet</title>
 <script type="text/javascript">
-
-var href = window.location.href;
-var hostBegin = href.indexOf('/') + 2;
-var hostEnd = href.lastIndexOf(':');
-var host = href.slice(hostBegin, hostEnd);
-var portBegin = hostEnd + 1;
-var portEnd = href.lastIndexOf('/');
-var port = href.slice(portBegin, portEnd);
-var scheme = href.indexOf('https') >= 0 ? 'wss' : 'ws';
-var url = scheme + '://' + host + ':' + port + '/close-with-split-packet';
+var protocol = location.protocol.replace('http', 'ws');
+var url = protocol + '//' + location.host + '/close-with-split-packet';
 
 // Do connection test.
 var ws = new WebSocket(url);

net/data/websocket/websocket_shared_worker.html

@@ -7,14 +7,8 @@ function log(message)
   document.getElementById("result").innerHTML += message + "<br>";
 }
 var worker = new SharedWorker("websocket_worker_simple.js");
-var href = window.location.href;
-var hostBegin = href.indexOf("/") + 2;
-var hostEnd = href.lastIndexOf(":");
-var host = href.slice(hostBegin, hostEnd);
-var portBegin = hostEnd + 1;
-var portEnd = href.lastIndexOf("/");
-var port = href.slice(portBegin, portEnd);
-var url = "ws://" + host + ":" + port + "/echo-with-no-extension";
+var protocol = location.protocol.replace('http', 'ws');
+var url = protocol + '//' + location.host + '/echo-with-no-extension';
 worker.port.onmessage = function (evt) {
   log(evt.data);
   if (evt.data == "DONE") {
@@ -28,4 +22,3 @@ worker.port.postMessage(url);
 </script>
 </body>
 </html>
-

net/websockets/websocket_channel.cc

@@ -465,6 +465,8 @@ void WebSocketChannel::SendFlowControl(int64 quota) {
 void WebSocketChannel::StartClosingHandshake(uint16 code,
                                              const std::string& reason) {
   if (InClosingState()) {
+    // When the associated renderer process is killed while the channel is in
+    // CLOSING state we reach here.
     DVLOG(1) << "StartClosingHandshake called in state " << state_
              << ". This may be a bug, or a harmless race.";
     return;