--downloads-new-ui completely disables the download shelf.

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 2de9e928ffd4f6f89a58d95c598c756628db7124..594430a8ad9a41117a9b493ac6a29e570bf550dc 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -3361,6 +3361,13 @@ int Browser::GetExtraRenderViewHeight() const {
   return window_->GetExtraRenderViewHeight();
 }
 
+namespace {
+bool DisplayOldDownloadsUI() {

[cbentzel, 2011/08/15 18:28:26]

Nit: extra newlines here.

[benjhayden, 2011/08/15 19:15:36]

Done.

+  return !CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kDownloadsNewUI);
+}
+}  // anonymous namespace
+
 void Browser::OnStartDownload(TabContents* source, DownloadItem* download) {
   TabContentsWrapper* wrapper =
       TabContentsWrapper::GetCurrentWrapperForContents(source);
@@ -3375,41 +3382,42 @@ void Browser::OnStartDownload(TabContents* source, DownloadItem* download) {
   if (!window())
     return;
 
+  if (DisplayOldDownloadsUI()) {
 #if defined(OS_CHROMEOS)
-  // Don't show content browser for extension/theme downloads from gallery.
-  if (download->is_extension_install()) {
+    // Don't show content browser for extension/theme downloads from gallery.
     ExtensionService* service = profile_->GetExtensionService();
-    if (service && service->IsDownloadFromGallery(download->GetURL(),
-                                                  download->referrer_url())) {
-      return;
+    if (!download->is_extension_install() ||
+        (service == NULL) ||
+        !service->IsDownloadFromGallery(download->GetURL(),
+                                        download->referrer_url())) {
+      // Open the Active Downloads ui for chromeos.
+      ActiveDownloadsUI::OpenPopup(profile_);
     }
-  }
-  // Open the Active Downloads ui for chromeos.
-  ActiveDownloadsUI::OpenPopup(profile_);
 #else
-  // GetDownloadShelf creates the download shelf if it was not yet created.
-  DownloadShelf* shelf = window()->GetDownloadShelf();
-  shelf->AddDownload(new DownloadItemModel(download));
-
-  // Don't show the animation for "Save file" downloads.
-  if (download->total_bytes() <= 0)
-    return;
-
-  // For non-theme extensions, we don't show the download animation.
-  if (download->is_extension_install() &&
-      !ExtensionService::IsDownloadFromMiniGallery(download->GetURL()))
-    return;
-
-  // Show animation in same window as the download shelf. Download shelf
-  // may not be in the same window that initiated the download, e.g. Panels.
-  TabContents* shelf_tab = shelf->browser()->GetSelectedTabContents();
-
-  // We make this check for the case of minimized windows, unit tests, etc.
-  if (platform_util::IsVisible(shelf_tab->GetNativeView()) &&
-      ui::Animation::ShouldRenderRichAnimation()) {
-    DownloadStartedAnimation::Show(shelf_tab);
-  }
+    // GetDownloadShelf creates the download shelf if it was not yet created.

[cbentzel, 2011/08/15 18:28:26]

I haven't looked at what happens if there isn't a DownloadItemModel/shelf. Do
downloads still proceed? Do they expect a DownloadItem?

[benjhayden, 2011/08/15 19:15:36]

Downloads still proceed. I haven't run all the tests with --downloads-new-ui,
but I ran ./out/Debug/chrome, turned the flag on, downloaded a safe file and a
dangerous file, and both showed up in chrome://downloads correctly, with the
danger confirmation dialog.

+    DownloadShelf* shelf = window()->GetDownloadShelf();
+    shelf->AddDownload(new DownloadItemModel(download));
+
+    // Don't show the animation for "Save file" downloads.
+    if (download->total_bytes() > 0) {

[Peter Kasting, 2011/08/15 18:34:02]

Nit: If you're going to change the early-return style of the old code, collapse
these conditionals together (like you did above).

[benjhayden, 2011/08/15 19:15:36]

Unfortunately, I can't merge these conditionals without significantly changing
functionality. There are significant differences between
ExtensionService::IsDownloadFrom{,Mini}Gallery(). The only commonality is
download->is_extension_install().

I added a TODO to talk to the original authors about merging them, but the new
UI will be a cross-platform extension, so I wonder how much we should worry
about merging these branches.

[Peter Kasting, 2011/08/15 19:19:13]

You misunderstand me.  I don't mean to merge the ChromeOS and non-ChromeOS
cases.  I mean merge the successive conditionals right here:

if (...) {
  if (...) {
    ...
    if (...) {
      ...

->

...
if ((...) && (...) && (...)) {
  ...

[benjhayden, 2011/08/15 19:51:42]

Done.

+      // For non-theme extensions, we don't show the download animation.
+      if (!download->is_extension_install() ||
+          ExtensionService::IsDownloadFromMiniGallery(download->GetURL())) {
+        // Show animation in same window as the download shelf. Download shelf
+        // may not be in the same window that initiated the download, e.g.
+        // Panels.
+        TabContents* shelf_tab = shelf->browser()->GetSelectedTabContents();
+
+        // We make this check for the case of minimized windows, unit tests,
+        // etc.
+        if (platform_util::IsVisible(shelf_tab->GetNativeView()) &&
+            ui::Animation::ShouldRenderRichAnimation()) {
+          DownloadStartedAnimation::Show(shelf_tab);
+        }
+      }
+    }
 #endif
+  }
 
   // If the download occurs in a new tab, close it.
   if (source->controller().IsInitialNavigation() && tab_count() > 1)