Downloads: Determine paused state from download job and in-progress db

Currently to determine whether a download was paused by user, we look at
DownloadJob. This doesn't take into account whether the download has
been restarted. This CL consults with the in-progress DB to
determine this correctly.

Change-Id: I00212fd995f2810ddbb68f97d5b8d1594bc2cd69
Bug: 909014
Reviewed-on: https://chromium-review.googlesource.com/c/1352615Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612940}

components/download/internal/common/download_item_impl.cc

@@ -297,6 +297,7 @@ DownloadItemImpl::DownloadItemImpl(
     DownloadItem::DownloadState state,
     DownloadDangerType danger_type,
     DownloadInterruptReason interrupt_reason,
+    bool paused,
     bool opened,
     base::Time last_access_time,
     bool transient,
@@ -322,6 +323,7 @@ DownloadItemImpl::DownloadItemImpl(
       state_(ExternalToInternalState(state)),
       danger_type_(danger_type),
       delegate_(delegate),
+      paused_(paused),
       opened_(opened),
       last_access_time_(last_access_time),
       transient_(transient),
@@ -517,20 +519,18 @@ void DownloadItemImpl::Pause() {
     case CANCELLED_INTERNAL:
     case COMPLETE_INTERNAL:
     case COMPLETING_INTERNAL:
+      return;
     case INITIAL_INTERNAL:
     case INTERRUPTED_INTERNAL:
     case INTERRUPTED_TARGET_PENDING_INTERNAL:
     case RESUMING_INTERNAL:
       // No active request.
-      // TODO(asanka): In the case of RESUMING_INTERNAL, consider setting
-      // |DownloadJob::is_paused_| even if there's no request currently
-      // associated with this DII. When a request is assigned (due to a
-      // resumption, for example) we can honor the |DownloadJob::is_paused_|
-      // setting.
+      paused_ = true;
       return;
 
     case IN_PROGRESS_INTERNAL:
     case TARGET_PENDING_INTERNAL:
+      paused_ = true;
       job_->Pause();
       UpdateObservers();
       return;
@@ -557,12 +557,14 @@ void DownloadItemImpl::Resume() {
     case IN_PROGRESS_INTERNAL:
       if (!IsPaused())
         return;
+      paused_ = false;
       if (job_)
         job_->Resume(true);
       UpdateObservers();
       return;
 
     case INTERRUPTED_INTERNAL:
+      paused_ = false;
       auto_resume_count_ = 0;  // User input resets the counter.
       ResumeInterruptedDownload(ResumptionRequestSource::USER);
       UpdateObservers();
@@ -646,7 +648,7 @@ DownloadInterruptReason DownloadItemImpl::GetLastReason() const {
 }
 
 bool DownloadItemImpl::IsPaused() const {
-  return job_ ? job_->is_paused() : false;
+  return paused_;
 }
 
 bool DownloadItemImpl::IsTemporary() const {

components/download/internal/common/in_progress_download_manager.cc

@@ -54,8 +54,9 @@ std::unique_ptr<DownloadItemImpl> CreateDownloadItemImpl(
       in_progress_info->last_modified, in_progress_info->received_bytes,
       in_progress_info->total_bytes, in_progress_info->hash,
       in_progress_info->state, in_progress_info->danger_type,
-      in_progress_info->interrupt_reason, false, base::Time(),
-      in_progress_info->transient, in_progress_info->received_slices);
+      in_progress_info->interrupt_reason, in_progress_info->paused, false,
+      base::Time(), in_progress_info->transient,
+      in_progress_info->received_slices);
 }
 
 void OnUrlDownloadHandlerCreated(

components/download/public/common/download_item.h

@@ -205,8 +205,10 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItem : public base::SupportsUserData {
   // reason.
   virtual DownloadInterruptReason GetLastReason() const = 0;
 
-  // The download is currently paused. Calling Resume() will transition out of
-  // this paused state.
+  // Returns whether download is currently paused explicitly by the user. The
+  // download state should be checked in conjunction with this method to
+  // determine whether the download was truly paused. Calling Resume() will
+  // transition out of this paused state.
   virtual bool IsPaused() const = 0;
 
   // DEPRECATED. True if this is a temporary download and should not be

components/download/public/common/download_item_impl.h

@@ -181,6 +181,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl
       DownloadItem::DownloadState state,
       DownloadDangerType danger_type,
       DownloadInterruptReason interrupt_reason,
+      bool paused,
       bool opened,
       base::Time last_access_time,
       bool transient,
@@ -706,6 +707,11 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl
   // True if the item was downloaded temporarily.
   bool is_temporary_ = false;
 
+  // True if the item was explicity paused by the user. This should be checked
+  // in conjunction with the download state to determine whether the download
+  // was truly paused.
+  bool paused_ = false;
+
   // Did the user open the item either directly or indirectly (such as by
   // setting always open files of this type)? The shelf also sets this field
   // when the user closes the shelf before the item has been opened but should

components/download/public/common/mock_download_item_impl.cc

@@ -30,6 +30,7 @@ MockDownloadItemImpl::MockDownloadItemImpl(DownloadItemImplDelegate* delegate)
                        DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
                        DOWNLOAD_INTERRUPT_REASON_NONE,
                        false,
+                       false,
                        base::Time(),
                        true,
                        DownloadItem::ReceivedSlices()) {}

content/browser/download/download_manager_impl.cc

@@ -237,7 +237,8 @@ class DownloadItemFactoryImpl : public download::DownloadItemFactory {
         referrer_url, site_url, tab_url, tab_refererr_url, mime_type,
         original_mime_type, start_time, end_time, etag, last_modified,
         received_bytes, total_bytes, hash, state, danger_type, interrupt_reason,
-        opened, last_access_time, transient, received_slices);
+        false /* paused */, opened, last_access_time, transient,
+        received_slices);
   }
 
   download::DownloadItemImpl* CreateActiveItem(

content/browser/download/download_manager_impl_unittest.cc

@@ -751,7 +751,7 @@ TEST_F(DownloadManagerTest, OnInProgressDownloadsLoaded) {
       base::Time::Now(), std::string(), std::string(), 10, 10, std::string(),
       download::DownloadItem::INTERRUPTED,
       download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
-      download::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED, false,
+      download::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED, false, false,
       base::Time::Now(), true,
       std::vector<download::DownloadItem::ReceivedSlice>());
   in_progress_manager->AddDownloadItem(std::move(in_progress_item));