Handle early pause, cancel for parallel requests.

content/browser/download/parallel_download_job.cc

Index: content/browser/download/parallel_download_job.cc
diff --git a/content/browser/download/parallel_download_job.cc b/content/browser/download/parallel_download_job.cc
index 1a8b93059b30f1c12f7d4801becd6a38bda5c651..0bc92bdc07ffb69bd7732715aaa7e8c7e81a8eab 100644
--- a/content/browser/download/parallel_download_job.cc
+++ b/content/browser/download/parallel_download_job.cc
@@ -11,6 +11,11 @@
 #include "content/public/browser/storage_partition.h"
 
 namespace content {
+namespace {
+
+const int kVerboseLevel = 1;
+
+}  // namespace
 
 ParallelDownloadJob::ParallelDownloadJob(
     DownloadItemImpl* download_item,
@@ -85,8 +90,28 @@ void ParallelDownloadJob::BuildParallelRequestAfterDelay() {
 void ParallelDownloadJob::OnByteStreamReady(
     DownloadWorker* worker,
     std::unique_ptr<ByteStreamReader> stream_reader) {
-  DownloadJob::AddByteStream(std::move(stream_reader), worker->offset(),
-                             worker->length());
+  // Destroy the request if user canceled.
+  if (DownloadJob::is_canceled()) {
+    VLOG(kVerboseLevel) << "Byte stream arrived after user cancel the request.";
+    worker->Cancel();

[qinmin, 2017/03/21 22:53:55]

do you need to do this? If cancel is called before OnByteStreamReady, shouldn't
worker->cancel() already been called in ParallelDownloadJob::Cancel()?

I think you can just early return here?

[xingliu, 2017/03/21 23:19:14]

At that time the request handle is not set into the worker. In
OnUrlDownloaderStarted the request handle and the byte stream reader is passed
in after response is handled.

So we either let worker know more states and defer the cancel call, or do it
here, I think do it here makes more sense so we don't duplicate states into
workers.

[xingliu, 2017/03/21 23:27:59]

I'm also fine with let worker defer the cancel call, so Job just need to call
worker->Cancel, and worker can query states from Job through Delegate interface.
wdyt?

[qinmin, 2017/03/21 23:48:33]

Yes, it should be worker's responsibility to handle it

[xingliu, 2017/03/22 01:25:59]

Make sense, done.

+    return;
+  }
+
+  // Pause the stream if user paused, still push the stream reader to the sink.
+  if (DownloadJob::is_paused()) {
+    VLOG(kVerboseLevel) << "Byte stream arrived after user pause the request.";
+    worker->Pause();

[qinmin, 2017/03/21 22:53:55]

same here.

[xingliu, 2017/03/21 23:19:14]

We still need to add the stream to DownloadFileImpl on the pause case, so it can
write data. So can't bait out here, but let the logic fall into AddByteStream.

[qinmin, 2017/03/21 23:48:33]

Yes. what i mean is that you don't need to call Pause(). You still need to call
AddByteStream though

[xingliu, 2017/03/22 01:25:59]

Done. Moved to worker.

+  }
+
+  bool success = DownloadJob::AddByteStream(std::move(stream_reader),
+                                            worker->offset(), worker->length());
+
+  // Destroy the request if the sink is gone.
+  if (!success) {
+    VLOG(kVerboseLevel)
+        << "Byte stream arrived after download file is released.";
+    worker->Cancel();
+  }
 }
 
 void ParallelDownloadJob::OnServerResponseError(