Split DownloadFile::Rename into RenameAndUniquify and RenameAndAnnotate.

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
 // interfaces Start, MaybeCompleteDownload, and OnDownloadCompleting
 // are 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
@@ skipping 81 matching lines @@
   virtual void ResumeRequest() const OVERRIDE {}
   virtual void CancelRequest() const OVERRIDE {}
   virtual std::string DebugString() const OVERRIDE {
     return "Null DownloadRequestHandle";
   }
 };
 
 // Wrapper around DownloadFile::Detach and DownloadFile::Cancel that
 // takes ownership of the DownloadFile and hence implicitly destroys it
 // at the end of the function.
-static void DownloadFileDetach(
-    scoped_ptr<DownloadFile> download_file,
-    const DownloadFile::DetachCompletionCallback& callback) {
+static void DownloadFileDetach(scoped_ptr<DownloadFile> download_file) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  download_file->Detach(callback);
+  download_file->Detach();
 }
 
 static void DownloadFileCancel(scoped_ptr<DownloadFile> download_file) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   download_file->Cancel();
 }
 
 }  // namespace
 
 // Our download table ID starts at 1, so we use 0 to represent a download that
@@ skipping 950 matching lines @@
   //               spurious rename when we can just rename to the final
   //               filename. Unnecessary renames may cause bugs like
   //               http://crbug.com/74187.
   DCHECK(!is_save_package_download_);
   DCHECK(download_file_.get());
   DownloadFile::RenameCompletionCallback callback =
       base::Bind(&DownloadItemImpl::OnDownloadRenamedToIntermediateName,
                  weak_ptr_factory_.GetWeakPtr());
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DownloadFile::Rename,
+      base::Bind(&DownloadFile::RenameAndUniquify,
                  // Safe because we control download file lifetime.
                  base::Unretained(download_file_.get()),
-                 intermediate_path, false, callback));
+                 intermediate_path, callback));
 }
 
 void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
     DownloadInterruptReason reason,
     const FilePath& full_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
     Interrupt(reason);
   } else {
     SetFullPath(full_path);
     UpdateObservers();
   }
 
   delegate_->DownloadRenamedToIntermediateName(this);
 }
 
 // When SavePackage downloads MHTML to GData (see
 // SavePackageFilePickerChromeOS), GData calls MaybeCompleteDownload() like it
 // does for non-SavePackage downloads, but SavePackage downloads never satisfy
 // 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())
     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).
@@ skipping 19 matching lines @@
   delegate_->ReadyForDownloadCompletion(
       this, base::Bind(&DownloadItemImpl::ReadyForDownloadCompletionDone,
                        weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DownloadItemImpl::ReadyForDownloadCompletionDone() {
   if (state_ != IN_PROGRESS_INTERNAL)
     return;
 
   VLOG(20) << __FUNCTION__ << "()"
-           << " needs rename = " << NeedsRename()
            << " " << 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.
-    OnDownloadFileReleased(DOWNLOAD_INTERRUPT_REASON_NONE);
+    // Strictly speaking, this skips giving the embedder a chance to open
+    // the download.  But on a save package download, there's no real
+    // concept of opening.
+    Completed();
     return;
   }
 
   DCHECK(download_file_.get());
-  if (NeedsRename()) {
-    DownloadFile::RenameCompletionCallback callback =
-        base::Bind(&DownloadItemImpl::OnDownloadRenamedToFinalName,
-                   weak_ptr_factory_.GetWeakPtr());
-    BrowserThread::PostTask(
-        BrowserThread::FILE, FROM_HERE,
-        base::Bind(&DownloadFile::Rename,
-                   base::Unretained(download_file_.get()),
-                   GetTargetFilePath(), true, callback));
-  } else {
-    ReleaseDownloadFile();
-  }
+  // Unilaterally rename; even if it already has the right name,
+  // we need theannotation.
+  DownloadFile::RenameCompletionCallback callback =
+      base::Bind(&DownloadItemImpl::OnDownloadRenamedToFinalName,
+                 weak_ptr_factory_.GetWeakPtr());
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DownloadFile::RenameAndAnnotate,
+                 base::Unretained(download_file_.get()),
+                 GetTargetFilePath(), callback));
 }
 
 void DownloadItemImpl::OnDownloadRenamedToFinalName(
     DownloadInterruptReason reason,
     const FilePath& full_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!is_save_package_download_);
 
   // If a cancel or interrupt hit, we'll cancel the DownloadFile, which
   // will result in deleting the file on the file thread.  So we don't
   // care about the name having been changed.
   if (state_ != IN_PROGRESS_INTERNAL)
     return;
 
   VLOG(20) << __FUNCTION__ << "()"
            << " full_path = \"" << full_path.value() << "\""
-           << " needed rename = " << NeedsRename()
            << " " << DebugString(false);
-  DCHECK(NeedsRename());
 
   if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
     Interrupt(reason);
     return;
   }
 
-  // full_path is now the current and target file path.
-  DCHECK(!full_path.empty());
-  target_path_ = full_path;
-  SetFullPath(full_path);
-  delegate_->DownloadRenamedToFinalName(this);
+  DCHECK(target_path_ == full_path);
 
-  ReleaseDownloadFile();
-}
+  if (full_path != current_path_) {
+    // full_path is now the current and target file path.
+    DCHECK(!full_path.empty());
+    SetFullPath(full_path);
+    delegate_->DownloadRenamedToFinalName(this);
+  }
 
-void DownloadItemImpl::ReleaseDownloadFile() {
   // Complete the download and release the DownloadFile.
-  DCHECK(!is_save_package_download_);
   DCHECK(download_file_.get());
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DownloadFileDetach, base::Passed(download_file_.Pass()),
-                 base::Bind(&DownloadItemImpl::OnDownloadFileReleased,
-                            weak_ptr_factory_.GetWeakPtr())));
+      base::Bind(&DownloadFileDetach, base::Passed(download_file_.Pass())));
 
   // We're not completely done with the download item yet, but at this
   // point we're committed to complete the download.  Cancels (or Interrupts,
   // though it's not clear how they could happen) after this point will be
   // ignored.
   TransitionTo(COMPLETING_INTERNAL);
-}
 
-void DownloadItemImpl::OnDownloadFileReleased(DownloadInterruptReason reason) {
-  if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
-    Interrupt(reason);
-    return;
-  }
   if (delegate_->ShouldOpenDownload(
           this, base::Bind(&DownloadItemImpl::DelayedDownloadOpened,
                            weak_ptr_factory_.GetWeakPtr()))) {
     Completed();
   } else {
     delegate_delayed_complete_ = true;
   }
 }
 
 void DownloadItemImpl::DelayedDownloadOpened(bool auto_opened) {
@@ skipping 35 matching lines @@
   // Somewhat counter-intuitively, it is possible for us to receive an
   // interrupt after we've already been interrupted.  The generation of
   // interrupts from the file thread Renames and the generation of
   // interrupts from disk writes go through two different mechanisms (driven
   // by rename requests from UI thread and by write requests from IO thread,
   // respectively), and since we choose not to keep state on the File thread,
   // this is the place where the races collide.  It's also possible for
   // interrupts to race with cancels.
 
   // Whatever happens, the first one to hit the UI thread wins.
-  if (state_ != IN_PROGRESS_INTERNAL && state_ != COMPLETING_INTERNAL)
+  if (state_ != IN_PROGRESS_INTERNAL)
     return;
 
   last_reason_ = reason;
   TransitionTo(INTERRUPTED_INTERNAL);
 
   CancelDownloadFile();
 
   // Cancel the originating URL request.
   request_handle_->CancelRequest();
 
@@ skipping 32 matching lines @@
 
   // If the download hasn't been inserted into the history system
   // (which occurs strictly after file name determination, intermediate
   // file rename, and UI display) then it's not ready for completion.
   if (!IsPersisted())
     return false;
 
   return true;
 }
 
-bool DownloadItemImpl::NeedsRename() const {
-  DCHECK(target_path_.DirName() == current_path_.DirName());
-  return target_path_ != current_path_;
-}
-
 void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) {
   if (state_ == new_state)
     return;
 
   DownloadInternalState old_state = state_;
   state_ = new_state;
 
   switch (state_) {
     case COMPLETING_INTERNAL:
       bound_net_log_.AddEvent(
@@ skipping 115 matching lines @@
       return "CANCELLED";
     case INTERRUPTED_INTERNAL:
       return "INTERRUPTED";
     default:
       NOTREACHED() << "Unknown download state " << state;
       return "unknown";
   };
 }
 
 }  // namespace content