Detect removed files and reflect the state in chrome://downloads and the download shelf

chrome/browser/download/download_item.h

 // Copyright (c) 2011 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.
 //
 // Each download is represented by a DownloadItem, and all DownloadItems
 // are owned by the DownloadManager which maintains a global list of all
 // downloads. DownloadItems are created when a user initiates a download,
 // and exist for the duration of the browser life time.
 //
 // Download observers:
@@ skipping 138 matching lines @@
   // exit (DownloadManager destructor) from user interface initiated cancels
   // because at exit, the history system may not exist, and any updates to it
   // require AddRef'ing the DownloadManager in the destructor which results in
   // a DCHECK failure. Set 'update_history' to false when canceling from at
   // exit to prevent this crash. This may result in a difference between the
   // downloaded file's size on disk, and what the history system's last record
   // of it is. At worst, we'll end up re-downloading a small portion of the file
   // when resuming a download (assuming the server supports byte ranges).
   void Cancel(bool update_history);
 
-  // Called when all data has been saved.  Only has display effects.
-  void OnAllDataSaved(int64 size);
-
   // Called by external code (SavePackage) using the DownloadItem interface
   // to display progress when the DownloadItem should be considered complete.
   void MarkAsComplete();
 
+  // Called when all data has been saved. Only has display effects.
+  void OnAllDataSaved(int64 size);
+
+  // Called when the downloaded file is removed.
+  void OnDownloadedFileRemoved();
+
   // Download operation had an error.
   // |size| is the amount of data received so far, and |os_error| is the error
   // code that the operation received.
   void Interrupted(int64 size, int os_error);
 
   // Deletes the file from disk and removes the download from the views and
   // history.  |user| should be true if this is the result of the user clicking
   // the discard button, and false if it is being deleted for other reasons like
   // browser shutdown.
   void Delete(DeleteReason reason);
@@ skipping 76 matching lines @@
   int64 received_bytes() const { return received_bytes_; }
   int32 id() const { return id_; }
   base::Time start_time() const { return start_time_; }
   void set_db_handle(int64 handle) { db_handle_ = handle; }
   int64 db_handle() const { return db_handle_; }
   bool is_paused() const { return is_paused_; }
   bool open_when_complete() const { return open_when_complete_; }
   void set_open_when_complete(bool open) { open_when_complete_ = open; }
   int render_process_id() const { return render_process_id_; }
   int request_id() const { return request_id_; }
+  bool file_exists() const { return file_exists_; }
   SafetyState safety_state() const { return safety_state_; }
   void set_safety_state(SafetyState safety_state) {
     safety_state_ = safety_state;
   }
   DangerType danger_type() { return danger_type_;}
   bool auto_opened() { return auto_opened_; }
   FilePath target_name() const { return target_name_; }
   bool save_as() const { return save_as_; }
   bool is_otr() const { return is_otr_; }
   bool is_extension_install() const { return is_extension_install_; }
@@ skipping 86 matching lines @@
 
   // Our owning object
   DownloadManager* download_manager_;
 
   // In progress downloads may be paused by the user, we note it here
   bool is_paused_;
 
   // A flag for indicating if the download should be opened at completion.
   bool open_when_complete_;
 
+  // A flag for indicating if the downloaded file exists.
+  bool file_exists_;

[Randy Smith (Not in Mondays), 2011/05/12 20:21:17]

This flag is currently only valid after the state has transitioned to
completed/the file has been renamed to its final name.  I'd like to either
change the flag name so that it reflects that, or change its behavior so that
it's valid throughout the lifetime of the DownloadItem (commenting that we can
only rely on it after the state transitions to COMPLETE is another option, but
one I'd like to avoid it we can.)

The first option is the easiest; invert the meaning, and change the name to
"file_externally_removed_".  The second involves initializing this flag as
false, and then setting it to true when we rename to the final name.  That's
tricky, as for some downloads (everything but drag and drop) that transition
occurs in OnDownloadRenamedToFinalName, but for drag-and-drop, it has the final
name throughout the download process (and it's a bit tricky to figure that out).
 But if you want to go that route I can dig in and guide you in how to figure it
out.

[Paweł Hajdan Jr., 2011/05/13 08:41:10]

I prefer file_exists for simplicity. file_externally_removed_ may result in many
double-negations in caller code, and the name is just longer.

I think that because of asynchronous operations on the FILE thread there will
always be a period of time when the flag will be invalid, no matter how it's
named.

My general recommendation is then - just make it simple.

[haraken1, 2011/05/13 14:08:17]

I also prefer |file_externally_removed_| in this case. As Randy advised, I
inverted the meaning and changed the name to |file_externally_removed_|.

[Randy Smith (Not in Mondays), 2011/05/13 20:45:32]

Sure, but having a race condition to watch for between threads is different that
the value lying for the entire length of the download processing (once we
complete the download we don't really run much code on it, so we care less about
the value).
haraken: If you want to follow both Pawel's and my preferences, the way to do it
would be to leave the name as file_exists_, initialize it to (target_name_ ==
full_path_.BaseName()), set it to that value again in SetFileCheckResults, and
set it unilaterally to true in DownloadItem::OnDownloadRenamedToFinalName(). 
But I really don't want to leave it as file_exists_ and not have it be valid for
the entire life of the DownloadItem.

+
   // Whether the download is considered potentially safe or dangerous
   // (executable files are typically considered dangerous).
   SafetyState safety_state_;
 
   // Why |safety_state_| is not SAFE.
   DangerType danger_type_;
 
   // Whether the download was auto-opened. We set this rather than using
   // an observer as it's frequently possible for the download to be auto opened
   // before the observer is added.
@@ skipping 26 matching lines @@
   // 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
   // be treated as though the user opened it.
   bool opened_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_H_