Revert "Fix an issue that ResourceDownloader was never deleted on download completion"

This reverts commit e0b12de0.

Reason for revert: The Cl probably introduced flakiness in multiple download related tests (https://crbug.com/864922)

Original change's description:
> Fix an issue that ResourceDownloader was never deleted on download completion
> 
> This CL calls the InProgressDownloadManager to delete ResourceDownloader
> after response is completed.
> It also fixes the issue when download is cancelled.
> 
> BUG=864189
> 
> Change-Id: I184d5faaace90f49874e45b4126a0a9821fdced7
> Reviewed-on: https://chromium-review.googlesource.com/1138676
> Commit-Queue: Min Qin <qinmin@chromium.org>
> Reviewed-by: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575882}

TBR=qinmin@chromium.org,shaktisahu@chromium.org,xingliu@chromium.org

Change-Id: I669a69fc135547508b44ccc8150847cc5824b2ec
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 864189
Reviewed-on: https://chromium-review.googlesource.com/1141684Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575981}

components/download/internal/common/BUILD.gn

@@ -48,7 +48,6 @@ source_set("internal") {
     "save_package_download_job.h",
     "stream_handle_input_stream.cc",
     "url_download_handler_factory.cc",
-    "url_download_request_handle.cc",
   ]
 
   public_deps = [

components/download/internal/common/download_response_handler.cc

@@ -202,10 +202,8 @@ void DownloadResponseHandler::OnComplete(
         ConvertInterruptReasonToMojoNetworkRequestStatus(reason));
   }
 
-  if (started_) {
-    delegate_->OnResponseCompleted();
+  if (started_)
     return;
-  }
 
   // OnComplete() called without OnReceiveResponse(). This should only
   // happen when the request was aborted.
@@ -213,7 +211,6 @@ void DownloadResponseHandler::OnComplete(
   create_info_->result = reason;
 
   OnResponseStarted(mojom::DownloadStreamHandlePtr());
-  delegate_->OnResponseCompleted();
 }
 
 void DownloadResponseHandler::OnResponseStarted(

components/download/internal/common/resource_downloader.cc

@@ -8,7 +8,6 @@
 
 #include "components/download/public/common/download_url_loader_factory_getter.h"
 #include "components/download/public/common/stream_handle_input_stream.h"
-#include "components/download/public/common/url_download_request_handle.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace network {
@@ -199,8 +198,6 @@ void ResourceDownloader::InterceptResponse(
 void ResourceDownloader::OnResponseStarted(
     std::unique_ptr<DownloadCreateInfo> download_create_info,
     mojom::DownloadStreamHandlePtr stream_handle) {
-  download_create_info->request_handle.reset(new UrlDownloadRequestHandle(
-      weak_ptr_factory_.GetWeakPtr(), base::SequencedTaskRunnerHandle::Get()));
   download_create_info->is_new_download = is_new_download_;
   download_create_info->guid = guid_;
   download_create_info->site_url = site_url_;
@@ -222,19 +219,4 @@ void ResourceDownloader::OnReceiveRedirect() {
   url_loader_->FollowRedirect(base::nullopt, base::nullopt);
 }
 
-void ResourceDownloader::OnResponseCompleted() {
-  Destroy();
-}
-
-void ResourceDownloader::CancelRequest() {
-  Destroy();
-}
-
-void ResourceDownloader::Destroy() {
-  delegate_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&UrlDownloadHandler::Delegate::OnUrlDownloadStopped,
-                     delegate_, this));
-}
-
 }  // namespace download

components/download/internal/common/resource_downloader.h

@@ -73,7 +73,6 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader
       std::unique_ptr<download::DownloadCreateInfo> download_create_info,
       download::mojom::DownloadStreamHandlePtr stream_handle) override;
   void OnReceiveRedirect() override;
-  void OnResponseCompleted() override;
 
  private:
   // Helper method to start the network request.
@@ -88,12 +87,6 @@ class COMPONENTS_DOWNLOAD_EXPORT ResourceDownloader
       net::CertStatus cert_status,
       network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints);
 
-  // UrlDownloadHandler implementations.
-  void CancelRequest() override;
-
-  // Ask the |delegate_| to destroy this object.
-  void Destroy();
-
   base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate_;
 
   // The ResourceRequest for this object.

components/download/internal/common/url_download_request_handle.cc

-// Copyright 2018 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 "components/download/public/common/url_download_request_handle.h"
-
-namespace download {
-
-UrlDownloadRequestHandle::UrlDownloadRequestHandle(
-    base::WeakPtr<UrlDownloadHandler> downloader,
-    scoped_refptr<base::SequencedTaskRunner> downloader_task_runner)
-    : downloader_(downloader),
-      downloader_task_runner_(downloader_task_runner) {}
-
-UrlDownloadRequestHandle::UrlDownloadRequestHandle(
-    UrlDownloadRequestHandle&& other)
-    : downloader_(std::move(other.downloader_)),
-      downloader_task_runner_(std::move(other.downloader_task_runner_)) {}
-
-UrlDownloadRequestHandle& UrlDownloadRequestHandle::operator=(
-    UrlDownloadRequestHandle&& other) {
-  downloader_ = std::move(other.downloader_);
-  downloader_task_runner_ = std::move(other.downloader_task_runner_);
-  return *this;
-}
-
-UrlDownloadRequestHandle::~UrlDownloadRequestHandle() = default;
-
-void UrlDownloadRequestHandle::PauseRequest() {
-  downloader_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&UrlDownloadHandler::PauseRequest, downloader_));
-}
-
-void UrlDownloadRequestHandle::ResumeRequest() {
-  downloader_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&UrlDownloadHandler::ResumeRequest, downloader_));
-}
-
-void UrlDownloadRequestHandle::CancelRequest(bool user_cancel) {
-  downloader_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&UrlDownloadHandler::CancelRequest, downloader_));
-}
-
-}  // namespace download

components/download/public/common/BUILD.gn

@@ -57,7 +57,6 @@ component("public") {
     "resume_mode.h",
     "stream_handle_input_stream.h",
     "url_download_handler_factory.h",
-    "url_download_request_handle.h",
   ]
 
   configs += [ ":components_download_implementation" ]

components/download/public/common/download_response_handler.h

@@ -26,14 +26,13 @@ namespace download {
 class COMPONENTS_DOWNLOAD_EXPORT DownloadResponseHandler
     : public network::mojom::URLLoaderClient {
  public:
-  // Class for handling the stream response.
+  // Class for handling the stream once response starts.
   class Delegate {
    public:
     virtual void OnResponseStarted(
         std::unique_ptr<DownloadCreateInfo> download_create_info,
         mojom::DownloadStreamHandlePtr stream_handle) = 0;
     virtual void OnReceiveRedirect() = 0;
-    virtual void OnResponseCompleted() = 0;
   };
 
   DownloadResponseHandler(

components/download/public/common/url_download_handler.h

@@ -40,15 +40,6 @@ class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadHandler {
   UrlDownloadHandler() = default;
   virtual ~UrlDownloadHandler() = default;
 
-  // Called on the io thread to pause the url request.
-  virtual void PauseRequest() {}
-
-  // Called on the io thread to resume the url request.
-  virtual void ResumeRequest() {}
-
-  // Called on the io thread to cancel the url request.
-  virtual void CancelRequest() {}
-
   DISALLOW_COPY_AND_ASSIGN(UrlDownloadHandler);
 };
 

components/download/public/common/url_download_request_handle.h

-// Copyright 2018 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.
-
-#ifndef COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_DOWNLOAD_REQUEST_HANDLE_H_
-#define COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_DOWNLOAD_REQUEST_HANDLE_H_
-
-#include "base/memory/weak_ptr.h"
-#include "components/download/public/common/download_export.h"
-#include "components/download/public/common/download_request_handle_interface.h"
-#include "components/download/public/common/url_download_handler.h"
-
-namespace download {
-
-// Implementation of the DownloadRequestHandleInterface to handle url download.
-class COMPONENTS_DOWNLOAD_EXPORT UrlDownloadRequestHandle
-    : public DownloadRequestHandleInterface {
- public:
-  UrlDownloadRequestHandle(
-      base::WeakPtr<UrlDownloadHandler> downloader,
-      scoped_refptr<base::SequencedTaskRunner> downloader_task_runner);
-  UrlDownloadRequestHandle(UrlDownloadRequestHandle&& other);
-  UrlDownloadRequestHandle& operator=(UrlDownloadRequestHandle&& other);
-  ~UrlDownloadRequestHandle() override;
-
-  // DownloadRequestHandleInterface
-  void PauseRequest() override;
-  void ResumeRequest() override;
-  void CancelRequest(bool user_cancel) override;
-
- private:
-  base::WeakPtr<UrlDownloadHandler> downloader_;
-  scoped_refptr<base::SequencedTaskRunner> downloader_task_runner_;
-
-  DISALLOW_COPY_AND_ASSIGN(UrlDownloadRequestHandle);
-};
-
-}  // namespace download
-
-#endif  // COMPONENTS_DOWNLOAD_PUBLIC_COMMON_URL_DOWNLOAD_REQUEST_HANDLE_H_

content/browser/download/url_downloader.cc

@@ -13,7 +13,6 @@
 #include "components/download/public/common/download_interrupt_reasons.h"
 #include "components/download/public/common/download_request_handle_interface.h"
 #include "components/download/public/common/download_url_parameters.h"
-#include "components/download/public/common/url_download_request_handle.h"
 #include "content/browser/byte_stream.h"
 #include "content/browser/download/byte_stream_input_stream.h"
 #include "content/public/browser/browser_thread.h"
@@ -26,6 +25,43 @@
 
 namespace content {
 
+class UrlDownloader::RequestHandle
+    : public download::DownloadRequestHandleInterface {
+ public:
+  RequestHandle(base::WeakPtr<UrlDownloader> downloader,
+                scoped_refptr<base::SequencedTaskRunner> downloader_task_runner)
+      : downloader_(downloader),
+        downloader_task_runner_(downloader_task_runner) {}
+  RequestHandle(RequestHandle&& other)
+      : downloader_(std::move(other.downloader_)),
+        downloader_task_runner_(std::move(other.downloader_task_runner_)) {}
+  RequestHandle& operator=(RequestHandle&& other) {
+    downloader_ = std::move(other.downloader_);
+    downloader_task_runner_ = std::move(other.downloader_task_runner_);
+    return *this;
+  }
+
+  // DownloadRequestHandleInterface
+  void PauseRequest() override {
+    downloader_task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&UrlDownloader::PauseRequest, downloader_));
+  }
+  void ResumeRequest() override {
+    downloader_task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&UrlDownloader::ResumeRequest, downloader_));
+  }
+  void CancelRequest(bool user_cancel) override {
+    downloader_task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&UrlDownloader::CancelRequest, downloader_));
+  }
+
+ private:
+  base::WeakPtr<UrlDownloader> downloader_;
+  scoped_refptr<base::SequencedTaskRunner> downloader_task_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(RequestHandle);
+};
+
 // static
 std::unique_ptr<UrlDownloader> UrlDownloader::BeginDownload(
     base::WeakPtr<download::UrlDownloadHandler::Delegate> delegate,
@@ -185,7 +221,7 @@ void UrlDownloader::OnStart(
     std::unique_ptr<download::DownloadCreateInfo> create_info,
     std::unique_ptr<ByteStreamReader> stream_reader,
     const download::DownloadUrlParameters::OnStartedCallback& callback) {
-  create_info->request_handle.reset(new download::UrlDownloadRequestHandle(
+  create_info->request_handle.reset(new RequestHandle(
       weak_ptr_factory_.GetWeakPtr(), base::SequencedTaskRunnerHandle::Get()));
 
   BrowserThread::PostTask(

content/browser/download/url_downloader.h

@@ -42,6 +42,8 @@ class UrlDownloader : public net::URLRequest::Delegate,
       bool is_parallel_request);
 
  private:
+  class RequestHandle;
+
   void Start();
 
   // URLRequest::Delegate:
@@ -62,10 +64,9 @@ class UrlDownloader : public net::URLRequest::Delegate,
       override;
   void OnReadyToRead() override;
 
-  // UrlDownloadHandler implementations.
-  void PauseRequest() override;
-  void ResumeRequest() override;
-  void CancelRequest() override;
+  void PauseRequest();
+  void ResumeRequest();
+  void CancelRequest();
 
   // Called when the UrlDownloader is done with the request. Posts a task to
   // remove itself from its download manager, which in turn would cause the