[Downloads] Always call DM::StartDownload() for explicit downloads.

content/browser/download/download_resource_handler.h

Index: content/browser/download/download_resource_handler.h
diff --git a/content/browser/download/download_resource_handler.h b/content/browser/download/download_resource_handler.h
index 22a10a8a04998691e58b7d0ed5a7d46c44b35464..9eda0744dda303d6b492869295ab606ed9fed3a1 100644
--- a/content/browser/download/download_resource_handler.h
+++ b/content/browser/download/download_resource_handler.h
@@ -33,17 +33,14 @@ struct DownloadCreateInfo;
 // Forwards data to the download thread.
 class CONTENT_EXPORT DownloadResourceHandler
     : public ResourceHandler,
+      public DownloadRequestCore::Delegate,
       public base::SupportsWeakPtr<DownloadResourceHandler> {
  public:
   struct DownloadTabInfo;
 
   // started_cb will be called exactly once on the UI thread.
   // |id| should be invalid if the id should be automatically assigned.
-  DownloadResourceHandler(
-      uint32_t id,
-      net::URLRequest* request,
-      const DownloadUrlParameters::OnStartedCallback& started_cb,
-      scoped_ptr<DownloadSaveInfo> save_info);
+  DownloadResourceHandler(net::URLRequest* request);
 
   bool OnRequestRedirected(const net::RedirectInfo& redirect_info,
                            ResourceResponse* response,
@@ -84,18 +81,12 @@ class CONTENT_EXPORT DownloadResourceHandler
  private:
   ~DownloadResourceHandler() override;
 
-  // Arrange for started_cb_ to be called on the UI thread with the
-  // below values, nulling out started_cb_.  Should only be called
-  // on the IO thread.
-  void CallStartedCB(DownloadInterruptReason interrupt_reason);
-
-  void OnCoreReadyToRead();
-
-  uint32_t download_id_;
-
-  // This is read only on the IO thread, but may only
-  // be called on the UI thread.
-  DownloadUrlParameters::OnStartedCallback started_cb_;
+  // DownloadRequestCore::Delegate
+  void OnStart(
+      scoped_ptr<DownloadCreateInfo> download_create_info,
+      scoped_ptr<ByteStreamReader> stream_reader,
+      const DownloadUrlParameters::OnStartedCallback& callback) override;
+  void OnReadyToRead() override;
 
   // Stores information about the download that must be acquired on the UI
   // thread before StartOnUIThread is called.
@@ -106,6 +97,7 @@ class CONTENT_EXPORT DownloadResourceHandler
   scoped_ptr<DownloadTabInfo> tab_info_;
 
   DownloadRequestCore core_;
+  scoped_refptr<ResourceResponse> response_;

[Randy Smith (Not in Mondays), 2016/02/10 21:48:45]

IIUC, this is being used to pass information from OnResponseStarted() to
OnStart() (which is called by way of core_.OnResponseStarted()).  I wince at the
pattern of using a class data member to convey information to another level of
the same call stack; it feels like using a global variable, though not quite as
bad.  I want to at least explore alternatives; would a "std::string
override_mime_type" (which might be empty) argument to
DownloadRequestCore::OnResponseStarted() be a reasonable alternative?

[asanka, 2016/02/11 03:43:07]

Switched to original_mime_type which is the name we use in DownloadItem to refer
to the field.

[Randy Smith (Not in Mondays), 2016/02/11 22:08:08]

Two thoughts:
* Huh.  Ok, but "original_mime_type_" I find a bit misleading, since the reason
we're keeping this here is to handle the case where the MimeTypeResourceHandler
overrode the *original* original mime type.
* Happy to go with your preference, but what's your thoughs around not doing
this as an argument to OnResponseStarted()?

[Randy Smith (Not in Mondays), 2016/02/12 18:01:18]

Still hoping you'll respond on this issue?

[asanka, 2016/02/12 18:31:46]

Went with an extra argument to OnResponseStarted().

   DISALLOW_COPY_AND_ASSIGN(DownloadResourceHandler);
 };