Remove auto-open items from the download shelf on auto-open. This has the delicious side-effect of auto-closing the download shelf after extensions are downloaded and auto installed.

While we're here, also add proper colors to the download buttons.

BUG=16232
TEST=Install a theme from the web, make sure the download shelf isn't shown after installation. Do the same for a filetype that is set to auto-open. Make sure the shelf stays open for other filetypes.

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

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

3 files changed, 34 insertions, 23 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 7f1cb63..c2024d0 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -127,6 +127,7 @@ DownloadItem::DownloadItem(const DownloadCreateInfo& info)
       is_paused_(false),
       open_when_complete_(false),
       safety_state_(SAFE),
+      auto_opened_(false),
       original_name_(info.original_name),
       render_process_id_(-1),
       request_id_(-1) {
@@ -160,6 +161,7 @@ DownloadItem::DownloadItem(int32 download_id,
       is_paused_(false),
       open_when_complete_(false),
       safety_state_(is_dangerous ? DANGEROUS : SAFE),
+      auto_opened_(false),
       original_name_(original_name),
       render_process_id_(render_process_id),
       request_id_(request_id) {
@@ -829,26 +831,26 @@ void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
   if (it != dangerous_finished_.end())
     dangerous_finished_.erase(it);
 
-  // Notify our observers that we are complete (the call to Finished() set the
-  // state to complete but did not notify).
-  download->UpdateObservers();
-
-  // Handle chrome extensions explicitly and skip the shell execute.
-  if (Extension::IsExtension(download->full_path())) {
-    OpenChromeExtension(download->full_path());
-    return;
-  }
-
   // Open the download if the user or user prefs indicate it should be.
   FilePath::StringType extension = download->full_path().Extension();
   // Drop the leading period. (The auto-open list is period-less.)
   if (extension.size() > 0)
     extension = extension.substr(1);
 
-  if (download->open_when_complete() || ShouldOpenFileExtension(extension))
+  // Handle chrome extensions explicitly and skip the shell execute.
+  if (Extension::IsExtension(download->full_path())) {
+    OpenChromeExtension(download->full_path());
+    download->set_auto_opened(true);
+  } else if (download->open_when_complete() ||
+             ShouldOpenFileExtension(extension)) {
     OpenDownloadInShell(download, NULL);
-}
+    download->set_auto_opened(true);
+  }
 
+  // Notify our observers that we are complete (the call to Finished() set the
+  // state to complete but did not notify).
+  download->UpdateObservers();
+}
 // Called on the file thread.  Renames the downloaded file to its original name.
 void DownloadManager::ProceedWithFinishedDangerousDownload(
     int64 download_handle,
@@ -1461,12 +1463,6 @@ void DownloadManager::OnSearchComplete(HistoryService::Handle handle,
 
 void DownloadManager::ShowDownloadInBrowser(const DownloadCreateInfo& info,
                                             DownloadItem* download) {
-  // Extension downloading skips the shelf.  This is a temporary fix until
-  // we can modularize the download system and develop specific extensiona
-  // install UI.
-  if (Extension::IsExtension(info.path))
-    return;
-
   // The 'contents' may no longer exist if the user closed the tab before we get
   // this start completion event. If it does, tell the origin TabContents to
   // display its download shelf.
@@ -1489,4 +1485,4 @@ void DownloadManager::ShowDownloadInBrowser(const DownloadCreateInfo& info,
 // Clears the last download path, used to initialize "save as" dialogs.
 void DownloadManager::ClearLastDownloadPath() {
   last_download_path_ = FilePath();
-}
+}
\ No newline at end of file
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index b581ca2..4fdc8dd 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -203,6 +203,8 @@ class DownloadItem {
   void set_safety_state(SafetyState safety_state) {
     safety_state_ = safety_state;
   }
+  bool auto_opened() { return auto_opened_; }
+  void set_auto_opened(bool auto_opened) { auto_opened_ = auto_opened; }
   FilePath original_name() const { return original_name_; }
   void set_original_name(const FilePath& name) { original_name_ = name; }
 
@@ -268,6 +270,11 @@ class DownloadItem {
   // (executable files are typically considered dangerous).
   SafetyState safety_state_;
 
+  // Whether the download was auto-opened. We set this rather than using
+  // an observer as it's frequently possible for the download to be auto opened
+  // before the observer is added.
+  bool auto_opened_;
+
   // Dangerous download are given temporary names until the user approves them.
   // This stores their original name.
   FilePath original_name_;
diff --git a/chrome/browser/views/download_item_view.cc b/chrome/browser/views/download_item_view.cc
index fcf832b..7ad4847 100644
--- a/chrome/browser/views/download_item_view.cc
+++ b/chrome/browser/views/download_item_view.cc
@@ -10,10 +10,12 @@
 #include "app/gfx/text_elider.h"
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
+#include "app/theme_provider.h"
 #include "base/file_path.h"
 #include "base/string_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_theme_provider.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/views/download_shelf_view.h"
@@ -55,7 +57,6 @@ static const int kButtonPadding = 5;  // Pixels.
 // The space on the left and right side of the dangerous download label.
 static const int kLabelPadding = 4;  // Pixels.
 
-static const SkColor kFileNameColor = SkColorSetRGB(87, 108, 149);
 static const SkColor kFileNameDisabledColor = SkColorSetRGB(171, 192, 212);
 static const SkColor kStatusColor = SkColorSetRGB(123, 141, 174);
 
@@ -305,7 +306,8 @@ DownloadItemView::DownloadItemView(DownloadItem* download,
     dangerous_download_label_->SetMultiLine(true);
     dangerous_download_label_->SetHorizontalAlignment(
         views::Label::ALIGN_LEFT);
-    dangerous_download_label_->SetColor(kFileNameColor);
+    dangerous_download_label_->SetColor(GetThemeProvider()->GetColor(
+        BrowserThemeProvider::COLOR_BOOKMARK_TEXT));
     AddChildView(dangerous_download_label_);
     SizeLabelToMinWidth();
   }
@@ -360,6 +362,10 @@ void DownloadItemView::OnDownloadUpdated(DownloadItem* download) {
       download_->is_paused() ? StopDownloadProgress() : StartDownloadProgress();
       break;
     case DownloadItem::COMPLETE:
+      if (download_->auto_opened()) {
+        parent_->RemoveDownloadView(this);  // This will delete us!
+        return;
+      }
       StopDownloadProgress();
       complete_animation_.reset(new SlideAnimation(this));
       complete_animation_->SetSlideDuration(kCompleteAnimationDurationMs);
@@ -596,12 +602,14 @@ void DownloadItemView::Paint(gfx::Canvas* canvas) {
 
     int mirrored_x = MirroredXWithWidthInsideView(
         download_util::kSmallProgressIconSize, kTextWidth);
+    SkColor file_name_color = GetThemeProvider()->GetColor(
+        BrowserThemeProvider::COLOR_BOOKMARK_TEXT);
     if (show_status_text_) {
       int y = box_y_ + kVerticalPadding;
 
       // Draw the file's name.
       canvas->DrawStringInt(filename, font_,
-                            IsEnabled() ? kFileNameColor :
+                            IsEnabled() ? file_name_color :
                                           kFileNameDisabledColor,
                             mirrored_x, y, kTextWidth, font_.height());
 
@@ -614,7 +622,7 @@ void DownloadItemView::Paint(gfx::Canvas* canvas) {
 
       // Draw the file's name.
       canvas->DrawStringInt(filename, font_,
-                            IsEnabled() ? kFileNameColor :
+                            IsEnabled() ? file_name_color :
                                           kFileNameDisabledColor,
                             mirrored_x, y, kTextWidth, font_.height());
     }