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

chrome/browser/download/download_item.h

Index: chrome/browser/download/download_item.h
diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h
index aafab618660ffba44e9cfa8be88381a38501bcf3..c4b9b492954741dee7e1ebea64c99189cdf8fb65 100644
--- a/chrome/browser/download/download_item.h
+++ b/chrome/browser/download/download_item.h
@@ -156,13 +156,16 @@ class DownloadItem {
   // 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.
@@ -259,6 +262,7 @@ class DownloadItem {
   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;
@@ -365,6 +369,9 @@ class DownloadItem {
   // 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_;