implementation for new duplicate download UI

chrome/browser/download/chrome_download_manager_delegate.cc

Index: chrome/browser/download/chrome_download_manager_delegate.cc
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index c151f3e44f63f0d7c13715d52176b9115e634039..c514a0f95f6354f50d3a69ca296401d08d1b960d 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -57,7 +57,7 @@
 #include "ui/base/l10n/l10n_util.h"
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
-#include "chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.h"
+#include "chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #endif
 
@@ -617,9 +617,11 @@ void ChromeDownloadManagerDelegate::PromptUserForDownloadPath(
     const DownloadTargetDeterminerDelegate::FileSelectedCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 #if BUILDFLAG(ANDROID_JAVA_UI)
-  chrome::android::ChromeDownloadManagerOverwriteInfoBarDelegate::Create(
-      InfoBarService::FromWebContents(download->GetWebContents()), download,
-      suggested_path, callback);
+  content::WebContents* web_contents = download->GetWebContents();

[asanka, 2016/11/10 20:49:39]

It's possible for the WebContents to be null here. E.g. a resumption attempt
encounters a filename conflict.

I see occasional crashes from the previous code path which also doesn't check if
the WebContents is null. But it's hard to tell whether those crashes were caused
by a missing WebContents.

[qinmin, 2016/11/10 21:37:08]

null WebContents caused by resumption attempt with a filename conflict is
handled in DownloadTargetDeterminer::ReserveVirtualPathDone(), instead of here. 

Anyway, did a null check here and call callback.Run(base::FilePath()) if
webcontents if it happens.

[asanka, 2016/11/10 21:57:13]

Sure. But ReserveVirtualPathDone() isn't where we initiate prompting. Even if
checking there works now, this function doesn't have any basis for assuming a
WebContents will still be there when it's called. Chances are eventually we'll
trip over these kinds of assumptions.

Either way, we are going to clean up some of these and add tests over at
https://codereview.chromium.org/2453633006 which also adds better signalling for
the type of error that caused the prompt.
Thanks. That hardcodes "Cancel" as the default action whenever a prompt can't be
displayed to the user. That's fine since that's what
DownloadTargetDeterminer::ReserveVirtualPathDone() currently does when there's
no WebContents.

+  chrome::android::ChromeDuplicateDownloadInfoBarDelegate::Create(
+      InfoBarService::FromWebContents(web_contents), download,
+      suggested_path, web_contents->GetBrowserContext()->IsOffTheRecord(),

[asanka, 2016/11/10 20:49:39]

Nit: IsOffTheRecord() is redundant. ChromeDuplicateDownloadInfoBarDelegate can
determine whether it's dealing with an off-the-record profile via the |download|
parameter and also via the |InfoBarService| parameter.

[qinmin, 2016/11/10 21:37:08]

Done.

+      callback);
 #else
   DownloadFilePicker::ShowFilePicker(download, suggested_path, callback);
 #endif