Pull delegate check back into IsDownloadReadyForCompletion so that if

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 1044 matching lines @@
 // IsDownloadReadyForCompletion(). GDataDownloadObserver manually calls
 // DownloadItem::UpdateObservers() when the upload completes so that SavePackage
 // notices that the upload has completed and runs its normal Finish() pathway.
 // MaybeCompleteDownload() is never the mechanism by which SavePackage completes
 // downloads. SavePackage always uses its own Finish() to mark downloads
 // complete.
 void DownloadItemImpl::MaybeCompleteDownload() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!is_save_package_download_);
 
-  if (!IsDownloadReadyForCompletion())
+  if (!IsDownloadReadyForCompletion(
+          base::Bind(&DownloadItemImpl::MaybeCompleteDownload,
+                     weak_ptr_factory_.GetWeakPtr())))
     return;
 
   // TODO(rdsmith): DCHECK that we only pass through this point
   // once per download.  The natural way to do this is by a state
   // transition on the DownloadItem.
 
   // Confirm we're in the proper set of states to be here;
   // have all data, have a history handle, (validated or safe).
   DCHECK_EQ(IN_PROGRESS_INTERNAL, state_);
   DCHECK_NE(DownloadItem::DANGEROUS, GetSafetyState());
   DCHECK(all_data_saved_);
 
   OnDownloadCompleting();
 }
 
 // Called by MaybeCompleteDownload() when it has determined that the download
 // is ready for completion.
 void DownloadItemImpl::OnDownloadCompleting() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (state_ != IN_PROGRESS_INTERNAL)
     return;
 
-  // Give the delegate a chance to override.
-  delegate_->ReadyForDownloadCompletion(
-      this, base::Bind(&DownloadItemImpl::ReadyForDownloadCompletionDone,
-                       weak_ptr_factory_.GetWeakPtr()));
-}
-
-void DownloadItemImpl::ReadyForDownloadCompletionDone() {
-  if (state_ != IN_PROGRESS_INTERNAL)
-    return;
-
   VLOG(20) << __FUNCTION__ << "()"
            << " " << DebugString(true);
   DCHECK(!GetTargetFilePath().empty());
   DCHECK_NE(DANGEROUS, GetSafetyState());
 
   // TODO(rdsmith/benjhayden): Remove as part of SavePackage integration.
   if (is_save_package_download_) {
     // Avoid doing anything on the file thread; there's nothing we control
     // there.
     // Strictly speaking, this skips giving the embedder a chance to open
@@ skipping 131 matching lines @@
   // download_file_ can be NULL if Interrupt() is called after the download file
   // has been released.
   if (!is_save_package_download_ && download_file_.get()) {
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         // Will be deleted at end of task execution.
         base::Bind(&DownloadFileCancel, base::Passed(&download_file_)));
   }
 }
 
-bool DownloadItemImpl::IsDownloadReadyForCompletion() {
-  VLOG(20) << __FUNCTION__ << " " << AllDataSaved()
-           << " " << (GetSafetyState() != DownloadItem::DANGEROUS)
-           << " " << (state_ == IN_PROGRESS_INTERNAL)
-           << " " << !GetTargetFilePath().empty()
-           << " " << (target_path_.DirName() == current_path_.DirName());
+bool DownloadItemImpl::IsDownloadReadyForCompletion(
+    const base::Closure& state_change_notification) {
   // If we don't have all the data, the download is not ready for
   // completion.
   if (!AllDataSaved())
     return false;
 
   // If the download is dangerous, but not yet validated, it's not ready for
   // completion.
   if (GetSafetyState() == DownloadItem::DANGEROUS)
     return false;
 
   // If the download isn't active (e.g. has been cancelled) it's not
   // ready for completion.
   if (state_ != IN_PROGRESS_INTERNAL)
     return false;
 
   // If the target filename hasn't been determined, then it's not ready for
   // completion. This is checked in ReadyForDownloadCompletionDone().
   if (GetTargetFilePath().empty())
     return false;
 
   // This is checked in NeedsRename(). Without this conditional,
   // browser_tests:DownloadTest.DownloadMimeType fails the DCHECK.
   if (target_path_.DirName() != current_path_.DirName())
     return false;
 
+  // Give the delegate a chance to hold up a stop sign.  It'll call
+  // use back through the passed callback if it does and that state changes.
+  if (!delegate_->ShouldCompleteDownload(this, state_change_notification))
+    return false;
+
   return true;
 }
 
 void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) {
   if (state_ == new_state)
     return;
 
   DownloadInternalState old_state = state_;
   state_ = new_state;
 
@@ skipping 122 matching lines @@
       return "CANCELLED";
     case INTERRUPTED_INTERNAL:
       return "INTERRUPTED";
     default:
       NOTREACHED() << "Unknown download state " << state;
       return "unknown";
   };
 }
 
 }  // namespace content