Fix crash in DownloadOverwriteInfoBarDelegate.

When the DownloadOverwriteInfoBar is dismissed by user action (e.g.
because a tab was launched only for the download), we shouldn't try and
remove the infobar ourselves. Propagate the value through to ensure we
don't double-free it.
BUG=481758

Review URL: https://codereview.chromium.org/1326033002

Cr-Commit-Position: refs/heads/master@{#347391}

9 files changed, 43 insertions, 29 deletions

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java b/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
index 40f67bd..348ec72 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
@@ -312,14 +312,15 @@ public class ChromeDownloadDelegate
      *
      * @param overwrite Whether or not we will overwrite the file.
      * @param downloadInfo The download info.
+     * @return true iff this request resulted in the tab creating the download to close.
      */
     @CalledByNative
-    private void enqueueDownloadManagerRequestFromNative(
+    private boolean enqueueDownloadManagerRequestFromNative(
             boolean overwrite, DownloadInfo downloadInfo) {
         // Android DownloadManager does not have an overwriting option.
         // We remove the file here instead.
         if (overwrite) deleteFileForOverwrite(downloadInfo);
-        enqueueDownloadManagerRequestInternal(downloadInfo);
+        return enqueueDownloadManagerRequestInternal(downloadInfo);
     }
 
     private void deleteFileForOverwrite(DownloadInfo info) {
@@ -331,10 +332,10 @@ public class ChromeDownloadDelegate
         }
     }
 
-    private void enqueueDownloadManagerRequestInternal(final DownloadInfo info) {
+    private boolean enqueueDownloadManagerRequestInternal(final DownloadInfo info) {
         DownloadManagerService.getDownloadManagerService(
                 mContext.getApplicationContext()).enqueueDownloadManagerRequest(info, true);
-        closeBlankTab();
+        return closeBlankTab();
     }
 
     /**
@@ -478,15 +479,17 @@ public class ChromeDownloadDelegate
 
     /**
      * Close a blank tab just opened for the download purpose.
+     * @return true iff the tab was closed.
      */
-    private void closeBlankTab() {
+    private boolean closeBlankTab() {
         WebContents contents = mTab.getWebContents();
         boolean isInitialNavigation = contents == null
                 || contents.getNavigationController().isInitialNavigation();
         if (isInitialNavigation) {
             // Tab is created just for download, close it.
-            mTabModelSelector.closeTab(mTab);
+            return mTabModelSelector.closeTab(mTab);
         }
+        return false;
     }
 
     /**
diff --git a/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.cc b/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.cc
index 7340024..5bb6bf1 100644
--- a/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.cc
+++ b/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.cc
@@ -53,14 +53,16 @@ AndroidDownloadManagerOverwriteInfoBarDelegate::
   download_info_.Reset(env, download_info);
 }
 
-void AndroidDownloadManagerOverwriteInfoBarDelegate::OverwriteExistingFile() {
-  ChromeDownloadDelegate::EnqueueDownloadManagerRequest(
+bool AndroidDownloadManagerOverwriteInfoBarDelegate::OverwriteExistingFile() {
+  bool tab_closed = ChromeDownloadDelegate::EnqueueDownloadManagerRequest(
       chrome_download_delegate_.obj(), true, download_info_.obj());
+  return !tab_closed;
 }
 
-void AndroidDownloadManagerOverwriteInfoBarDelegate::CreateNewFile() {
-  ChromeDownloadDelegate::EnqueueDownloadManagerRequest(
+bool AndroidDownloadManagerOverwriteInfoBarDelegate::CreateNewFile() {
+  bool tab_closed = ChromeDownloadDelegate::EnqueueDownloadManagerRequest(
       chrome_download_delegate_.obj(), false, download_info_.obj());
+  return !tab_closed;
 }
 
 std::string AndroidDownloadManagerOverwriteInfoBarDelegate::GetFileName()
diff --git a/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.h b/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.h
index c36d4a7..5f38d6c 100644
--- a/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.h
+++ b/chrome/browser/android/download/android_download_manager_overwrite_infobar_delegate.h
@@ -39,8 +39,8 @@ class AndroidDownloadManagerOverwriteInfoBarDelegate
       jobject download_info);
 
   // DownloadOverwriteInfoBarDelegate:
-  void OverwriteExistingFile() override;
-  void CreateNewFile() override;
+  bool OverwriteExistingFile() override;
+  bool CreateNewFile() override;
   std::string GetFileName() const override;
   std::string GetDirName() const override;
   std::string GetDirFullPath() const override;
diff --git a/chrome/browser/android/download/chrome_download_delegate.cc b/chrome/browser/android/download/chrome_download_delegate.cc
index e570d24..0bfa641 100644
--- a/chrome/browser/android/download/chrome_download_delegate.cc
+++ b/chrome/browser/android/download/chrome_download_delegate.cc
@@ -54,13 +54,13 @@ static void DangerousDownloadValidated(JNIEnv* env,
 }
 
 // static
-void ChromeDownloadDelegate::EnqueueDownloadManagerRequest(
+bool ChromeDownloadDelegate::EnqueueDownloadManagerRequest(
     jobject chrome_download_delegate,
     bool overwrite,
     jobject download_info) {
   JNIEnv* env = base::android::AttachCurrentThread();
 
-  Java_ChromeDownloadDelegate_enqueueDownloadManagerRequestFromNative(
+  return Java_ChromeDownloadDelegate_enqueueDownloadManagerRequestFromNative(
       env, chrome_download_delegate, overwrite, download_info);
 }
 
diff --git a/chrome/browser/android/download/chrome_download_delegate.h b/chrome/browser/android/download/chrome_download_delegate.h
index b9bdc91..1ad717c 100644
--- a/chrome/browser/android/download/chrome_download_delegate.h
+++ b/chrome/browser/android/download/chrome_download_delegate.h
@@ -11,7 +11,9 @@
 
 class ChromeDownloadDelegate {
  public:
-  static void EnqueueDownloadManagerRequest(jobject chrome_download_delegate,
+  // Returns true iff this request resulted in the tab creating the download
+  // to close.
+  static bool EnqueueDownloadManagerRequest(jobject chrome_download_delegate,
                                             bool overwrite,
                                             jobject download_info);
 };
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 d4c0cf8..c40a584 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
@@ -41,16 +41,18 @@ ChromeDownloadManagerOverwriteInfoBarDelegate::
       file_selected_callback_(file_selected_callback) {
 }
 
-void ChromeDownloadManagerOverwriteInfoBarDelegate::OverwriteExistingFile() {
+bool ChromeDownloadManagerOverwriteInfoBarDelegate::OverwriteExistingFile() {
   file_selected_callback_.Run(suggested_download_path_);
+  return true;
 }
 
-void ChromeDownloadManagerOverwriteInfoBarDelegate::CreateNewFile() {
+bool ChromeDownloadManagerOverwriteInfoBarDelegate::CreateNewFile() {
   content::BrowserThread::PostTask(
       content::BrowserThread::FILE, FROM_HERE,
       base::Bind(
           &ChromeDownloadManagerOverwriteInfoBarDelegate::CreateNewFileInternal,
           suggested_download_path_, file_selected_callback_));
+  return true;
 }
 
 std::string ChromeDownloadManagerOverwriteInfoBarDelegate::GetFileName() const {
diff --git a/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.h b/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.h
index 30b7f40..1267faa 100644
--- a/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.h
+++ b/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.h
@@ -37,8 +37,8 @@ class ChromeDownloadManagerOverwriteInfoBarDelegate
       const DownloadTargetDeterminerDelegate::FileSelectedCallback& callback);
 
   // DownloadOverwriteInfoBarDelegate:
-  void OverwriteExistingFile() override;
-  void CreateNewFile() override;
+  bool OverwriteExistingFile() override;
+  bool CreateNewFile() override;
   std::string GetFileName() const override;
   std::string GetDirName() const override;
   std::string GetDirFullPath() const override;
diff --git a/chrome/browser/android/download/download_overwrite_infobar_delegate.h b/chrome/browser/android/download/download_overwrite_infobar_delegate.h
index 7d990d9..9afe1fa 100644
--- a/chrome/browser/android/download/download_overwrite_infobar_delegate.h
+++ b/chrome/browser/android/download/download_overwrite_infobar_delegate.h
@@ -31,10 +31,14 @@ namespace android {
 class DownloadOverwriteInfoBarDelegate : public infobars::InfoBarDelegate {
  public:
   // This is called when the user chooses to overwrite the existing file.
-  virtual void OverwriteExistingFile() = 0;
+  // If handling the operation results in dismissing the infobar, returns false
+  // (i.e. the caller must not dismiss the infobar).
+  virtual bool OverwriteExistingFile() = 0;
 
   // This is called when the user chooses to create a new file.
-  virtual void CreateNewFile() = 0;
+  // If handling the operation results in dismissing the infobar, returns false
+  // (i.e. the caller must not dismiss the infobar).
+  virtual bool CreateNewFile() = 0;
 
   // Gets the file name to be downloaded.
   virtual std::string GetFileName() const = 0;
diff --git a/chrome/browser/ui/android/infobars/download_overwrite_infobar.cc b/chrome/browser/ui/android/infobars/download_overwrite_infobar.cc
index ec5f13a..4c58a0e 100644
--- a/chrome/browser/ui/android/infobars/download_overwrite_infobar.cc
+++ b/chrome/browser/ui/android/infobars/download_overwrite_infobar.cc
@@ -50,14 +50,15 @@ void DownloadOverwriteInfoBar::ProcessButton(int action,
     return;  // We're closing; don't call anything, it might access the owner.
 
   DownloadOverwriteInfoBarDelegate* delegate = GetDelegate();
-  if (action == InfoBarAndroid::ACTION_OVERWRITE)
-    delegate->OverwriteExistingFile();
-  else if (action == InfoBarAndroid::ACTION_CREATE_NEW_FILE)
-    delegate->CreateNewFile();
-  else
-    DCHECK(false);
-
-  RemoveSelf();
+  if (action == InfoBarAndroid::ACTION_OVERWRITE) {
+    if (delegate->OverwriteExistingFile())
+      RemoveSelf();
+  } else if (action == InfoBarAndroid::ACTION_CREATE_NEW_FILE) {
+    if (delegate->CreateNewFile())
+      RemoveSelf();
+  } else {
+    CHECK(false);
+  }
 }
 
 DownloadOverwriteInfoBarDelegate* DownloadOverwriteInfoBar::GetDelegate() {