Fix use-after-free in net::WebSocketChannel::ReadFrames

Recently network::WebSocket started using mojo datapipe rather than
ReadOnlyBuffer. Unlike usual mojo message, an error to write bytes to
data pipe is detected synchronously. We called WebSocket::Reset
which deletes the associated net::WebSocketChannel synchronously, but
the net::WebSocketChannel didn't know it and keeped running after
destruction.

This CL makes the Reset call asynchronous. This doesn't lead to any races
because we actually don't write bytes in such a case - it
looks like the data pipe buffer becomes full, and Reset is called afterwards.

Bug: 994000
Change-Id: I8b9213720b60314f1e638af9cd492e807cbab56d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1767478
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690275}

services/network/websocket.cc

@@ -581,10 +581,7 @@ void WebSocket::SendPendingDataFrames() {
            << ", pending_data_frames_.size=" << pending_data_frames_.size();
   while (!pending_data_frames_.empty()) {
     WebSocket::DataFrame& data_frame = pending_data_frames_.front();
-    if (!SendDataFrame(&data_frame)) {
-      Reset();
-      return;
-    }
+    SendDataFrame(&data_frame);
     if (data_frame.size > 0) {
       // Mojo doesn't have any write buffer so far.
       writable_watcher_.ArmOrNotify();
@@ -594,7 +591,7 @@ void WebSocket::SendPendingDataFrames() {
   }
 }
 
-bool WebSocket::SendDataFrame(DataFrame* data_frame) {
+void WebSocket::SendDataFrame(DataFrame* data_frame) {
   DCHECK_GT(data_frame->size, 0u);
   MojoResult begin_result;
   void* buffer;
@@ -618,9 +615,12 @@ bool WebSocket::SendDataFrame(DataFrame* data_frame) {
   if (begin_result != MOJO_RESULT_OK &&
       begin_result != MOJO_RESULT_SHOULD_WAIT) {
     DVLOG(1) << "WebSocket::OnWritable mojo error=" << begin_result;
-    return false;
+    DCHECK_EQ(begin_result, MOJO_RESULT_FAILED_PRECONDITION);
+    base::SequencedTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(&WebSocket::OnConnectionError,
+                                  weak_ptr_factory_.GetWeakPtr()));
   }
-  return true;
+  return;
 }
 
 void WebSocket::OnSSLCertificateErrorResponse(

services/network/websocket.h

@@ -140,8 +140,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) WebSocket : public mojom::WebSocket {
   // Datapipe functions to receive.
   void OnWritable(MojoResult result, const mojo::HandleSignalsState& state);
   void SendPendingDataFrames();
-  // Returns false if mojo error occurs.
-  bool SendDataFrame(DataFrame*);
+  void SendDataFrame(DataFrame*);
 
   // |factory_| owns |this|.
   WebSocketFactory* const factory_;