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

chrome/browser/net/websocket_browsertest.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 "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #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"
 
 namespace {
 
 class WebSocketBrowserTest : public InProcessBrowserTest {
  public:
   WebSocketBrowserTest()
     : ws_server_(net::SpawnedTestServer::TYPE_WS,

[mmenke, 2014/07/24 14:34:32]

This should use 4-space indent.

[yhirano, 2014/07/25 01:48:59]

Done.

                  net::SpawnedTestServer::kLocalhost,
                  net::GetWebSocketTestDataDirectory()),
       wss_server_(net::SpawnedTestServer::TYPE_WSS,
                   SSLOptions(SSLOptions::CERT_OK),
                   net::GetWebSocketTestDataDirectory()) {
   }
 
  protected:
   net::SpawnedTestServer ws_server_;
   net::SpawnedTestServer wss_server_;

[mmenke, 2014/07/24 14:34:32]

data members should go at the end of a section.

[yhirano, 2014/07/25 01:48:59]

Done.

 
+  void NavigateToHTTP(const std::string& path) {

[mmenke, 2014/07/24 14:34:32]

Should probably include string header.

[yhirano, 2014/07/25 01:48:59]

Done.

+      // Visit a HTTP page for testing.

[mmenke, 2014/07/24 14:34:32]

This section should use 2-space indent.

[yhirano, 2014/07/25 01:48:59]

Done.

+      std::string scheme("http");
+      GURL::Replacements replacements;
+      replacements.SetSchemeStr(scheme);

[mmenke, 2014/07/24 14:34:32]

And the gurl header as well.

[yhirano, 2014/07/25 01:48:59]

Done.

+      ui_test_utils::NavigateToURL(
+          browser(),
+          ws_server_.GetURL(path).ReplaceComponents(replacements));
+  }
+
+  void NavigateToHTTPS(const std::string& path) {
+      // Visit a HTTPS page for testing.

[mmenke, 2014/07/24 14:34:31]

This section should use 2-space indent.

[yhirano, 2014/07/25 01:48:59]

Done.

+      std::string scheme("https");
+      GURL::Replacements replacements;
+      replacements.SetSchemeStr(scheme);
+      ui_test_utils::NavigateToURL(
+          browser(),
+          wss_server_.GetURL(path).ReplaceComponents(replacements));
+  }
+
  private:
   typedef net::SpawnedTestServer::SSLOptions SSLOptions;
 
   DISALLOW_COPY_AND_ASSIGN(WebSocketBrowserTest);
 };
 
 // Test that the browser can handle a WebSocket frame split into multiple TCP
 // segments.
 IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, WebSocketSplitSegments) {
   // Launch a WebSocket server.
   ASSERT_TRUE(ws_server_.Start());
 
   // Setup page title observer.
   content::WebContents* tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   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"));

[mmenke, 2014/07/24 14:34:32]

optional:  Suggest using:

EXPECT_EQ(base::ASCIIToUTF16("PASS"), result);

On all of these.  Believe that will print out both strings on a mismatch, or at
least attempt to.

[yhirano, 2014/07/25 01:48:59]

Done.

 }
 
 IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, SecureWebSocketSplitRecords) {
   // Launch a secure WebSocket server.
   ASSERT_TRUE(wss_server_.Start());
 
   // Setup page title observer.
   content::WebContents* tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   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"));
 }
 
+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* newTab =

[mmenke, 2014/07/24 14:34:32]

new_tab.

[yhirano, 2014/07/25 01:48:59]

Done.

+        content::WebContents::Create(
+            content::WebContents::CreateParams(tab->GetBrowserContext()));
+    browser()->tab_strip_model()->AppendWebContents(newTab, true);
+    ASSERT_EQ(newTab, browser()->tab_strip_model()->GetWebContentsAt(1));
+    content::TitleWatcher title_watcher(newTab,

[mmenke, 2014/07/24 14:34:32]

Suggest using another name for this watcher, to make it clearer it's watching
the new tab, and to more clearly distinguish it from the title_watcher below

[yhirano, 2014/07/25 01:48:59]

Done.

+                                        base::ASCIIToUTF16("CONNECTED"));
+    title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("CLOSED"));
+    content::WebContentsDestroyedWatcher destroyed_watcher(newTab);

[mmenke, 2014/07/24 14:34:32]

Any reason to declare this here, instead of just before it's actually used (Per
google style guidelines)?  I don't think there's any reason why it should be
destroyed before your call to CloseWebContentsAt, but could be missing
something.

[yhirano, 2014/07/25 01:48:59]

Done.

+
+    NavigateToHTTP("counted_connection.html");
+    const base::string16 result = title_watcher.WaitAndGetTitle();
+    EXPECT_TRUE(EqualsASCII(result, "CONNECTED"));
+    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_TRUE(EqualsASCII(result, "PASS"));
+}
+
 }  // namespace