Ask download manager to remove a download if it is overwritten by another

chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.cc

Index: chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.cc
diff --git a/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.cc b/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.cc
index 902dbe00145d0c38ef02ba56e5c3cc79ee1ed97d..c5ed4a503391ecc08c0abc610bfa86d10a0dfffa 100644
--- a/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.cc
+++ b/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.cc
@@ -13,6 +13,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/android/download/download_controller.h"
+#include "chrome/browser/android/download/download_manager_service.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/ui/android/infobars/download_overwrite_infobar.h"
 #include "components/infobars/core/infobar.h"
@@ -21,6 +22,15 @@
 
 namespace {
 
+void DeleteExistingDownloadFileDone(
+    const base::FilePath& download_path,
+    const DownloadTargetDeterminerDelegate::FileSelectedCallback& callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DownloadManagerService::GetInstance()->RemoveExternallyRemovedDownloads(
+      download_path);
+  callback.Run(download_path);
+}
+
 void DeleteExistingDownloadFile(
     const base::FilePath& download_path,
     const DownloadTargetDeterminerDelegate::FileSelectedCallback& callback) {
@@ -29,8 +39,9 @@ void DeleteExistingDownloadFile(
   if (GetFileInfo(download_path, &info) && !info.is_directory)
     base::DeleteFile(download_path, false);
 
-  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-                                   base::Bind(callback, download_path));
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&DeleteExistingDownloadFileDone, download_path, callback));
 }
 
 void CreateNewFileDone(
@@ -89,6 +100,9 @@ ChromeDownloadManagerOverwriteInfoBarDelegate::GetIdentifier() const {
 }
 
 bool ChromeDownloadManagerOverwriteInfoBarDelegate::OverwriteExistingFile() {
+  if (!download_item_)

[Theresa, 2016/09/21 17:40:43]

I'm not very familiar with this class, but looking at the comments in the
header, download_item_ is the item requesting the infobar and it could be
deleted while the infobar is showing.

Previously this code would still delete the existing download file and run the
callback even if download_item_ == nullptr, but now it returns immediately. When
this returns true, will the new file no longer be downloaded?

[qinmin, 2016/09/21 21:20:29]

Yes, if this returns true, the file will no longer be downloaded.
In order to download the file, |file_selected_callback_| has to be called.

The previous code is relies on the weak_ptr in download_target_determiner.cc to
guarantee the safety in case download_item_ is destroyed while the infobar is
shown. The new code just early returns so we don't hit the weak_ptr check in
download_target_determiner.cc


On 2016/09/21 17:40:43, Theresa Wellington wrote:

+    return true;
+
   content::BrowserThread::PostTask(
       content::BrowserThread::FILE, FROM_HERE,
       base::Bind(&DeleteExistingDownloadFile, suggested_download_path_,