Move closing-blank-tab logic in Browser class to DownloadUIConttroller

chrome/browser/download/download_ui_controller.cc

Index: chrome/browser/download/download_ui_controller.cc
diff --git a/chrome/browser/download/download_ui_controller.cc b/chrome/browser/download/download_ui_controller.cc
index f41d2b3d01256d8904f1684137940f08ab8a2e72..60219db0593145d27a3b72c5b42b209303fb88b6 100644
--- a/chrome/browser/download/download_ui_controller.cc
+++ b/chrome/browser/download/download_ui_controller.cc
@@ -8,8 +8,11 @@
 #include "base/command_line.h"
 #include "base/stl_util.h"
 #include "chrome/browser/download/download_item_model.h"
+#include "chrome/browser/download/download_shelf.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/web_contents.h"
@@ -85,8 +88,12 @@ void DownloadShelfUIControllerDelegate::OnNewDownloadReady(
                                                 chrome::GetActiveDesktop());
   }
 
-  if (browser)
-    browser->ShowDownload(item);
+  if (browser && browser->window() &&
+      // Some (app downloads) are not supposed to appear on the shelf.

[msw, 2015/07/17 01:15:34]

nit: please move this comment above the start of the conditional
(you can remove either of these comments, neither adds much value)

[yoshiki, 2015/07/17 01:40:04]

Done. Removed the first comment.

+      DownloadItemModel(item).ShouldShowInShelf()) {
+    // GetDownloadShelf creates the download shelf if it was not yet created.
+    browser->window()->GetDownloadShelf()->AddDownload(item);
+  }
 }
 
 #endif  // !OS_ANDROID
@@ -143,5 +150,25 @@ void DownloadUIController::OnDownloadUpdated(content::DownloadManager* manager,
     return;
 
   DownloadItemModel(item).SetWasUINotified(true);
+
+#if !defined(OS_ANDROID)
+  content::WebContents* web_contents = item->GetWebContents();
+  if (web_contents) {
+    Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+    // If the download occurs in a new tab, and it's not a save page
+    // download (started before initial navigation completed) close it.
+    // Avoid calling CloseContents if the tab is not in this browser's tab strip
+    // model; this can happen if the download was initiated by something
+    // internal to Chrome, such as by the app list.
+    if (browser && web_contents->GetController().IsInitialNavigation() &&
+        browser->tab_strip_model()->count() > 1 &&
+        browser->tab_strip_model()->GetIndexOfWebContents(web_contents) !=
+            TabStripModel::kNoTab &&
+        !item->IsSavePackageDownload()) {
+      web_contents->Close();

[msw, 2015/07/17 01:15:34]

Will changing Browser::CloseContents to WebContents::Close have any impact?
(I'd expect that there isn't any meaningful difference, but thought I'd ask)

[yoshiki, 2015/07/17 01:40:04]

WebContents::Close calls Browser::CloseContents internally. The only difference
is a RenderViewHost check
(https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/we...).
Hence I think there is no diffirence. 

[msw, 2015/07/17 02:11:33]

Acknowledged.

+    }
+  }
+#endif
+
   delegate_->OnNewDownloadReady(item);
 }