[download] Make DownloadItemObserver be owned by DownloadItem.

chrome/browser/download/download_path_reservation_tracker.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.
 
 #include "chrome/browser/download/download_path_reservation_tracker.h"
 
 #include <map>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 31 matching lines @@
 // there are no more reservations.
 //
 // It is not an error, although undesirable, to have multiple DownloadItem*s
 // that are mapped to the same path. This can happen if a reservation is created
 // that is supposed to overwrite an existing reservation.
 ReservationMap* g_reservation_map = NULL;
 
 // Observes a DownloadItem for changes to its target path and state. Updates or
 // revokes associated download path reservations as necessary. Created, invoked
 // and destroyed on the UI thread.
-class DownloadItemObserver : public DownloadItem::Observer {
+class DownloadItemObserver : public DownloadItem::Observer,
+                             public base::SupportsUserData::Data {
  public:
   explicit DownloadItemObserver(DownloadItem* download_item);
 
  private:
   ~DownloadItemObserver() override;
 
   // DownloadItem::Observer
   void OnDownloadUpdated(DownloadItem* download) override;
   void OnDownloadDestroyed(DownloadItem* download) override;
 
   DownloadItem* download_item_;
 
   // Last known target path for the download.
   base::FilePath last_target_path_;
 
+  static const int kUserDataKey;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadItemObserver);
 };
 
 // Returns true if the given path is in use by a path reservation.
 bool IsPathReserved(const base::FilePath& path) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   // No reservation map => no reservations.
   if (g_reservation_map == NULL)
     return false;
   // Unfortunately path normalization doesn't work reliably for non-existant
@@ skipping 77 matching lines @@
     bool create_directory,
     DownloadPathReservationTracker::FilenameConflictAction conflict_action,
     base::FilePath* reserved_path) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   DCHECK(suggested_path.IsAbsolute());
 
   // Create a reservation map if one doesn't exist. It will be automatically
   // deleted when all the reservations are revoked.
   if (g_reservation_map == NULL)
     g_reservation_map = new ReservationMap;
+  ReservationMap& reservations = *g_reservation_map;
 
-  ReservationMap& reservations = *g_reservation_map;
-  DCHECK(!ContainsKey(reservations, key));
+  // Erase the reservation if it already exists. This can happen during
+  // automatic resumption where a new target determination request may be issued
+  // for a DownloadItem without an intervening transition to INTERRUPTED.
+  //
+  // Revoking and re-acquiring the reservation forces us to re-verify the claims
+  // we are making about the path.
+  reservations.erase(key);
 
   base::FilePath target_path(suggested_path.NormalizePathSeparators());
   base::FilePath target_dir = target_path.DirName();
   base::FilePath filename = target_path.BaseName();
   bool is_path_writeable = true;
   bool has_conflicts = false;
   bool name_too_long = false;
 
   // Create target_dir if necessary and appropriate. target_dir may be the last
   // directory that the user selected in a FilePicker; if that directory has
@@ skipping 104 matching lines @@
     bool verified) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   callback.Run(*reserved_path, verified);
 }
 
 DownloadItemObserver::DownloadItemObserver(DownloadItem* download_item)
     : download_item_(download_item),
       last_target_path_(download_item->GetTargetFilePath()) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   download_item_->AddObserver(this);
+  download_item_->SetUserData(&kUserDataKey, this);
 }
 
 DownloadItemObserver::~DownloadItemObserver() {
   download_item_->RemoveObserver(this);
+  // DownloadItemObserver is owned by DownloadItem. It should only be getting
+  // destroyed because it's being removed from the UserData pool. No need to
+  // call DownloadItem::RemoveUserData().
 }
 
 void DownloadItemObserver::OnDownloadUpdated(DownloadItem* download) {
   switch (download->GetState()) {
     case DownloadItem::IN_PROGRESS: {
       // Update the reservation.
       base::FilePath new_target_path = download->GetTargetFilePath();
       if (new_target_path != last_target_path_) {
         BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
             &UpdateReservation, download, new_target_path));
@@ skipping 10 matching lines @@
     case DownloadItem::CANCELLED:
       // We no longer need the reservation if the download is being removed.
 
     case DownloadItem::INTERRUPTED:
       // The download filename will need to be re-generated when the download is
       // restarted. Holding on to the reservation now would prevent the name
       // from being used for a subsequent retry attempt.
 
       BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
           &RevokeReservation, download));
-      delete this;
+      download->RemoveUserData(&kUserDataKey);
       break;
 
     case DownloadItem::MAX_DOWNLOAD_STATE:
       // Compiler appeasement.
       NOTREACHED();
   }
 }
 
 void DownloadItemObserver::OnDownloadDestroyed(DownloadItem* download) {
   // Items should be COMPLETE/INTERRUPTED/CANCELLED before being destroyed.
   NOTREACHED();
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
       &RevokeReservation, download));
-  delete this;
 }
 
+// static
+const int DownloadItemObserver::kUserDataKey = 0;
+
 }  // namespace
 
 // static
 void DownloadPathReservationTracker::GetReservedPath(
     DownloadItem* download_item,
     const base::FilePath& target_path,
     const base::FilePath& default_path,
     bool create_directory,
     FilenameConflictAction conflict_action,
     const ReservedPathCallback& callback) {
@@ skipping 17 matching lines @@
       base::Bind(&RunGetReservedPathCallback,
                  callback,
                  base::Owned(reserved_path)));
 }
 
 // static
 bool DownloadPathReservationTracker::IsPathInUseForTesting(
     const base::FilePath& path) {
   return IsPathInUse(path);
 }