Delay transient downloads a few seconds before displaying.

Downloads that are temporary or will be opened automatically will be
removed from the shelf when they complete. If these are quick
downloads, the user will not be able to interact with them on the
shelf. Delay showing these on the shelf by a few seconds to avoid
showing quick downloads on the shelf.

BUG=164710


Review URL: https://chromiumcodereview.appspot.com/11689003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176018 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 456 insertions, 82 deletions

diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc
index d6242b1..9b4013e 100644
--- a/chrome/browser/download/download_item_model.cc
+++ b/chrome/browser/download/download_item_model.cc
@@ -369,6 +369,35 @@ bool DownloadItemModel::IsMalicious() const {
   return false;
 }
 
+bool DownloadItemModel::ShouldRemoveFromShelfWhenComplete() const {
+  // If the download was already opened automatically, it should be removed.
+  if (download_->GetAutoOpened())
+    return true;
+
+  // If the download is interrupted or cancelled, it should not be removed.
+  if (download_->IsInterrupted() || download_->IsCancelled())
+    return false;
+
+  // If the download is dangerous or malicious, we should display a warning on
+  // the shelf until the user accepts the download.
+  if (IsDangerous())
+    return false;
+
+  // If the download is an extension, temporary, or will be opened
+  // automatically, then it should be removed from the shelf on completion.
+  // TODO(asanka): The logic for deciding opening behavior should be in a
+  //               central location. http://crbug.com/167702
+  return (download_crx_util::IsExtensionDownload(*download_) ||
+          download_->IsTemporary() ||
+          download_->GetOpenWhenComplete() ||
+          download_->ShouldOpenFileBasedOnExtension());
+}
+
+bool DownloadItemModel::ShouldShowDownloadStartedAnimation() const {
+  return !download_->IsSavePackageDownload() &&
+      !download_crx_util::IsExtensionDownload(*download_);
+}
+
 bool DownloadItemModel::ShouldShowInShelf() const {
   const DownloadItemModelData* data = DownloadItemModelData::Get(download_);
   return !data || data->should_show_in_shelf();
diff --git a/chrome/browser/download/download_item_model.h b/chrome/browser/download/download_item_model.h
index 6ac4f59..7b65fe5 100644
--- a/chrome/browser/download/download_item_model.h
+++ b/chrome/browser/download/download_item_model.h
@@ -79,6 +79,24 @@ class DownloadItemModel {
   // Is this considered a malicious download? Implies IsDangerous().
   bool IsMalicious() const;
 
+  // Returns |true| if this download is expected to complete successfully and
+  // thereafter be removed from the shelf.  Downloads that are opened
+  // automatically or are temporary will be removed from the shelf on successful
+  // completion.
+  //
+  // Returns |false| if the download is not expected to complete (interrupted,
+  // cancelled, dangerous, malicious), or won't be removed on completion.
+  //
+  // Since the expectation of successful completion may change, the return value
+  // of this function will change over the course of a download.
+  bool ShouldRemoveFromShelfWhenComplete() const;
+
+  // Returns |true| if the download started animation (big download arrow
+  // animates down towards the shelf) should be displayed for this download.
+  // Downloads that were initiated via "Save As" or are extension installs don't
+  // show the animation.
+  bool ShouldShowDownloadStartedAnimation() const;
+
   // Returns |true| if this download should be displayed in the downloads shelf.
   bool ShouldShowInShelf() const;
 
diff --git a/chrome/browser/download/download_item_model_unittest.cc b/chrome/browser/download/download_item_model_unittest.cc
index 123fe72..fdd971a 100644
--- a/chrome/browser/download/download_item_model_unittest.cc
+++ b/chrome/browser/download/download_item_model_unittest.cc
@@ -20,6 +20,7 @@
 #include "ui/base/text/bytes_formatting.h"
 #include "ui/gfx/font.h"
 
+using content::DownloadItem;
 using ::testing::Mock;
 using ::testing::NiceMock;
 using ::testing::Return;
@@ -72,7 +73,7 @@ class DownloadItemModelTest : public testing::Test {
     ON_CALL(item_, GetOpenWhenComplete()).WillByDefault(Return(false));
     ON_CALL(item_, GetFileExternallyRemoved()).WillByDefault(Return(false));
     ON_CALL(item_, GetState())
-        .WillByDefault(Return(content::DownloadItem::IN_PROGRESS));
+        .WillByDefault(Return(DownloadItem::IN_PROGRESS));
     ON_CALL(item_, GetURL())
         .WillByDefault(ReturnRefOfCopy(GURL(kDefaultURL)));
     ON_CALL(item_, GetFileNameToReportUser())
@@ -81,7 +82,7 @@ class DownloadItemModelTest : public testing::Test {
         .WillByDefault(ReturnRefOfCopy(FilePath(kDefaultTargetFilePath)));
     ON_CALL(item_, GetTargetDisposition())
         .WillByDefault(
-            Return(content::DownloadItem::TARGET_DISPOSITION_OVERWRITE));
+            Return(DownloadItem::TARGET_DISPOSITION_OVERWRITE));
     ON_CALL(item_, IsPaused()).WillByDefault(Return(false));
   }
 
@@ -90,8 +91,8 @@ class DownloadItemModelTest : public testing::Test {
     EXPECT_CALL(item_, GetState())
         .WillRepeatedly(Return(
             (reason == content::DOWNLOAD_INTERRUPT_REASON_NONE) ?
-                content::DownloadItem::IN_PROGRESS :
-                content::DownloadItem::INTERRUPTED));
+                DownloadItem::IN_PROGRESS :
+                DownloadItem::INTERRUPTED));
     EXPECT_CALL(item_, IsInProgress())
         .WillRepeatedly(Return(
             reason == content::DOWNLOAD_INTERRUPT_REASON_NONE));
@@ -269,7 +270,7 @@ TEST_F(DownloadItemModelTest, InterruptTooltip) {
 }
 
 TEST_F(DownloadItemModelTest, InProgressStatus) {
-  struct TestCase {
+  const struct TestCase {
     int64 received_bytes;               // Return value of GetReceivedBytes().
     int64 total_bytes;                  // Return value of GetTotalBytes().
     bool  time_remaining_known;         // If TimeRemaining() is known.
@@ -319,7 +320,7 @@ TEST_F(DownloadItemModelTest, InProgressStatus) {
   SetupDownloadItemDefaults();
 
   for (unsigned i = 0; i < ARRAYSIZE_UNSAFE(kTestCases); i++) {
-    TestCase& test_case = kTestCases[i];
+    const TestCase& test_case = kTestCases[i];
     Mock::VerifyAndClearExpectations(&item());
     Mock::VerifyAndClearExpectations(&model());
     EXPECT_CALL(item(), GetReceivedBytes())
@@ -353,3 +354,59 @@ TEST_F(DownloadItemModelTest, ShouldShowInShelf) {
   model().SetShouldShowInShelf(true);
   EXPECT_TRUE(model().ShouldShowInShelf());
 }
+
+TEST_F(DownloadItemModelTest, ShouldRemoveFromShelfWhenComplete) {
+  const struct TestCase {
+    DownloadItem::DownloadState state;
+    bool is_dangerous;
+    bool is_auto_open;  // Either an extension install, temporary, open when
+                        // complete or open based on extension.
+    bool expected_result;
+  } kTestCases[] = {
+    // All the valid combinations of state, is_dangerous and is_auto_open.
+    //
+    //                              .--- Is dangerous.
+    //                             |       .--- Auto open or temporary.
+    //                             |      |      .--- Expected result.
+    { DownloadItem::IN_PROGRESS, false, false, false },
+    { DownloadItem::IN_PROGRESS, false, true , true  },
+    { DownloadItem::IN_PROGRESS, true , false, false },
+    { DownloadItem::IN_PROGRESS, true , true , false },
+    { DownloadItem::COMPLETE,    false, false, false },
+    { DownloadItem::COMPLETE,    false, true , true  },
+    { DownloadItem::CANCELLED,   false, false, false },
+    { DownloadItem::CANCELLED,   false, true , false },
+    { DownloadItem::CANCELLED,   true , false, false },
+    { DownloadItem::CANCELLED,   true , true , false },
+    { DownloadItem::INTERRUPTED, false, false, false },
+    { DownloadItem::INTERRUPTED, false, true , false },
+    { DownloadItem::INTERRUPTED, true , false, false },
+    { DownloadItem::INTERRUPTED, true , true , false }
+  };
+
+  SetupDownloadItemDefaults();
+
+  for (unsigned i = 0; i < ARRAYSIZE_UNSAFE(kTestCases); i++) {
+    const TestCase& test_case = kTestCases[i];
+    EXPECT_CALL(item(), GetOpenWhenComplete())
+        .WillRepeatedly(Return(test_case.is_auto_open));
+    EXPECT_CALL(item(), GetState())
+        .WillRepeatedly(Return(test_case.state));
+    EXPECT_CALL(item(), IsCancelled())
+        .WillRepeatedly(Return(test_case.state == DownloadItem::CANCELLED));
+    EXPECT_CALL(item(), IsComplete())
+        .WillRepeatedly(Return(test_case.state == DownloadItem::COMPLETE));
+    EXPECT_CALL(item(), IsInProgress())
+        .WillRepeatedly(Return(test_case.state == DownloadItem::IN_PROGRESS));
+    EXPECT_CALL(item(), IsInterrupted())
+        .WillRepeatedly(Return(test_case.state == DownloadItem::INTERRUPTED));
+    EXPECT_CALL(item(), GetSafetyState())
+        .WillRepeatedly(Return(test_case.is_dangerous ? DownloadItem::DANGEROUS
+                                                      : DownloadItem::SAFE));
+
+    EXPECT_EQ(test_case.expected_result,
+              model().ShouldRemoveFromShelfWhenComplete());
+    Mock::VerifyAndClearExpectations(&item());
+    Mock::VerifyAndClearExpectations(&model());
+  }
+}
diff --git a/chrome/browser/download/download_shelf.cc b/chrome/browser/download/download_shelf.cc
index 524ff7d..be24469 100644
--- a/chrome/browser/download/download_shelf.cc
+++ b/chrome/browser/download/download_shelf.cc
@@ -4,15 +4,54 @@
 
 #include "chrome/browser/download/download_shelf.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/message_loop.h"
+#include "chrome/browser/download/download_item_model.h"
+#include "chrome/browser/download/download_started_animation.h"
+#include "chrome/browser/platform_util.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/download_item.h"
+#include "content/public/browser/download_manager.h"
+#include "content/public/browser/web_contents.h"
+#include "ui/base/animation/animation.h"
+
+using content::DownloadItem;
+
+namespace {
+
+// Delay before we show a transient download.
+const int64 kDownloadShowDelayInSeconds = 2;
+
+} // namespace
+
 DownloadShelf::DownloadShelf()
     : should_show_on_unhide_(false),
-      is_hidden_(false) {}
+      is_hidden_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+}
 
-void DownloadShelf::AddDownload(content::DownloadItem* download) {
-  if (is_hidden_)
-    Unhide();
-  Show();
-  DoAddDownload(download);
+DownloadShelf::~DownloadShelf() {
+}
+
+void DownloadShelf::AddDownload(DownloadItem* download) {
+  DCHECK(download);
+  if (DownloadItemModel(download).ShouldRemoveFromShelfWhenComplete()) {
+    // If we are going to remove the download from the shelf upon completion,
+    // wait a few seconds to see if it completes quickly. If it's a small
+    // download, then the user won't have time to interact with it.
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&DownloadShelf::ShowDownloadById,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   download->GetId()),
+        GetTransientDownloadShowDelay());
+  } else {
+    ShowDownload(download);
+  }
 }
 
 void DownloadShelf::Show() {
@@ -50,3 +89,54 @@ void DownloadShelf::Unhide() {
     DoShow();
   }
 }
+
+base::TimeDelta DownloadShelf::GetTransientDownloadShowDelay() {
+  return base::TimeDelta::FromSeconds(kDownloadShowDelayInSeconds);
+}
+
+content::DownloadManager* DownloadShelf::GetDownloadManager() {
+  DCHECK(browser());
+  return content::BrowserContext::GetDownloadManager(browser()->profile());
+}
+
+void DownloadShelf::ShowDownload(DownloadItem* download) {
+  if (download->IsComplete() &&
+      DownloadItemModel(download).ShouldRemoveFromShelfWhenComplete()) {
+    return;
+  }
+
+  if (is_hidden_)
+    Unhide();
+  Show();
+  DoAddDownload(download);
+
+  // browser() can be NULL for tests.
+  if (!browser())
+    return;
+
+  // Show the download started animation if:
+  // - Download started animation is enabled for this download. It is disabled
+  //   for "Save As" downloads and extension installs, for example.
+  // - The browser has an active visble WebContents. (browser isn't minimized,
+  //   or running under a test etc.)
+  // - Rich animations are enabled.
+  content::WebContents* shelf_tab = chrome::GetActiveWebContents(browser());
+  if (DownloadItemModel(download).ShouldShowDownloadStartedAnimation() &&
+      shelf_tab &&
+      platform_util::IsVisible(shelf_tab->GetNativeView()) &&
+      ui::Animation::ShouldRenderRichAnimation()) {
+    DownloadStartedAnimation::Show(shelf_tab);
+  }
+}
+
+void DownloadShelf::ShowDownloadById(int32 download_id) {
+  content::DownloadManager* download_manager = GetDownloadManager();
+  if (!download_manager)
+    return;
+
+  DownloadItem* download = download_manager->GetDownload(download_id);
+  if (!download)
+    return;
+
+  ShowDownload(download);
+}
diff --git a/chrome/browser/download/download_shelf.h b/chrome/browser/download/download_shelf.h
index b04b5c2..9bf0ab4 100644
--- a/chrome/browser/download/download_shelf.h
+++ b/chrome/browser/download/download_shelf.h
@@ -5,8 +5,12 @@
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SHELF_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SHELF_H_
 
+#include "base/memory/weak_ptr.h"
+#include "base/time.h"
+
 namespace content {
 class DownloadItem;
+class DownloadManager;
 }
 
 class Browser;
@@ -16,10 +20,15 @@ class Browser;
 class DownloadShelf {
  public:
   DownloadShelf();
-  virtual ~DownloadShelf() {}
-
-  // A new download has started, so add it to our shelf. Also make the shelf
-  // visible.
+  virtual ~DownloadShelf();
+
+  // A new download has started. Add it to our shelf and show the download
+  // started animation.
+  //
+  // Some downloads are removed from the shelf on completion (See
+  // DownloadItemModel::ShouldRemoveFromShelfWhenComplete()). These transient
+  // downloads are added to the shelf after a delay. If the download completes
+  // before the delay duration, it will not be added to the shelf at all.
   void AddDownload(content::DownloadItem* download);
 
   // The browser view needs to know when we are going away to properly return
@@ -54,9 +63,28 @@ class DownloadShelf {
   virtual void DoShow() = 0;
   virtual void DoClose() = 0;
 
+  // Time delay to wait before adding a transient download to the shelf.
+  // Protected virtual for testing.
+  virtual base::TimeDelta GetTransientDownloadShowDelay();
+
+  // Returns the DownloadManager associated with this DownloadShelf. All
+  // downloads that are shown on this shelf is expected to belong to this
+  // DownloadManager. Protected virtual for testing.
+  virtual content::DownloadManager* GetDownloadManager();
+
  private:
+  // Show the download on the shelf immediately. Also displayes the download
+  // started animation if necessary.
+  void ShowDownload(content::DownloadItem* download);
+
+  // Similar to ShowDownload() but refers to the download using an ID. This
+  // download should belong to the DownloadManager returned by
+  // GetDownloadManager().
+  void ShowDownloadById(int32 download_id);
+
   bool should_show_on_unhide_;
   bool is_hidden_;
+  base::WeakPtrFactory<DownloadShelf> weak_ptr_factory_;
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SHELF_H_
diff --git a/chrome/browser/download/download_shelf_unittest.cc b/chrome/browser/download/download_shelf_unittest.cc
index fd5dc84..b77326e 100644
--- a/chrome/browser/download/download_shelf_unittest.cc
+++ b/chrome/browser/download/download_shelf_unittest.cc
@@ -2,48 +2,186 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "testing/gtest/include/gtest/gtest.h"
-
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/test_download_shelf.h"
+#include "content/public/test/mock_download_item.h"
+#include "content/public/test/mock_download_manager.h"
+#include "content/public/test/test_browser_thread.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::Return;
+using ::testing::ReturnRefOfCopy;
+using ::testing::SaveArg;
+using ::testing::_;
+using content::DownloadItem;
+
+namespace {
+
+class DownloadShelfTest : public testing::Test {
+ public:
+  DownloadShelfTest();
+
+ protected:
+  content::MockDownloadItem* download_item() {
+    return download_item_.get();
+  }
+  content::MockDownloadManager* download_manager() {
+    return download_manager_.get();
+  }
+  TestDownloadShelf* shelf() {
+    return &shelf_;
+  }
+
+ private:
+  scoped_ptr<content::MockDownloadItem> GetInProgressMockDownload();
+
+  MessageLoopForUI message_loop_;
+  content::TestBrowserThread ui_thread_;
+  scoped_ptr<content::MockDownloadItem> download_item_;
+  scoped_refptr<content::MockDownloadManager> download_manager_;
+  TestDownloadShelf shelf_;
+};
+
+DownloadShelfTest::DownloadShelfTest()
+    : ui_thread_(content::BrowserThread::UI, &message_loop_) {
+  download_item_.reset(new ::testing::NiceMock<content::MockDownloadItem>());
+  ON_CALL(*download_item_, GetAutoOpened()).WillByDefault(Return(false));
+  ON_CALL(*download_item_, GetMimeType()).WillByDefault(Return("text/plain"));
+  ON_CALL(*download_item_, GetOpenWhenComplete()).WillByDefault(Return(false));
+  ON_CALL(*download_item_, GetSafetyState())
+      .WillByDefault(Return(DownloadItem::SAFE));
+  ON_CALL(*download_item_, GetTargetDisposition())
+      .WillByDefault(Return(DownloadItem::TARGET_DISPOSITION_OVERWRITE));
+  ON_CALL(*download_item_, GetURL())
+      .WillByDefault(ReturnRefOfCopy(GURL("http://example.com/foo")));
+  ON_CALL(*download_item_, IsComplete()).WillByDefault(Return(false));
+  ON_CALL(*download_item_, IsInProgress()).WillByDefault(Return(true));
+  ON_CALL(*download_item_, IsTemporary()).WillByDefault(Return(false));
+  ON_CALL(*download_item_, ShouldOpenFileBasedOnExtension())
+      .WillByDefault(Return(false));
+
+  download_manager_ = new ::testing::NiceMock<content::MockDownloadManager>();
+  ON_CALL(*download_manager_, GetDownload(_))
+      .WillByDefault(Return(download_item_.get()));
+
+  shelf_.set_download_manager(download_manager_.get());
+}
+
+} // namespace
+
+TEST_F(DownloadShelfTest, ClosesShelfWhenHidden) {
+  shelf()->Show();
+  EXPECT_TRUE(shelf()->IsShowing());
+  shelf()->Hide();
+  EXPECT_FALSE(shelf()->IsShowing());
+  shelf()->Unhide();
+  EXPECT_TRUE(shelf()->IsShowing());
+}
+
+TEST_F(DownloadShelfTest, CloseWhileHiddenPreventsShowOnUnhide) {
+  shelf()->Show();
+  shelf()->Hide();
+  shelf()->Close();
+  shelf()->Unhide();
+  EXPECT_FALSE(shelf()->IsShowing());
+}
+
+TEST_F(DownloadShelfTest, UnhideDoesntShowIfNotShownOnHide) {
+  shelf()->Hide();
+  shelf()->Unhide();
+  EXPECT_FALSE(shelf()->IsShowing());
+}
+
+TEST_F(DownloadShelfTest, AddDownloadWhileHiddenUnhides) {
+  shelf()->Show();
+  shelf()->Hide();
+  shelf()->AddDownload(download_item());
+  EXPECT_TRUE(shelf()->IsShowing());
+}
+
+TEST_F(DownloadShelfTest, AddDownloadWhileHiddenUnhidesAndShows) {
+  shelf()->Hide();
+  shelf()->AddDownload(download_item());
+  EXPECT_TRUE(shelf()->IsShowing());
+}
+
+// Normal downloads should be added synchronously and cause the shelf to show.
+TEST_F(DownloadShelfTest, AddNormalDownload) {
+  EXPECT_FALSE(shelf()->IsShowing());
+  shelf()->AddDownload(download_item());
+  EXPECT_TRUE(shelf()->did_add_download());
+  EXPECT_TRUE(shelf()->IsShowing());
+}
+
+// Add a transient download. It should not be added immediately. Instead it
+// should be added after a delay. For testing, the delay is set to 0 seconds. So
+// the download should be added once the message loop is flushed.
+TEST_F(DownloadShelfTest, AddDelayedDownload) {
+  EXPECT_CALL(*download_item(), ShouldOpenFileBasedOnExtension())
+      .WillRepeatedly(Return(true));
+  ASSERT_TRUE(DownloadItemModel(download_item())
+              .ShouldRemoveFromShelfWhenComplete());
+  shelf()->AddDownload(download_item());
+
+  EXPECT_FALSE(shelf()->did_add_download());
+  EXPECT_FALSE(shelf()->IsShowing());
+
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
+
+  EXPECT_TRUE(shelf()->did_add_download());
+  EXPECT_TRUE(shelf()->IsShowing());
+}
+
+// Add a transient download that completes before the delay. It should not be
+// displayed on the shelf.
+TEST_F(DownloadShelfTest, AddDelayedCompletedDownload) {
+  EXPECT_CALL(*download_item(), ShouldOpenFileBasedOnExtension())
+      .WillRepeatedly(Return(true));
+  ASSERT_TRUE(DownloadItemModel(download_item())
+              .ShouldRemoveFromShelfWhenComplete());
+  shelf()->AddDownload(download_item());
+
+  EXPECT_FALSE(shelf()->did_add_download());
+  EXPECT_FALSE(shelf()->IsShowing());
+
+  EXPECT_CALL(*download_item(), IsComplete())
+      .WillRepeatedly(Return(true));
+
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
+
+  EXPECT_FALSE(shelf()->did_add_download());
+  EXPECT_FALSE(shelf()->IsShowing());
+}
+
+// Add a transient download that completes and becomes non-transient before the
+// delay. It should be displayed on the shelf even though it is complete.
+TEST_F(DownloadShelfTest, AddDelayedCompleteNonTransientDownload) {
+  EXPECT_CALL(*download_item(), ShouldOpenFileBasedOnExtension())
+      .WillRepeatedly(Return(true));
+  ASSERT_TRUE(DownloadItemModel(download_item())
+              .ShouldRemoveFromShelfWhenComplete());
+  shelf()->AddDownload(download_item());
+
+  EXPECT_FALSE(shelf()->did_add_download());
+  EXPECT_FALSE(shelf()->IsShowing());
+
+  EXPECT_CALL(*download_item(), IsComplete())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*download_item(), ShouldOpenFileBasedOnExtension())
+      .WillRepeatedly(Return(false));
+  ASSERT_FALSE(DownloadItemModel(download_item())
+               .ShouldRemoveFromShelfWhenComplete());
+
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
 
-TEST(DownloadShelfTest, ClosesShelfWhenHidden) {
-  TestDownloadShelf shelf;
-  shelf.Show();
-  EXPECT_TRUE(shelf.IsShowing());
-  shelf.Hide();
-  EXPECT_FALSE(shelf.IsShowing());
-  shelf.Unhide();
-  EXPECT_TRUE(shelf.IsShowing());
-}
-
-TEST(DownloadShelfTest, CloseWhileHiddenPreventsShowOnUnhide) {
-  TestDownloadShelf shelf;
-  shelf.Show();
-  shelf.Hide();
-  shelf.Close();
-  shelf.Unhide();
-  EXPECT_FALSE(shelf.IsShowing());
-}
-
-TEST(DownloadShelfTest, UnhideDoesntShowIfNotShownOnHide) {
-  TestDownloadShelf shelf;
-  shelf.Hide();
-  shelf.Unhide();
-  EXPECT_FALSE(shelf.IsShowing());
-}
-
-TEST(DownloadShelfTest, AddDownloadWhileHiddenUnhides) {
-  TestDownloadShelf shelf;
-  shelf.Show();
-  shelf.Hide();
-  shelf.AddDownload(NULL);
-  EXPECT_TRUE(shelf.IsShowing());
-}
-
-TEST(DownloadShelfTest, AddDownloadWhileHiddenUnhidesAndShows) {
-  TestDownloadShelf shelf;
-  shelf.Hide();
-  shelf.AddDownload(NULL);
-  EXPECT_TRUE(shelf.IsShowing());
+  EXPECT_TRUE(shelf()->did_add_download());
+  EXPECT_TRUE(shelf()->IsShowing());
 }
diff --git a/chrome/browser/download/test_download_shelf.cc b/chrome/browser/download/test_download_shelf.cc
index 9957c64..6eef694 100644
--- a/chrome/browser/download/test_download_shelf.cc
+++ b/chrome/browser/download/test_download_shelf.cc
@@ -4,8 +4,11 @@
 
 #include "chrome/browser/download/test_download_shelf.h"
 
+#include "content/public/browser/download_manager.h"
+
 TestDownloadShelf::TestDownloadShelf()
-    : is_showing_(false) {
+    : is_showing_(false),
+      did_add_download_(false) {
 }
 
 TestDownloadShelf::~TestDownloadShelf() {
@@ -23,7 +26,13 @@ Browser* TestDownloadShelf::browser() const {
   return NULL;
 }
 
+void TestDownloadShelf::set_download_manager(
+    content::DownloadManager* download_manager) {
+  download_manager_ = download_manager;
+}
+
 void TestDownloadShelf::DoAddDownload(content::DownloadItem* download) {
+  did_add_download_ = true;
 }
 
 void TestDownloadShelf::DoShow() {
@@ -34,3 +43,10 @@ void TestDownloadShelf::DoClose() {
   is_showing_ = false;
 }
 
+base::TimeDelta TestDownloadShelf::GetTransientDownloadShowDelay() {
+  return base::TimeDelta();
+}
+
+content::DownloadManager* TestDownloadShelf::GetDownloadManager() {
+  return download_manager_.get();
+}
diff --git a/chrome/browser/download/test_download_shelf.h b/chrome/browser/download/test_download_shelf.h
index da7bb57..32d5c81 100644
--- a/chrome/browser/download/test_download_shelf.h
+++ b/chrome/browser/download/test_download_shelf.h
@@ -7,8 +7,13 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
 #include "chrome/browser/download/download_shelf.h"
 
+namespace content {
+class DownloadManager;
+}
+
 // An implementation of DownloadShelf for testing.
 class TestDownloadShelf : public DownloadShelf {
  public:
@@ -16,18 +21,27 @@ class TestDownloadShelf : public DownloadShelf {
   virtual ~TestDownloadShelf();
 
   // DownloadShelf:
-
   virtual bool IsShowing() const OVERRIDE;
   virtual bool IsClosing() const OVERRIDE;
   virtual Browser* browser() const OVERRIDE;
 
+  // Return |true| if a download was added to this shelf.
+  bool did_add_download() const { return did_add_download_; }
+
+  // Set download_manager_ (and the result of calling GetDownloadManager())
+  void set_download_manager(content::DownloadManager* download_manager);
+
  protected:
   virtual void DoAddDownload(content::DownloadItem* download) OVERRIDE;
   virtual void DoShow() OVERRIDE;
   virtual void DoClose() OVERRIDE;
+  virtual base::TimeDelta GetTransientDownloadShowDelay() OVERRIDE;
+  virtual content::DownloadManager* GetDownloadManager() OVERRIDE;
 
  private:
   bool is_showing_;
+  bool did_add_download_;
+  scoped_refptr<content::DownloadManager> download_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(TestDownloadShelf);
 };
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 9feaab1..b519b0e 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -44,13 +44,10 @@
 #include "chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h"
 #include "chrome/browser/devtools/devtools_toggle_action.h"
 #include "chrome/browser/devtools/devtools_window.h"
-#include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/download/download_shelf.h"
-#include "chrome/browser/download/download_started_animation.h"
-#include "chrome/browser/download/download_util.h"
 #include "chrome/browser/extensions/browser_extension_window_controller.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -68,7 +65,6 @@
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/pepper_broker_infobar_delegate.h"
-#include "chrome/browser/platform_util.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/printing/cloud_print/cloud_print_setup_flow.h"
@@ -187,7 +183,6 @@
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/url_request/url_request_context.h"
-#include "ui/base/animation/animation.h"
 #include "ui/base/dialogs/selected_file_info.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/point.h"
@@ -1448,19 +1443,6 @@ void Browser::OnStartDownload(WebContents* source,
   // GetDownloadShelf creates the download shelf if it was not yet created.
   DownloadShelf* shelf = window()->GetDownloadShelf();
   shelf->AddDownload(download);
-  // 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.
diff --git a/chrome/browser/ui/cocoa/download/download_item_mac.mm b/chrome/browser/ui/cocoa/download/download_item_mac.mm
index ea3dd9b..c9f278a 100644
--- a/chrome/browser/ui/cocoa/download/download_item_mac.mm
+++ b/chrome/browser/ui/cocoa/download/download_item_mac.mm
@@ -47,7 +47,7 @@ void DownloadItemMac::OnDownloadUpdated(content::DownloadItem* download) {
 
   switch (download->GetState()) {
     case DownloadItem::COMPLETE:
-      if (download->GetAutoOpened()) {
+      if (download_model_.ShouldRemoveFromShelfWhenComplete()) {
         [item_controller_ remove];  // We're deleted now!
         return;
       }
diff --git a/chrome/browser/ui/gtk/download/download_item_gtk.cc b/chrome/browser/ui/gtk/download/download_item_gtk.cc
index e1da54d..802910e 100644
--- a/chrome/browser/ui/gtk/download/download_item_gtk.cc
+++ b/chrome/browser/ui/gtk/download/download_item_gtk.cc
@@ -339,9 +339,10 @@ void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download_item) {
       complete_animation_.Show();
       break;
     case DownloadItem::COMPLETE:
-      // GetAutoOpened() may change after the download's initial transition to
-      // COMPLETE, so we check it before the idemopotency shield below.
-      if (download()->GetAutoOpened()) {
+      // ShouldRemoveFromShelfWhenComplete() may change after the download's
+      // initial transition to COMPLETE, so we check it before the idemopotency
+      // shield below.
+      if (download_model_.ShouldRemoveFromShelfWhenComplete()) {
         parent_shelf_->RemoveDownloadItem(this);  // This will delete us!
         return;
       }
diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc
index 13bbd761..46896f6 100644
--- a/chrome/browser/ui/views/download/download_item_view.cc
+++ b/chrome/browser/ui/views/download/download_item_view.cc
@@ -270,7 +270,7 @@ void DownloadItemView::OnDownloadUpdated(DownloadItem* download_item) {
         LoadIcon();
         break;
       case DownloadItem::COMPLETE:
-        if (download()->GetAutoOpened()) {
+        if (model_.ShouldRemoveFromShelfWhenComplete()) {
           shelf_->RemoveDownloadView(this);  // This will delete us!
           return;
         }
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 5a6977e..e0240b7 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -2102,6 +2102,7 @@
 
             # There's no Browser/BrowserList on Android.
             'browser/browser_commands_unittest.cc',
+            'browser/download/download_shelf_unittest.cc',
             'browser/managed_mode/managed_mode_unittest.cc',
             'browser/managed_mode/managed_mode_url_filter_unittest.cc',
             'browser/net/gaia/gaia_oauth_fetcher_unittest.cc',