Cancel loading when body stream reader is detached.

content/child/web_url_loader_impl.cc

Index: content/child/web_url_loader_impl.cc
diff --git a/content/child/web_url_loader_impl.cc b/content/child/web_url_loader_impl.cc
index 65086cf956bda35d3334a79953108ac9f87314f4..e61dc10dfca84250244c55a377439e249115f167 100644
--- a/content/child/web_url_loader_impl.cc
+++ b/content/child/web_url_loader_impl.cc
@@ -323,6 +323,8 @@ class WebURLLoaderImpl::Context : public base::RefCounted<Context>,
   friend class base::RefCounted<Context>;
   ~Context() override;
 
+  // Called when the body data stream is detached from the reader side.
+  void CancelBodyStreaming();
   // We can optimize the handling of data URLs in most cases.
   bool CanHandleDataURLRequestLocally() const;
   void HandleDataURL();
@@ -618,11 +620,18 @@ void WebURLLoaderImpl::Context::OnReceivedResponse(
       mode = SharedMemoryDataConsumerHandle::kApplyBackpressure;
     }
 
-    auto reader = make_scoped_ptr(
-        new SharedMemoryDataConsumerHandle(mode, &body_stream_writer_));
+    auto read_handle = make_scoped_ptr(new SharedMemoryDataConsumerHandle(
+        mode, base::Bind(&Context::CancelBodyStreaming, this),
+        &body_stream_writer_));
 
-    // The client takes |reader|'s ownership.
-    client_->didReceiveResponse(loader_, response, reader.release());
+    // Here |body_stream_writer_| has an indirect reference to |this| and that
+    // creates a reference cycle, but it is not a problem because the cycle
+    // will break if one of the following happens:
+    //  1) The body data transfer is done (with or without an error).
+    //  2) |read_handle| (and its reader) is detached.
+
+    // The client takes |read_handle|'s ownership.
+    client_->didReceiveResponse(loader_, response, read_handle.release());
     // TODO(yhirano): Support ftp listening and multipart
     return;
   } else {
@@ -754,6 +763,28 @@ WebURLLoaderImpl::Context::~Context() {
   }
 }
 
+void WebURLLoaderImpl::Context::CancelBodyStreaming() {

[hiroshige, 2015/06/22 18:23:16]

L768-784 cancels downstream (the same as OnCompletedRequest() with error) and
Cancel() cancels upstream, right? Adding a comment for it might be helpful.

optional: merging L768-784 with OnCompletedRequest() (or calling
OnCompletedRequest() here) might be helpful for avoiding duplicated code.

[yhirano, 2015/06/23 05:28:38]

Done.
I don't know what I should provide as parameters (was_ignored_by_handler,
stale_copy_in_cache and security_info), so I would keep the current, more direct
way.

+  scoped_refptr<Context> protect(this);
+  if (ftp_listing_delegate_) {
+    ftp_listing_delegate_->OnCompletedRequest();
+    ftp_listing_delegate_.reset(NULL);
+  } else if (multipart_delegate_) {
+    multipart_delegate_->OnCompletedRequest();
+    multipart_delegate_.reset(NULL);
+  }
+
+  if (body_stream_writer_) {
+    body_stream_writer_->Fail();
+    body_stream_writer_.reset();
+  }
+  if (client_) {
+    // TODO(yhirano): Set |stale_copy_in_cache| appropriately if possible.
+    client_->didFail(
+        loader_, CreateWebURLError(request_.url(), false, net::ERR_ABORTED));
+  }
+  Cancel();
+}
+
 bool WebURLLoaderImpl::Context::CanHandleDataURLRequestLocally() const {
   GURL url = request_.url();
   if (!url.SchemeIs(url::kDataScheme))