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 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;
 
 }  // 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 != 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);
   }
   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);
 }