Fix an issue that we didn't clean url request properly.

content/browser/download/parallel_download_job.cc

 // Copyright 2017 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 "content/browser/download/parallel_download_job.h"
 
 #include <algorithm>
 
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 79 matching lines @@
 }
 
 int ParallelDownloadJob::GetMinRemainingTimeInSeconds() const {
   return GetParallelRequestRemainingTimeConfig().InSeconds();
 }
 
 bool ParallelDownloadJob::UsesParallelRequests() const {
   return true;
 }
 
+void ParallelDownloadJob::CancelRequestWithOffset(int64_t offset) {
+  if (initial_request_offset_ == offset) {
+    DownloadJobImpl::Cancel(false);

[asanka, 2017/04/21 15:09:28]

I guess the expectation is that this will go through the DownloadJobImpl ->
DownloadResourceHandle -> ResourceDispatcherHost -> ResourceLoader ->
net::URLRequest -> ResourceLoader -> DownloadResourceHandler ->
DownloadRequestCore -> DownloadFileImpl -> DownloadItemImpl and cancel the
download? Can you add a test for this case because that's a pretty big chunk of
code with a good chance of breaking. I don't know if this even works for
anything other than http/s.

[xingliu, 2017/04/21 18:07:00]

ParallelDownloadComplete browser test covers this flow. Will add more unit tests
in later CL.


The flow is used when we need to close the ByteStream in DownloadFileImpl, so
the beginning point of the flow is actually DownloadFileImpl.

After we refactor the download file to be owned by DownloadJob, this function
will be removed.

One question about the non http cases, do we mean quic here? or other protocol
also supported in UrlRequest like ftp?

[xingliu, 2017/04/22 00:07:24]

The flow is DownloadFileImpl ==> DownloadItemImpl ==> DownloadJob ==> close a
particular connection.

+    return;
+  }
+
+  auto it = workers_.find(offset);
+  if (it != workers_.end())
+    it->second->Cancel();

[asanka, 2017/04/21 15:09:28]

Is there a reason to consider the |it == workers_.end()| as a valid outcome
here? It looks like workers are added to workers_, but never removed.

[xingliu, 2017/04/22 00:07:24]

Done, added a DCHECK.

In the long term, after the job owns the download file, job's life cycle may be
changed to be the similar to download file.

+}
+
 void ParallelDownloadJob::BuildParallelRequestAfterDelay() {
   DCHECK(workers_.empty());
   DCHECK(!requests_sent_);
   DCHECK(!timer_.IsRunning());
 
   timer_.Start(FROM_HERE, GetParallelRequestDelayConfig(), this,
                &ParallelDownloadJob::BuildParallelRequests);
 }
 
 void ParallelDownloadJob::OnByteStreamReady(
@@ skipping 126 matching lines @@
   // download request.
   download_params->set_referrer(Referrer(download_item_->GetReferrerUrl(),
                                          blink::kWebReferrerPolicyAlways));
   // Send the request.
   worker->SendRequest(std::move(download_params));
   DCHECK(workers_.find(offset) == workers_.end());
   workers_[offset] = std::move(worker);
 }
 
 }  // namespace content