downloads: add the ability to undo download removal.

content/browser/download/download_item_impl.cc

 // 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.
 
 // File method ordering: Methods in this file are in the same order as
 // in download_item_impl.h, with the following exception: The public
 // interface Start is placed in chronological order with the other
 // (private) routines that together define a DownloadItem's state
 // transitions as the download progresses.  See "Download progression
 // cascade" later in this file.
@@ skipping 361 matching lines @@
     case CANCELLED_INTERNAL:
     case RESUMING_INTERNAL:
       return;
 
     case INTERRUPTED_INTERNAL:
       auto_resume_count_ = 0;  // User input resets the counter.
       ResumeInterruptedDownload();
       return;
 
     case MAX_DOWNLOAD_INTERNAL_STATE:
+    case REMOVED_INTERNAL:

[benjhayden, 2014/11/13 18:01:59]

Out of order?

Also, how do you ensure that this isn't reached?

       NOTREACHED();
   }
 }
 
 void DownloadItemImpl::Cancel(bool user_cancel) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
   if (state_ != IN_PROGRESS_INTERNAL &&
       state_ != INTERRUPTED_INTERNAL &&
@@ skipping 34 matching lines @@
       !current_path_.empty()) {
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(base::IgnoreResult(&DeleteDownloadedFile), current_path_));
     current_path_.clear();
   }
 
   TransitionTo(CANCELLED_INTERNAL, UPDATE_OBSERVERS);
 }
 
+void DownloadItemImpl::MarkRemoved() {
+  TransitionTo(REMOVED_INTERNAL, UPDATE_OBSERVERS);
+}
+
+void DownloadItemImpl::UndoRemove() {
+  if (state_ != REMOVED_INTERNAL) {

[arv (Not doing code reviews), 2014/11/13 19:02:30]

Why not DCHECK if this stat cannot happen?

+    NOTREACHED();
+    return;
+  }
+  TransitionTo(prev_state_, UPDATE_OBSERVERS);
+}
+
 void DownloadItemImpl::Remove() {
   VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   delegate_->AssertStateConsistent(this);
   Cancel(true);
   delegate_->AssertStateConsistent(this);
 
   NotifyRemoved();
   delegate_->DownloadRemoved(this);
@@ skipping 78 matching lines @@
     case COMPLETE_INTERNAL:
     case CANCELLED_INTERNAL:
       return true;
 
     case INTERRUPTED_INTERNAL:
       return !CanResume();
 
     case RESUMING_INTERNAL:
       return false;
 
+    case REMOVED_INTERNAL:

[benjhayden, 2014/11/13 18:01:59]

How do you ensure that this isn't reached?

     case MAX_DOWNLOAD_INTERNAL_STATE:
       break;
   }
   NOTREACHED();
   return true;
 }
 
 const GURL& DownloadItemImpl::GetURL() const {
   return url_chain_.empty() ? GURL::EmptyGURL() : url_chain_.back();
 }
@@ skipping 1013 matching lines @@
   return true;
 }
 
 void DownloadItemImpl::TransitionTo(DownloadInternalState new_state,
                                     ShouldUpdateObservers notify_action) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (state_ == new_state)
     return;
 
-  DownloadInternalState old_state = state_;
+  prev_state_ = state_;

[benjhayden, 2014/11/13 18:01:59]

I might want to be a bit more conservative here. I'd like to at least discourage
future use of prev_state_ for anything other than UndoRemove(). Maybe rename it
"removed_state_" and only set it when transitioning to REMOVED?

   state_ = new_state;
 
   switch (state_) {
     case COMPLETING_INTERNAL:
       bound_net_log_.AddEvent(
           net::NetLog::TYPE_DOWNLOAD_ITEM_COMPLETING,
           base::Bind(&ItemCompletingNetLogCallback, received_bytes_, &hash_));
       break;
     case COMPLETE_INTERNAL:
       bound_net_log_.AddEvent(
           net::NetLog::TYPE_DOWNLOAD_ITEM_FINISHED,
           base::Bind(&ItemFinishedNetLogCallback, auto_opened_));
       break;
     case INTERRUPTED_INTERNAL:
       bound_net_log_.AddEvent(
           net::NetLog::TYPE_DOWNLOAD_ITEM_INTERRUPTED,
           base::Bind(&ItemInterruptedNetLogCallback, last_reason_,
                      received_bytes_, &hash_state_));
       break;
     case IN_PROGRESS_INTERNAL:
-      if (old_state == INTERRUPTED_INTERNAL) {
+      if (prev_state_ == INTERRUPTED_INTERNAL) {
         bound_net_log_.AddEvent(
             net::NetLog::TYPE_DOWNLOAD_ITEM_RESUMED,
             base::Bind(&ItemResumingNetLogCallback,
                        false, last_reason_, received_bytes_, &hash_state_));
       }
       break;
     case CANCELLED_INTERNAL:
       bound_net_log_.AddEvent(
           net::NetLog::TYPE_DOWNLOAD_ITEM_CANCELED,
           base::Bind(&ItemCanceledNetLogCallback, received_bytes_,
                      &hash_state_));
       break;
     default:
       break;
   }
 
   VLOG(20) << " " << __FUNCTION__ << "()" << " this = " << DebugString(true)
-    << " " << InternalToExternalState(old_state)
+    << " " << InternalToExternalState(prev_state_)
     << " " << InternalToExternalState(state_);
 
   bool is_done = (state_ != IN_PROGRESS_INTERNAL &&
                   state_ != COMPLETING_INTERNAL);
-  bool was_done = (old_state != IN_PROGRESS_INTERNAL &&
-                   old_state != COMPLETING_INTERNAL);
+  bool was_done = (prev_state_ != IN_PROGRESS_INTERNAL &&
+                   prev_state_ != COMPLETING_INTERNAL);
   // Termination
   if (is_done && !was_done)
     bound_net_log_.EndEvent(net::NetLog::TYPE_DOWNLOAD_ITEM_ACTIVE);
 
   // Resumption
   if (was_done && !is_done) {
     std::string file_name(target_path_.BaseName().AsUTF8Unsafe());
     bound_net_log_.BeginEvent(net::NetLog::TYPE_DOWNLOAD_ITEM_ACTIVE,
                               base::Bind(&ItemActivatedNetLogCallback,
                                          this, SRC_ACTIVE_DOWNLOAD,
@@ skipping 114 matching lines @@
     case COMPLETING_INTERNAL:
       return IN_PROGRESS;
     case COMPLETE_INTERNAL:
       return COMPLETE;
     case CANCELLED_INTERNAL:
       return CANCELLED;
     case INTERRUPTED_INTERNAL:
       return INTERRUPTED;
     case RESUMING_INTERNAL:
       return INTERRUPTED;
+    case REMOVED_INTERNAL:
+      return REMOVED;
     case MAX_DOWNLOAD_INTERNAL_STATE:
       break;
   }
   NOTREACHED();
   return MAX_DOWNLOAD_STATE;
 }
 
 // static
 DownloadItemImpl::DownloadInternalState
 DownloadItemImpl::ExternalToInternalState(
     DownloadState external_state) {
   switch (external_state) {
     case IN_PROGRESS:
       return IN_PROGRESS_INTERNAL;
     case COMPLETE:
       return COMPLETE_INTERNAL;
     case CANCELLED:
       return CANCELLED_INTERNAL;
     case INTERRUPTED:
       return INTERRUPTED_INTERNAL;
+    case REMOVED:
+      return REMOVED_INTERNAL;
     default:
       NOTREACHED();
   }
   return MAX_DOWNLOAD_INTERNAL_STATE;
 }
 
 const char* DownloadItemImpl::DebugDownloadStateString(
     DownloadInternalState state) {
   switch (state) {
     case IN_PROGRESS_INTERNAL:
       return "IN_PROGRESS";
     case COMPLETING_INTERNAL:
       return "COMPLETING";
     case COMPLETE_INTERNAL:
       return "COMPLETE";
     case CANCELLED_INTERNAL:
       return "CANCELLED";
     case INTERRUPTED_INTERNAL:
       return "INTERRUPTED";
     case RESUMING_INTERNAL:
       return "RESUMING";
+    case REMOVED_INTERNAL:
+      return "REMOVED";
     case MAX_DOWNLOAD_INTERNAL_STATE:
       break;
   };
   NOTREACHED() << "Unknown download state " << state;
   return "unknown";
 }
 
 const char* DownloadItemImpl::DebugResumeModeString(ResumeMode mode) {
   switch (mode) {
     case RESUME_MODE_INVALID:
       return "INVALID";
     case RESUME_MODE_IMMEDIATE_CONTINUE:
       return "IMMEDIATE_CONTINUE";
     case RESUME_MODE_IMMEDIATE_RESTART:
       return "IMMEDIATE_RESTART";
     case RESUME_MODE_USER_CONTINUE:
       return "USER_CONTINUE";
     case RESUME_MODE_USER_RESTART:
       return "USER_RESTART";
   }
   NOTREACHED() << "Unknown resume mode " << mode;
   return "unknown";
 }
 
 }  // namespace content