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

content/browser/download/download_resource_handler.h

 // Copyright (c) 2012 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 CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_RESOURCE_HANDLER_H_
 #define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_RESOURCE_HANDLER_H_
 
 #include <stdint.h>
 
 #include <string>
@@ skipping 15 matching lines @@
 namespace content {
 class ByteStreamReader;
 class ByteStreamWriter;
 class DownloadRequestHandle;
 class PowerSaveBlocker;
 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,
                            bool* defer) override;
 
   // Send the download creation information to the download thread.
   bool OnResponseStarted(ResourceResponse* response, bool* defer) override;
 
   // Pass-through implementation.
   bool OnWillStart(const GURL& url, bool* defer) override;
@@ skipping 20 matching lines @@
   void ResumeRequest();
 
   // May result in this object being deleted by its owner.
   void CancelRequest();
 
   std::string DebugString() const;
 
  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.
   // Created on IO thread, but only accessed on UI thread. |tab_info_| holds
   // the pointer until we pass it off to StartOnUIThread or DeleteSoon.
   // Marked as a scoped_ptr<> to indicate ownership of the structure, but
   // deletion must occur on the IO thread.
   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);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_RESOURCE_HANDLER_H_