Fix an issue that temp files are left permanently on storage after chrome crash

chrome/browser/download/download_history.cc

Index: chrome/browser/download/download_history.cc
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc
index c472e7c112535870f0cacb482dce7ff11cae35af..def9fbc5a73f7bb223da7fbb481af4a8c6116b30 100644
--- a/chrome/browser/download/download_history.cc
+++ b/chrome/browser/download/download_history.cc
@@ -143,25 +143,43 @@ history::DownloadRow GetDownloadRow(
       item->GetOpened(), by_ext_id, by_ext_name);
 }
 
-bool ShouldUpdateHistory(const history::DownloadRow* previous,
-                         const history::DownloadRow& current) {
+enum ShouldUpdateHistoryResult {

[asanka, 2016/11/21 20:17:57]

use 'enum class' instead. Regular enums pollute the namespace and names like
'NO' and 'UPDATE' are likely to collide. 

[qinmin, 2016/11/21 23:14:54]

Done.

+    NO = 0,
+    UPDATE,
+    UPDATE_IMMEDIATELY,
+};
+
+ShouldUpdateHistoryResult ShouldUpdateHistory(
+    const history::DownloadRow* previous,
+    const history::DownloadRow& current) {
+  if (previous == nullptr)
+    return UPDATE;

[asanka, 2016/11/21 20:17:57]

Should return UPDATE_IMMEDIATELY here. If this condition is true, we can't
determine whether the current path was modified and we won't be issuing any more
updates unless something else changes with the download.

[qinmin, 2016/11/21 23:14:54]

Done.

+
+  // When download path is determined, Chrome should commit the history
+  // immediately. Otherwise the file will be left permanently on the external
+  // storage if Chrome crashes right away.
+  if (previous->current_path != current.current_path)
+    return UPDATE_IMMEDIATELY;
+
   // Ignore url_chain, referrer, site_url, http_method, mime_type,
   // original_mime_type, start_time, id, and guid. These fields don't change.
-  return ((previous == NULL) ||
-          (previous->current_path != current.current_path) ||
-          (previous->target_path != current.target_path) ||
-          (previous->end_time != current.end_time) ||
-          (previous->received_bytes != current.received_bytes) ||
-          (previous->total_bytes != current.total_bytes) ||
-          (previous->etag != current.etag) ||
-          (previous->last_modified != current.last_modified) ||
-          (previous->state != current.state) ||
-          (previous->danger_type != current.danger_type) ||
-          (previous->interrupt_reason != current.interrupt_reason) ||
-          (previous->hash != current.hash) ||
-          (previous->opened != current.opened) ||
-          (previous->by_ext_id != current.by_ext_id) ||
-          (previous->by_ext_name != current.by_ext_name));
+  if ((previous->target_path != current.target_path) ||
+      (previous->end_time != current.end_time) ||
+      (previous->received_bytes != current.received_bytes) ||
+      (previous->total_bytes != current.total_bytes) ||
+      (previous->etag != current.etag) ||
+      (previous->last_modified != current.last_modified) ||
+      (previous->state != current.state) ||
+      (previous->danger_type != current.danger_type) ||
+      (previous->interrupt_reason != current.interrupt_reason) ||
+      (previous->hash != current.hash) ||
+      (previous->opened != current.opened) ||
+      (previous->by_ext_id != current.by_ext_id) ||
+      (previous->by_ext_name != current.by_ext_name)) {
+    return UPDATE;
+  }
+
+  return NO;
 }
 
 typedef std::vector<history::DownloadRow> InfoVector;
@@ -186,8 +204,8 @@ void DownloadHistory::HistoryAdapter::CreateDownload(
 }
 
 void DownloadHistory::HistoryAdapter::UpdateDownload(
-    const history::DownloadRow& data) {
-  history_->UpdateDownload(data);
+    const history::DownloadRow& data, bool should_commit_immediately) {
+  history_->UpdateDownload(data, should_commit_immediately);
 }
 
 void DownloadHistory::HistoryAdapter::RemoveDownloads(
@@ -408,11 +426,14 @@ void DownloadHistory::OnDownloadUpdated(
   }
 
   history::DownloadRow current_info(GetDownloadRow(item));
-  bool should_update = ShouldUpdateHistory(data->info(), current_info);
+  ShouldUpdateHistoryResult should_update_result =
+      ShouldUpdateHistory(data->info(), current_info);
+  bool should_update = (should_update_result != NO);
   UMA_HISTOGRAM_ENUMERATION("Download.HistoryPropagatedUpdate",
                             should_update, 2);
   if (should_update) {
-    history_->UpdateDownload(current_info);
+    history_->UpdateDownload(
+        current_info, should_update_result == UPDATE_IMMEDIATELY);
     for (Observer& observer : observers_)
       observer.OnDownloadStored(item, current_info);
   }