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

chrome/browser/download/download_history.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.
 
 // DownloadHistory manages persisting DownloadItems to the history service by
 // observing a single DownloadManager and all its DownloadItems using an
 // AllDownloadItemNotifier.
 //
 // DownloadHistory decides whether and when to add items to, remove items from,
 // and update items in the database. DownloadHistory uses DownloadHistoryData to
@@ skipping 125 matching lines @@
       item->GetEndTime(), item->GetETag(), item->GetLastModifiedTime(),
       item->GetReceivedBytes(), item->GetTotalBytes(),
       history::ToHistoryDownloadState(item->GetState()),
       history::ToHistoryDownloadDangerType(item->GetDangerType()),
       history::ToHistoryDownloadInterruptReason(item->GetLastReason()),
       std::string(),  // Hash value (not available yet)
       history::ToHistoryDownloadId(item->GetId()), item->GetGuid(),
       item->GetOpened(), by_ext_id, by_ext_name);
 }
 
-bool ShouldUpdateHistory(const history::DownloadRow* previous,
-                         const history::DownloadRow& current) {
+enum class ShouldUpdateHistoryResult {
+    NO,
+    UPDATE,
+    UPDATE_IMMEDIATELY,
+};
+
+ShouldUpdateHistoryResult ShouldUpdateHistory(
+    const history::DownloadRow* previous,
+    const history::DownloadRow& current) {
+  // 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.
+  // TODO(qinmin): this doesn't solve all the issues. When download starts,
+  // Chrome will write the http response data to a temporary file, and later
+  // rename it. If Chrome is killed before committing the history here,
+  // that temporary file will still get permanently left.
+  // See http://crbug.com/664677.
+  if (previous == nullptr || previous->current_path != current.current_path)
+    return ShouldUpdateHistoryResult::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 ShouldUpdateHistoryResult::UPDATE;
+  }
+
+  return ShouldUpdateHistoryResult::NO;
 }
 
 typedef std::vector<history::DownloadRow> InfoVector;
 
 }  // anonymous namespace
 
 DownloadHistory::HistoryAdapter::HistoryAdapter(
     history::HistoryService* history)
     : history_(history) {
 }
 DownloadHistory::HistoryAdapter::~HistoryAdapter() {}
 
 void DownloadHistory::HistoryAdapter::QueryDownloads(
     const history::HistoryService::DownloadQueryCallback& callback) {
   history_->QueryDownloads(callback);
 }
 
 void DownloadHistory::HistoryAdapter::CreateDownload(
     const history::DownloadRow& info,
     const history::HistoryService::DownloadCreateCallback& callback) {
   history_->CreateDownload(info, callback);
 }
 
 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(
     const std::set<uint32_t>& ids) {
   history_->RemoveDownloads(ids);
 }
 
 DownloadHistory::Observer::Observer() {}
 DownloadHistory::Observer::~Observer() {}
 
@@ skipping 200 matching lines @@
   if (data->state() == DownloadHistoryData::NOT_PERSISTED) {
     MaybeAddToHistory(item);
     return;
   }
   if (item->IsTemporary()) {
     OnDownloadRemoved(notifier_.GetManager(), item);
     return;
   }
 
   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 != ShouldUpdateHistoryResult::NO);
   UMA_HISTOGRAM_ENUMERATION("Download.HistoryPropagatedUpdate",
                             should_update, 2);
   if (should_update) {
-    history_->UpdateDownload(current_info);
+    history_->UpdateDownload(
+        current_info,
+        should_update_result == ShouldUpdateHistoryResult::UPDATE_IMMEDIATELY);
     for (Observer& observer : observers_)
       observer.OnDownloadStored(item, current_info);
   }
   if (item->GetState() == content::DownloadItem::IN_PROGRESS) {
     data->set_info(current_info);
   } else {
     data->clear_info();
   }
 }
 
@@ skipping 38 matching lines @@
 }
 
 void DownloadHistory::RemoveDownloadsBatch() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   IdSet remove_ids;
   removing_ids_.swap(remove_ids);
   history_->RemoveDownloads(remove_ids);
   for (Observer& observer : observers_)
     observer.OnDownloadsRemoved(remove_ids);
 }