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

content/browser/download/download_request_core.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_REQUEST_CORE_H_
 #define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_CORE_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <string>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "content/browser/loader/resource_handler.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_save_info.h"
 #include "content/public/browser/download_url_parameters.h"
 
 namespace net {
+class HttpResponseHeaders;
 class URLRequest;
+class URLRequestStatus;
 }  // namespace net
 
 namespace content {
 class DownloadManagerImpl;
 class ByteStreamReader;
 class ByteStreamWriter;
 class PowerSaveBlocker;
 struct DownloadCreateInfo;
 
 // This class encapsulates the core logic for reading data from a URLRequest and
 // writing it into a ByteStream. It's common to both DownloadResourceHandler and
 // UrlDownloader.
 //
 // Created, lives on and dies on the IO thread.
 class CONTENT_EXPORT DownloadRequestCore
     : public base::SupportsWeakPtr<DownloadRequestCore> {
  public:
-  // Size of the buffer used between the DownloadRequestCore and the
-  // downstream receiver of its output.
-  static const int kDownloadByteStreamSize;
+  class Delegate {
+   public:
+    virtual void OnReadyToRead() = 0;
+    virtual void OnStart(
+        scoped_ptr<DownloadCreateInfo> download_create_info,
+        scoped_ptr<ByteStreamReader> stream_reader,
+        const DownloadUrlParameters::OnStartedCallback& callback) = 0;
+  };
 
-  // |request| *must* outlive the DownloadRequestCore. |save_info| must be
-  // valid.
-  //
-  // Invokes |on_ready_to_read_callback| if a previous call to OnReadCompleted()
-  // resulted in |defer| being set to true, and DownloadRequestCore is now ready
-  // to commence reading.
-  DownloadRequestCore(net::URLRequest* request,
-                      scoped_ptr<DownloadSaveInfo> save_info,
-                      const base::Closure& on_ready_to_read_callback);
+  // All parameters are required. |request| and |delegate| must outlive
+  // DownloadRequestCore.
+  DownloadRequestCore(net::URLRequest* request, Delegate* delegate);
   ~DownloadRequestCore();
 
   // Should be called when the URLRequest::Delegate receives OnResponseStarted.
-  // Constructs a DownloadCreateInfo and a ByteStreamReader that should be
-  // passed into DownloadManagerImpl::StartDownload().
-  //
-  // Only populates the response derived fields of DownloadCreateInfo, with the
-  // exception of |save_info|.
-  void OnResponseStarted(scoped_ptr<DownloadCreateInfo>* info,
-                         scoped_ptr<ByteStreamReader>* stream_reader);
+  // Invokes Delegate::OnStart() with download start parameters.
+  bool OnResponseStarted();
 
   // Starts a read cycle. Creates a new IOBuffer which can be passed into
   // URLRequest::Read(). Call OnReadCompleted() when the Read operation
   // completes.
   bool OnWillRead(scoped_refptr<net::IOBuffer>* buf,
                   int* buf_size,
                   int min_size);
 
   // Should be called when the Read() operation completes. |defer| will be set
   // to true if reading is to be suspended. In the latter case, once more data
   // can be read, invokes the |on_ready_to_read_callback|.
   bool OnReadCompleted(int bytes_read, bool* defer);
 
-  // Called to signal that the response is complete. If the return value is
-  // something other than DOWNLOAD_INTERRUPT_REASON_NONE, then the download
-  // should be considered interrupted.
+  // Called to signal that the response is complete.
   //
   // It is expected that once this method is invoked, the DownloadRequestCore
   // object will be destroyed in short order without invoking any other methods
   // other than the destructor.
-  DownloadInterruptReason OnResponseCompleted(
-      const net::URLRequestStatus& status);
+  void OnResponseCompleted(const net::URLRequestStatus& status);
+
+  // Used to notify DownloadRequestCore that the caller is about to abort the
+  // outer request. |reason| will be used as the final interrupt reason when
+  // OnResponseCompleted() is called.
+  void OnWillAbort(DownloadInterruptReason reason);
+
+  void OnAbortedBeforeStart(DownloadInterruptReason reason);

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

Um ... this is probably my being clueless, but I can't find callers for either
of these routines in this CL (searching the raw diff).  Where are they called
from?

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

Ping?  git grep on a patched checkout seems to agree with the raw diff search.

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

Removed. There are for the dependent CL.

 
   // Called if the request should suspend reading. A subsequent
   // OnReadCompleted() will result in |defer| being set to true.
   //
   // Each PauseRequest() must be balanced with a call to ResumeRequest().
   void PauseRequest();
 
   // Balances a call to PauseRequest(). If no more pauses are outstanding and
   // the reader end of the ByteStream is ready to receive more data,
   // DownloadRequestCore will invoke the |on_ready_to_read_callback| to signal
   // to the caller that the read cycles should commence.
   void ResumeRequest();
 
   std::string DebugString() const;
 
+  static scoped_ptr<net::URLRequest> CreateRequestOnIOThread(
+      uint32_t download_id,
+      DownloadUrlParameters* params);
+
+  // Size of the buffer used between the DownloadRequestCore and the
+  // downstream receiver of its output.
+  static const int kDownloadByteStreamSize;
+
  protected:
   net::URLRequest* request() const { return request_; }
 
  private:
-  base::Closure on_ready_to_read_callback_;
+  static DownloadInterruptReason HandleRequestStatus(
+      const net::URLRequestStatus& status);
+
+  static DownloadInterruptReason HandleSuccessfulServerResponse(
+      const net::HttpResponseHeaders& http_headers,
+      DownloadSaveInfo* save_info);
+
+  scoped_ptr<DownloadCreateInfo> CreateDownloadCreateInfo(
+      DownloadInterruptReason result);
+
+  Delegate* delegate_;
   net::URLRequest* request_;
+
+  // "Passthrough" fields. These are only kept here so that they can be used to
+  // populate the DownloadCreateInfo when the time comes.

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

nit, suggestion: Given that we have the URLRequest and these fields are the ones
we're putting in the UserData on the URLRequest, could we grab them from there
when populating the DCI?  This is a little wonky since the DRC makes more
conceptual sense than UserData on URLRequest, but I think that storing in one
place is better than storing in two.

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

Ping?

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

I started out doing that, but it ended up not being quite as clean as I wanted
because of requests that don't go through CreateRequestOnIOThread()

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

Ok with this not being in this CL, but: Aren't those exactly the requests where
these fields aren't needed (i.e. the ones in which we're getting a new request
from the renderers, and thus none of save_info_, id, or the callback is used)?

[asanka, 2016/02/12 20:52:28]

True. But there would also not be a UserData. So we'd end up having to check a
few places whether there's a UserData or not. The alternative is to use the
default (empty) values for these fields when there are no explicit values to
use. That's what we are doing in this CL.

   scoped_ptr<DownloadSaveInfo> save_info_;
+  uint32_t download_id_;
+  DownloadUrlParameters::OnStartedCallback on_started_callback_;
 
   // Data flow
   scoped_refptr<net::IOBuffer> read_buffer_;    // From URLRequest.
   scoped_ptr<ByteStreamWriter> stream_writer_;  // To rest of system.
 
   // Keeps the system from sleeping while this is alive. If the
   // system enters power saving mode while a request is alive, it can cause the
   // request to fail and the associated download will be interrupted.
   scoped_ptr<PowerSaveBlocker> power_save_blocker_;
 
   // The following are used to collect stats.
   base::TimeTicks download_start_time_;
   base::TimeTicks last_read_time_;
   base::TimeTicks last_stream_pause_time_;
   base::TimeDelta total_pause_time_;
   size_t last_buffer_size_;
   int64_t bytes_read_;
 
   int pause_count_;
   bool was_deferred_;
+  bool is_resumption_request_;
+  bool started_;
+
+  // When DownloadRequestCore initiates an abort (by blocking a redirect, for
+  // example) it expects to eventually receive a OnResponseCompleted() with a
+  // status indicating that the request was aborted. When this happens, the
+  // interrupt reason in |abort_reason_| will be used instead of USER_CANCELED
+  // which is vague.
+  DownloadInterruptReason abort_reason_;
 
   // Each successful OnWillRead will yield a buffer of this size.
   static const int kReadBufSize = 32768;   // bytes
 
   DISALLOW_COPY_AND_ASSIGN(DownloadRequestCore);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_CORE_H_