Handle extensions correctly when the default download location is not writable

BUG=28668
TEST=Set the Download location a NON-writable location (such as '/Application' on Mac) in 'Under the Hood' preferences.  Make sure 'Ask where to save...' is kept unchecked.
Go to 'https://tools.google.com/chrome/intl/en/themes/index.html' and click 'apply theme'.
Make sure 'Save as...' dialog opens to ask users for an alternative download location.  Save the file in a writable directory.
Make sure the theme applies (or an extension installation dialog opens for extensions).
Make sure the download does NOT appear in Downloads history.
Make sure it also works for chrome extensions.

TEST=(regression) Make sure theme/extension installation works as expected in normal cases (when the default download location is pointing to a writable location and 'Ask where to save...' is unchecked in 'Under the Hood' preferences.)     Make sure the download does NOT appear in Downloads history.                    Make sure the same thing happens for chrome extensions.
TEST=(regression) Make sure right-clicking on theme/extension files and selecting 'Save link as...' works as a normal download.
Make sure the file is downloaded and saved with the given name.
Make sure the theme does NOT apply and an extension installation dialog does NOT open.
Make sure the download appears in Downloads history.

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

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

3 files changed, 25 insertions, 34 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 21642ef..ed0149b 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -154,7 +154,8 @@ DownloadItem::DownloadItem(int32 download_id,
                            int render_process_id,
                            int request_id,
                            bool is_dangerous,
-                           bool save_as)
+                           bool save_as,
+                           bool is_extension_install)
     : id_(download_id),
       full_path_(path),
       path_uniquifier_(path_uniquifier),
@@ -175,7 +176,8 @@ DownloadItem::DownloadItem(int32 download_id,
       original_name_(original_name),
       render_process_id_(render_process_id),
       request_id_(request_id),
-      save_as_(save_as) {
+      save_as_(save_as),
+      is_extension_install_(is_extension_install) {
   Init(true /* start progress timer */);
 }
 
@@ -543,6 +545,13 @@ void DownloadManager::StartDownload(DownloadCreateInfo* info) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   DCHECK(info);
 
+  // Check whether this download is for an extension install or not.
+  if (!info->save_as) {  // Allow extensions to be explicitly saved.
+    if (UserScript::HasUserScriptFileExtension(info->url) ||
+        info->mime_type == Extension::kMimeType)
+      info->is_extension_install = true;
+  }
+
   // Freeze the user's preference for showing a Save As dialog.  We're going to
   // bounce around a bunch of threads and we don't want to worry about race
   // conditions where the user changes this pref out from under us.
@@ -550,7 +559,7 @@ void DownloadManager::StartDownload(DownloadCreateInfo* info) {
     // But never obey the preference for extension installation. Note that we
     // only care here about the case where an extension is installed, not when
     // one is downloaded with "save as...".
-    if (!IsExtensionInstall(info))
+    if (!info->is_extension_install)
       info->save_as = true;
   }
 
@@ -570,7 +579,7 @@ void DownloadManager::StartDownload(DownloadCreateInfo* info) {
     // b) They are an extension that is not from the gallery
     if (IsDangerous(info->suggested_path.BaseName()))
       info->is_dangerous = true;
-    else if (IsExtensionInstall(info) &&
+    else if (info->is_extension_install &&
              !ExtensionsService::IsDownloadFromGallery(info->url,
                                                        info->referrer_url)) {
       info->is_dangerous = true;
@@ -693,7 +702,8 @@ void DownloadManager::ContinueStartDownload(DownloadCreateInfo* info,
                                 info->child_id,
                                 info->request_id,
                                 info->is_dangerous,
-                                info->save_as);
+                                info->save_as,
+                                info->is_extension_install);
     download->set_manager(this);
     in_progress_[info->download_id] = download;
   } else {
@@ -725,7 +735,7 @@ void DownloadManager::ContinueStartDownload(DownloadCreateInfo* info,
   // handles, so we use a negative value. Eventually, they could overlap, but
   // you'd have to do enough downloading that your ISP would likely stab you in
   // the neck first. YMMV.
-  if (profile_->IsOffTheRecord() || IsExtensionInstall(download)) {
+  if (profile_->IsOffTheRecord() || download->is_extension_install()) {
     static int64 fake_db_handle = kUninitializedHandle - 1;
     OnCreateDownloadEntryComplete(*info, fake_db_handle--);
   } else {
@@ -848,7 +858,7 @@ void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
     dangerous_finished_.erase(it);
 
   // Handle chrome extensions explicitly and skip the shell execute.
-  if (IsExtensionInstall(download)) {
+  if (download->is_extension_install()) {
     OpenChromeExtension(download->full_path(), download->url(),
                         download->referrer_url());
     download->set_auto_opened(true);
@@ -1229,7 +1239,7 @@ void DownloadManager::OpenDownload(const DownloadItem* download,
                                    gfx::NativeView parent_window) {
   // Open Chrome extensions with ExtensionsService. For everything else do shell
   // execute.
-  if (IsExtensionInstall(download)) {
+  if (download->is_extension_install()) {
     OpenChromeExtension(download->full_path(), download->url(),
                         download->referrer_url());
   } else {
@@ -1468,26 +1478,6 @@ void DownloadManager::GenerateSafeFilename(const std::string& mime_type,
 #endif
 }
 
-bool DownloadManager::IsExtensionInstall(const DownloadItem* item) {
-  if (item->save_as())
-    return false;
-
-  if (UserScript::HasUserScriptFileExtension(item->url()))
-    return true;
-
-  return item->mime_type() == Extension::kMimeType;
-}
-
-bool DownloadManager::IsExtensionInstall(const DownloadCreateInfo* info) {
-  if (info->save_as)
-    return false;
-
-  if (UserScript::HasUserScriptFileExtension(info->url))
-    return true;
-
-  return info->mime_type == Extension::kMimeType;
-}
-
 // Operations posted to us from the history service ----------------------------
 
 // The history service has retrieved all download entries. 'entries' contains
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 81588ee..1d27625 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -118,7 +118,8 @@ class DownloadItem {
                int render_process_id,
                int request_id,
                bool is_dangerous,
-               bool save_as);
+               bool save_as,
+               bool is_extension_install);
 
   ~DownloadItem();
 
@@ -215,6 +216,7 @@ class DownloadItem {
   FilePath original_name() const { return original_name_; }
   void set_original_name(const FilePath& name) { original_name_ = name; }
   bool save_as() const { return save_as_; }
+  bool is_extension_install() const { return is_extension_install_; }
 
   // Returns the file-name that should be reported to the user, which is
   // file_name_ for safe downloads and original_name_ for dangerous ones with
@@ -300,6 +302,9 @@ class DownloadItem {
   // True if the item was downloaded as a result of 'save as...'
   bool save_as_;
 
+  // True if the item was downloaded for an extension installation.
+  bool is_extension_install_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);
 };
 
@@ -460,10 +465,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   void GenerateSafeFilename(const std::string& mime_type,
                             FilePath* file_name);
 
-  // Used to determine whether the download item is an extension file or not.
-  static bool IsExtensionInstall(const DownloadItem* item);
-  static bool IsExtensionInstall(const DownloadCreateInfo* info);
-
   // Runs the network cancel.  Must be called on the IO thread.
   static void OnCancelDownloadRequest(ResourceDispatcherHost* rdh,
                                       int render_process_id,
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 7b89674..03a4cb4 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -304,7 +304,7 @@ bool SavePackage::Init() {
 
   // Create the fake DownloadItem and display the view.
   download_ = new DownloadItem(1, saved_main_file_path_, 0, page_url_, GURL(),
-      "", FilePath(), Time::Now(), 0, -1, -1, false, false);
+      "", FilePath(), Time::Now(), 0, -1, -1, false, false, false);
   download_->set_manager(tab_contents_->profile()->GetDownloadManager());
   tab_contents_->OnStartDownload(download_);