Fix race condition with WebSocket shutdown

There was a race condition in the network service where if a disconnect
on the writing or reading data pipes was detected before the disconnect
on the mojo interfaces it would fail to send a close frame. To fix, stop
resetting the connection when the data pipes are closed.

Re-enable the SendCloseFrameWhenTabIsClosed browser test. It should no
longer be flaky, and it's the only test we have for this behaviour.

Also modify the SendCloseFrameWhenTabIsClosed test to no longer timeout
on failure.

BUG=1123349

Change-Id: Ib4aaa46cb1f9f97e09509f59ef421228d02c941d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398036
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810463}

chrome/browser/net/websocket_browsertest.cc

@@ -195,8 +195,7 @@ IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, SecureWebSocketSplitRecords) {
   EXPECT_EQ("PASS", WaitAndGetTitle());
 }
 
-IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest,
-                       DISABLED_SendCloseFrameWhenTabIsClosed) {
+IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, SendCloseFrameWhenTabIsClosed) {
   // Launch a WebSocket server.
   ASSERT_TRUE(ws_server_.Start());
 

net/data/websocket/close-observer_wsh.py

@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 from six.moves.urllib import parse
+from mod_pywebsocket import stream
 import threading
 
 
@@ -27,7 +28,10 @@ def be_observed(request):
   with cv:
     connected = True
   # Wait for a Close frame
-  request.ws_stream.receive_message()
+  try:
+    request.ws_stream.receive_message()
+  except stream.ConnectionTerminatedException:
+    observe_close(1006)  # "Abnormal Closure"
 
 
 def be_observer(request):
@@ -60,9 +64,13 @@ def web_socket_transfer_data(request):
 
 
 def web_socket_passive_closing_handshake(request):
-  global close_code
   if get_role(request) == 'observed':
-    with cv:
-      close_code = request.ws_close_code
-      cv.notify()
+    observe_close(request.ws_close_code)
   return request.ws_close_code, request.ws_close_reason
+
+
+def observe_close(code):
+  global close_code
+  with cv:
+    close_code = code
+    cv.notify()

services/network/websocket.cc

@@ -612,8 +612,11 @@ void WebSocket::AddChannel(
 void WebSocket::OnWritable(MojoResult result,
                            const mojo::HandleSignalsState& state) {
   if (result != MOJO_RESULT_OK) {
+    // MOJO_RESULT_FAILED_PRECONDITION (=9) is common when the other end of the
+    // pipe is closed.
     DVLOG(1) << "WebSocket::OnWritable mojo error=" << result;
-    Reset();
+
+    OnConnectionError(FROM_HERE);
     return;
   }
   wait_for_writable_ = false;
@@ -678,8 +681,11 @@ void WebSocket::SendDataFrame(base::span<const char>* payload) {
 void WebSocket::OnReadable(MojoResult result,
                            const mojo::HandleSignalsState& state) {
   if (result != MOJO_RESULT_OK) {
-    DVLOG(1) << "WebSocket::OnWritable mojo error=" << result;
-    Reset();
+    // MOJO_RESULT_FAILED_PRECONDITION (=9) is common when the other end of the
+    // pipe is closed.
+    DVLOG(1) << "WebSocket::OnReadable mojo error=" << result;
+
+    OnConnectionError(FROM_HERE);
     return;
   }
   wait_for_readable_ = false;