Remove completed downloads from shelf when the shelf closes.

BUG=15712
TEST=Download a few things, close shelf, download something else, note initial few things are no longer on the shelf.

Special cases:

* Dangerous downloads still awaiting affirmation should never be removed, no matter if they are completed or not.

* Cancelled downloads should be removed.

* In-progress downloads should not be removed.

Review URL: http://codereview.chromium.org/173186

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

9 files changed, 91 insertions, 14 deletions

diff --git a/chrome/browser/cocoa/download_item_controller.h b/chrome/browser/cocoa/download_item_controller.h
index dd8d4e2c..c34c557 100644
--- a/chrome/browser/cocoa/download_item_controller.h
+++ b/chrome/browser/cocoa/download_item_controller.h
@@ -9,6 +9,7 @@
 
 class BaseDownloadItemModel;
 @class DownloadItemCell;
+class DownloadItem;
 class DownloadItemMac;
 class DownloadShelfContextMenuMac;
 @class DownloadShelfController;
@@ -68,6 +69,9 @@ class DownloadShelfContextMenuMac;
 // Returns the size this item wants to have.
 - (NSSize)preferredSize;
 
+// Returns the DownloadItem model object belonging to this item.
+- (DownloadItem*)download;
+
 // Handling of dangerous downloads
 - (void)clearDangerousMode;
 - (BOOL)isDangerousMode;
diff --git a/chrome/browser/cocoa/download_item_controller.mm b/chrome/browser/cocoa/download_item_controller.mm
index fd40a6a..7058a2a 100644
--- a/chrome/browser/cocoa/download_item_controller.mm
+++ b/chrome/browser/cocoa/download_item_controller.mm
@@ -137,6 +137,10 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu {
   return [dangerousDownloadView_ frame].size;
 }
 
+- (DownloadItem*)download {
+  return bridge_->download_model()->download();
+}
+
 - (void)clearDangerousMode {
   [self setState:kNormal];
 }
diff --git a/chrome/browser/cocoa/download_shelf_controller.mm b/chrome/browser/cocoa/download_shelf_controller.mm
index c9448d6..73b2af5 100644
--- a/chrome/browser/cocoa/download_shelf_controller.mm
+++ b/chrome/browser/cocoa/download_shelf_controller.mm
@@ -13,6 +13,7 @@
 #include "chrome/browser/cocoa/download_item_controller.h"
 #include "chrome/browser/cocoa/download_shelf_mac.h"
 #import "chrome/browser/cocoa/download_shelf_view.h"
+#include "chrome/browser/download/download_manager.h"
 #include "grit/generated_resources.h"
 
 namespace {
@@ -34,6 +35,7 @@ const NSTimeInterval kDownloadItemOpenDuration = 0.8;
 - (void)showDownloadShelf:(BOOL)enable;
 - (void)resizeDownloadLinkToFit;
 - (void)layoutItems:(BOOL)skipFirst;
+- (void)closed;
 @end
 
 
@@ -178,6 +180,10 @@ const NSTimeInterval kDownloadItemOpenDuration = 0.8;
     bridge_->Close();
   else
     [self showDownloadShelf:NO];
+
+  // TODO(port): When closing the shelf is animated, call this only after the
+  // animation has ended:
+  [self closed];
 }
 
 - (float)height {
@@ -254,4 +260,21 @@ const NSTimeInterval kDownloadItemOpenDuration = 0.8;
   [self layoutItems:YES];
 }
 
+- (void)closed {
+  NSUInteger i = 0;
+  while (i < [downloadItemControllers_.get() count]) {
+    DownloadItemController* itemController = [downloadItemControllers_.get()
+        objectAtIndex:i];
+    bool isTransferDone =
+        [itemController download]->state() == DownloadItem::COMPLETE ||
+        [itemController download]->state() == DownloadItem::CANCELLED;
+    if (isTransferDone &&
+        [itemController download]->safety_state() != DownloadItem::DANGEROUS) {
+      [self remove:itemController];
+    } else {
+      ++i;
+    }
+  }
+}
+
 @end
diff --git a/chrome/browser/gtk/download_item_gtk.h b/chrome/browser/gtk/download_item_gtk.h
index 4557695..c490c94 100644
--- a/chrome/browser/gtk/download_item_gtk.h
+++ b/chrome/browser/gtk/download_item_gtk.h
@@ -53,11 +53,12 @@ class DownloadItemGtk : public DownloadItem::Observer,
   // ownership of |icon_bitmap|.
   void OnLoadIconComplete(IconManager::Handle handle, SkBitmap* icon_bitmap);
 
+  // Returns the DownloadItem model object belonging to this item.
+  DownloadItem* get_download();
+
  private:
   friend class DownloadShelfContextMenuGtk;
 
-  DownloadItem* get_download();
-
   // Returns true IFF the download is dangerous and unconfirmed.
   bool IsDangerous();
 
@@ -203,6 +204,7 @@ class DownloadItemGtk : public DownloadItem::Observer,
 
   // The file icon for the download. May be null.
   SkBitmap* icon_;
+
   // The last download file path for which we requested an icon.
   FilePath icon_filepath_;
 
diff --git a/chrome/browser/gtk/download_shelf_gtk.cc b/chrome/browser/gtk/download_shelf_gtk.cc
index 7917c21..6188b75 100644
--- a/chrome/browser/gtk/download_shelf_gtk.cc
+++ b/chrome/browser/gtk/download_shelf_gtk.cc
@@ -15,7 +15,6 @@
 #include "chrome/browser/gtk/download_item_gtk.h"
 #include "chrome/browser/gtk/gtk_chrome_link_button.h"
 #include "chrome/browser/gtk/gtk_theme_provider.h"
-#include "chrome/browser/gtk/slide_animator_gtk.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/gtk_util.h"
 #include "chrome/common/notification_service.h"
@@ -120,7 +119,7 @@ DownloadShelfGtk::DownloadShelfGtk(Browser* browser, GtkWidget* parent)
   slide_widget_.reset(new SlideAnimatorGtk(shelf_.get(),
                                            SlideAnimatorGtk::UP,
                                            kShelfAnimationDurationMs,
-                                           false, true, NULL));
+                                           false, true, this));
 
   theme_provider_->InitThemesFor(this);
   registrar_.Add(this, NotificationType::BROWSER_THEME_CHANGED,
@@ -171,6 +170,22 @@ void DownloadShelfGtk::Close() {
   browser_->UpdateDownloadShelfVisibility(false);
 }
 
+void DownloadShelfGtk::Closed() {
+  // When the close animation is complete, remove all completed downloads.
+  size_t i = 0;
+  while (i < download_items_.size()) {
+    DownloadItem* download = download_items_[i]->get_download();
+    bool is_transfer_done = download->state() == DownloadItem::COMPLETE ||
+                            download->state() == DownloadItem::CANCELLED;
+    if (is_transfer_done &&
+        download->safety_state() != DownloadItem::DANGEROUS) {
+      RemoveDownloadItem(download_items_[i]);
+    } else {
+      ++i;
+    }
+  }
+}
+
 void DownloadShelfGtk::Observe(NotificationType type,
                                const NotificationSource& source,
                                const NotificationDetails& details) {
diff --git a/chrome/browser/gtk/download_shelf_gtk.h b/chrome/browser/gtk/download_shelf_gtk.h
index e5bb01b..94a34e2 100644
--- a/chrome/browser/gtk/download_shelf_gtk.h
+++ b/chrome/browser/gtk/download_shelf_gtk.h
@@ -12,6 +12,7 @@
 #include "base/gfx/native_widget_types.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/download/download_shelf.h"
+#include "chrome/browser/gtk/slide_animator_gtk.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/owned_widget_gtk.h"
@@ -24,7 +25,8 @@ class GtkThemeProvider;
 class SlideAnimatorGtk;
 
 class DownloadShelfGtk : public DownloadShelf,
-                         public NotificationObserver {
+                         public NotificationObserver,
+                         public SlideAnimatorGtk::Delegate {
  public:
   explicit DownloadShelfGtk(Browser* browser, gfx::NativeView view);
 
@@ -38,6 +40,9 @@ class DownloadShelfGtk : public DownloadShelf,
   virtual void Close();
   virtual Browser* browser() const { return browser_; }
 
+  // SlideAnimatorGtk::Delegate implementation.
+  virtual void Closed();
+
   // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
diff --git a/chrome/browser/views/download_item_view.h b/chrome/browser/views/download_item_view.h
index e3fd235..a97d6a0 100644
--- a/chrome/browser/views/download_item_view.h
+++ b/chrome/browser/views/download_item_view.h
@@ -77,6 +77,9 @@ class DownloadItemView : public views::ButtonListener,
   // IconManager::Client interface.
   void OnExtractIconComplete(IconManager::Handle handle, SkBitmap* icon_bitmap);
 
+  // Returns the DownloadItem model object belonging to this item.
+  DownloadItem* get_download() { return download_; }
+
  private:
   enum State {
     NORMAL = 0,
diff --git a/chrome/browser/views/download_shelf_view.cc b/chrome/browser/views/download_shelf_view.cc
index c708713..59a730ce 100644
--- a/chrome/browser/views/download_shelf_view.cc
+++ b/chrome/browser/views/download_shelf_view.cc
@@ -111,7 +111,7 @@ void DownloadShelfView::Init() {
   Show();
 }
 
-void DownloadShelfView::AddDownloadView(View* view) {
+void DownloadShelfView::AddDownloadView(DownloadItemView* view) {
   Show();
 
   DCHECK(view);
@@ -132,7 +132,7 @@ void DownloadShelfView::AddDownload(BaseDownloadItemModel* download_model) {
 
 void DownloadShelfView::RemoveDownloadView(View* view) {
   DCHECK(view);
-  std::vector<View*>::iterator i =
+  std::vector<DownloadItemView*>::iterator i =
     find(download_views_.begin(), download_views_.end(), view);
   DCHECK(i != download_views_.end());
   download_views_.erase(i);
@@ -187,8 +187,11 @@ void DownloadShelfView::AnimationProgressed(const Animation *animation) {
 }
 
 void DownloadShelfView::AnimationEnded(const Animation *animation) {
-  if (animation == shelf_animation_.get())
+  if (animation == shelf_animation_.get()) {
     parent_->SetDownloadShelfVisible(shelf_animation_->IsShowing());
+    if (!shelf_animation_->IsShowing())
+      Closed();
+  }
 }
 
 void DownloadShelfView::Layout() {
@@ -232,14 +235,14 @@ void DownloadShelfView::Layout() {
                            close_button_size.height());
   if (show_link_only) {
     // Let's hide all the items.
-    std::vector<View*>::reverse_iterator ri;
+    std::vector<DownloadItemView*>::reverse_iterator ri;
     for (ri = download_views_.rbegin(); ri != download_views_.rend(); ++ri)
       (*ri)->SetVisible(false);
     return;
   }
 
   next_x = kLeftPadding;
-  std::vector<View*>::reverse_iterator ri;
+  std::vector<DownloadItemView*>::reverse_iterator ri;
   for (ri = download_views_.rbegin(); ri != download_views_.rend(); ++ri) {
     gfx::Size view_size = (*ri)->GetPreferredSize();
 
@@ -324,3 +327,19 @@ void DownloadShelfView::Close() {
   parent_->SetDownloadShelfVisible(false);
   shelf_animation_->Hide();
 }
+
+void DownloadShelfView::Closed() {
+  // When the close animation is complete, remove all completed downloads.
+  size_t i = 0;
+  while (i < download_views_.size()) {
+    DownloadItem* download = download_views_[i]->get_download();
+    bool is_transfer_done = download->state() == DownloadItem::COMPLETE ||
+                            download->state() == DownloadItem::CANCELLED;
+    if (is_transfer_done &&
+        download->safety_state() != DownloadItem::DANGEROUS) {
+      RemoveDownloadView(download_views_[i]);
+    } else {
+      ++i;
+    }
+  }
+}
diff --git a/chrome/browser/views/download_shelf_view.h b/chrome/browser/views/download_shelf_view.h
index a35a6b73..148acff 100644
--- a/chrome/browser/views/download_shelf_view.h
+++ b/chrome/browser/views/download_shelf_view.h
@@ -20,12 +20,11 @@ class Browser;
 class BrowserView;
 
 class DownloadAnimation;
+class DownloadItemView;
 
 // DownloadShelfView is a view that contains individual views for each download,
 // as well as a close button and a link to show all downloads.
 //
-// To add a view representing a download to DownloadShelfView, invoke
-// AddDownloadView. AddDownloadView takes ownership of the passed in View.
 // DownloadShelfView does not hold an infinite number of download views, rather
 // it'll automatically remove views once a certain point is reached.
 class DownloadShelfView : public DownloadShelf,
@@ -73,7 +72,7 @@ class DownloadShelfView : public DownloadShelf,
   // Adds a View representing a download to this DownloadShelfView.
   // DownloadShelfView takes ownership of the View, and will delete it as
   // necessary.
-  void AddDownloadView(views::View* view);
+  void AddDownloadView(DownloadItemView* view);
 
   // Paints the border.
   void PaintBorder(gfx::Canvas* canvas);
@@ -87,6 +86,9 @@ class DownloadShelfView : public DownloadShelf,
   // Overridden from views::View.
   virtual void ThemeChanged();
 
+  // Called when the "close shelf" animation ended.
+  void Closed();
+
   // The browser for this shelf.
   Browser* browser_;
 
@@ -98,7 +100,7 @@ class DownloadShelfView : public DownloadShelf,
 
   // The download views. These are also child Views, and deleted when
   // the DownloadShelfView is deleted.
-  std::vector<View*> download_views_;
+  std::vector<DownloadItemView*> download_views_;
 
   // An image displayed on the right of the "Show all downloads..." link.
   views::ImageView* arrow_image_;