Make the UI an observer of downloads.

chrome/browser/ui/browser.cc

Index: chrome/browser/ui/browser.cc
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index f1d1d7dcd4ee502a62019b6aeec6220704ccb12e..772672f8a075b8e095e3a4f4d4390bca8b834add 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -1334,6 +1334,41 @@ void Browser::MaybeUpdateBookmarkBarStateForInstantPreview(
   }
 }
 
+void Browser::ShowDownload(content::DownloadItem* download) {

[benjhayden, 2012/12/20 21:09:14]

Can this method be moved out to the Observer? It looks like it doesn't use
anything in the Browser except window(), which is public, and CloseContents,
which could be accessed by up-casting the Browser*. If you don't want to up-cast
it, could you move the rest of this method out into the Observer?

[asanka, 2012/12/20 22:41:19]

The Browser class:
- Handles closing the tab that initiated the download if that was the only thing
that the tab did.
- Handles showing the download in the shelf.
- Handles displaying the "download started" animation.

The DownloadUIController class:
- Figures out which Browser to notify and notifies it.

I think this is a reasonable layering scheme. I don't think DownloadUIController
should reach into a Browser object and close its tabs or insert downloads into
the shelf or trigger animations, even if its possible with the API.

+  scoped_ptr<DownloadItemModel> download_model(new DownloadItemModel(download));
+  if (!window())
+    return;
+
+  // If the download occurs in a new tab, and it's not a save page
+  // download (started before initial navigation completed) close it.
+  WebContents* source = download->GetWebContents();
+  if (source && source->GetController().IsInitialNavigation() &&
+      tab_count() > 1 && !download->IsSavePackageDownload()) {
+    CloseContents(source);
+  }
+
+  // Some (app downloads) are not supposed to appear on the shelf.
+  if (!download_model->ShouldShowInShelf())
+    return;
+
+  // GetDownloadShelf creates the download shelf if it was not yet created.
+  DownloadShelf* shelf = window()->GetDownloadShelf();
+  shelf->AddDownload(download_model.release());
+  // Don't show the animation for "Save file" downloads.
+  // For non-theme extensions, we don't show the download animation.
+  // Show animation in same window as the download shelf. Download shelf
+  // may not be in the same window that initiated the download.
+  // Don't show the animation if the selected tab is not visible (i.e. the
+  // window is minimized, we're in a unit test, etc.).
+  WebContents* shelf_tab = chrome::GetActiveWebContents(shelf->browser());
+  if ((download->GetTotalBytes() > 0) &&
+      !download_crx_util::IsExtensionDownload(*download) &&
+      platform_util::IsVisible(shelf_tab->GetNativeView()) &&
+      ui::Animation::ShouldRenderRichAnimation()) {
+    DownloadStartedAnimation::Show(shelf_tab);
+  }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, content::WebContentsDelegate implementation:
 
@@ -1495,46 +1530,6 @@ int Browser::GetExtraRenderViewHeight() const {
   return window_->GetExtraRenderViewHeight();
 }
 
-void Browser::OnStartDownload(WebContents* source,
-                              content::DownloadItem* download) {
-  scoped_ptr<DownloadItemModel> download_model(new DownloadItemModel(download));
-  if (!download_model->ShouldShowInShelf())
-    return;
-
-  WebContents* constrained = GetConstrainingWebContents(source);
-  if (constrained != source) {
-    // Download in a constrained popup is shown in the tab that opened it.
-    constrained->GetDelegate()->OnStartDownload(constrained, download);
-    return;
-  }
-
-  if (!window())
-    return;
-
-  // GetDownloadShelf creates the download shelf if it was not yet created.
-  DownloadShelf* shelf = window()->GetDownloadShelf();
-  shelf->AddDownload(download_model.release());
-  // Don't show the animation for "Save file" downloads.
-  // For non-theme extensions, we don't show the download animation.
-  // Show animation in same window as the download shelf. Download shelf
-  // may not be in the same window that initiated the download.
-  // Don't show the animation if the selected tab is not visible (i.e. the
-  // window is minimized, we're in a unit test, etc.).
-  WebContents* shelf_tab = chrome::GetActiveWebContents(shelf->browser());
-  if ((download->GetTotalBytes() > 0) &&
-      !download_crx_util::IsExtensionDownload(*download) &&
-      platform_util::IsVisible(shelf_tab->GetNativeView()) &&
-      ui::Animation::ShouldRenderRichAnimation()) {
-    DownloadStartedAnimation::Show(shelf_tab);
-  }
-
-  // If the download occurs in a new tab, and it's not a save page
-  // download (started before initial navigation completed) close it.
-  if (source->GetController().IsInitialNavigation() && tab_count() > 1 &&
-      !download->IsSavePackageDownload())
-    CloseContents(source);
-}
-
 void Browser::ViewSourceForTab(WebContents* source, const GURL& page_url) {
   DCHECK(source);
   chrome::ViewSource(this, source);