Don't prompt to save malicious downloads on exit

chrome/browser/lifetime/browser_close_manager_browsertest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/download/chrome_download_manager_delegate.h"
+#include "chrome/browser/download/download_service.h"
+#include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/lifetime/browser_close_manager.h"
 #include "chrome/browser/net/url_request_mock_util.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.h"
 #include "chrome/browser/ui/app_modal_dialogs/native_app_modal_dialog.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.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/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/download_test_observer.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/test/net/url_request_mock_http_job.h"
 #include "content/test/net/url_request_slow_download_job.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 
 namespace {
 
 class AppModalDialogObserver {
  public:
   AppModalDialogObserver() {}
 
   void Start() {
@@ skipping 70 matching lines @@
   base::RunLoop run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(RepeatedNotificationObserver);
 };
 
 class TestBrowserCloseManager : public BrowserCloseManager {
  public:
   enum UserChoice {
     USER_CHOICE_USER_CANCELS_CLOSE,
     USER_CHOICE_USER_ALLOWS_CLOSE,
+    NO_USER_CHOICE
   };
 
   static void AttemptClose(UserChoice user_choice) {
     scoped_refptr<BrowserCloseManager> browser_close_manager =
         new TestBrowserCloseManager(user_choice);
     browser_close_manager->StartClosingBrowsers();
   }
 
  protected:
   virtual ~TestBrowserCloseManager() {}
 
   virtual void ConfirmCloseWithPendingDownloads(
       int download_count,
       const base::Callback<void(bool)>& callback) OVERRIDE {
+    EXPECT_NE(NO_USER_CHOICE, user_choice_);
     switch (user_choice_) {
+      case NO_USER_CHOICE:
       case USER_CHOICE_USER_CANCELS_CLOSE: {
         callback.Run(false);
         break;
       }
       case USER_CHOICE_USER_ALLOWS_CLOSE: {
         callback.Run(true);
         break;
       }
     }
   }
 
  private:
   explicit TestBrowserCloseManager(UserChoice user_choice)
       : user_choice_(user_choice) {}
 
   UserChoice user_choice_;
 
   DISALLOW_COPY_AND_ASSIGN(TestBrowserCloseManager);
 };
 
+class TestDownloadManagerDelegate : public ChromeDownloadManagerDelegate {
+ public:
+  explicit TestDownloadManagerDelegate(Profile* profile)
+      : ChromeDownloadManagerDelegate(profile) {

[asanka, 2013/10/19 00:42:31]

On ChromeOS, the DriveIntegrationService kicks in very early and pokes at the
DownloadManager causing ChromeDownloadManagerDelegate and the DownloadHistory
classes to be created and initialized. One bit that has changed recently is that
CDMD::SetNextId() needs to be called in order for CDMD to be able to handle new
downloads. This is done once by DownloadHistory.

By the time your test calls DownloadService::SetDownloadManagerForTesting(),
this has already happened on Chrome OS. So the new TestDownloadManagerDelegate
never gets SetNextId() called on it. This is likely why your download hangs.

You can just call "SetNextId(content::DownloadItem::kInvalidId + 1);" here in
the constructor so that this test delegate can handle downloads.

[felt, 2013/10/19 02:51:50]

Thank you for looking into this. That has fixed it!!! (The failures that remain
in the try jobs are unrelated to this change. All the relevant browsertests are
running green.)

+  }
+
+  virtual bool DetermineDownloadTarget(
+      content::DownloadItem* item,
+      const content::DownloadTargetCallback& callback) OVERRIDE {
+    content::DownloadTargetCallback dangerous_callback =
+        base::Bind(&TestDownloadManagerDelegate::SetDangerous, this, callback);
+    ChromeDownloadManagerDelegate::DetermineDownloadTarget(
+        item, dangerous_callback);

[asanka, 2013/10/19 00:42:31]

Nit: For extra resiliency:

return CDMD::DetermineDownloadTarget(...);

[felt, 2013/10/19 02:51:50]

Done.

+    return true;
+  }
+
+  void SetDangerous(
+      const content::DownloadTargetCallback& callback,
+      const base::FilePath& target_path,
+      content::DownloadItem::TargetDisposition disp,
+      content::DownloadDangerType danger_type,
+      const base::FilePath& intermediate_path) {
+    callback.Run(target_path,
+                 disp,
+                 content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL,
+                 intermediate_path);
+  }
+
+ private:
+  virtual ~TestDownloadManagerDelegate() {}
+};
+
 }  // namespace
 
 class BrowserCloseManagerBrowserTest
     : public InProcessBrowserTest,
       public testing::WithParamInterface<bool> {
  protected:
   virtual void SetUpOnMainThread() OVERRIDE {
     InProcessBrowserTest::SetUpOnMainThread();
     SessionStartupPref::SetStartupPref(
         browser()->profile(), SessionStartupPref(SessionStartupPref::LAST));
@@ skipping 443 matching lines @@
   chrome::CloseAllBrowsers();
   ASSERT_FALSE(browsers_[0]->ShouldCloseWindow());
   ASSERT_NO_FATAL_FAILURE(dialogs_.AcceptClose());
   ASSERT_NO_FATAL_FAILURE(dialogs_.AcceptClose());
 
   close_observer.Wait();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(chrome::BrowserIterator().done());
 }
 
+#if defined(CHROMEOS)
+// TODO(felt): figure out why this isn't working on CrOS.
+#define MAYBE_TestWithDangerousUrlDownload DISABLED_TestWithDangerousUrlDownload
+#else
+#define MAYBE_TestWithDangerousUrlDownload TestWithDangerousUrlDownload
+#endif
+// Test shutdown with a DANGEROUS_URL download undecided.
+IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
+    TestWithDangerousUrlDownload) {
+  ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+
+  // Set up the fake delegate that forces the download to be malicious.
+  scoped_refptr<TestDownloadManagerDelegate> test_delegate(
+      new TestDownloadManagerDelegate(browser()->profile()));
+  DownloadServiceFactory::GetForBrowserContext(browser()->profile())->
+      SetDownloadManagerDelegateForTesting(test_delegate.get());
+
+  // Run a dangerous download, but the user doesn't make a decision.
+  // This .swf normally would be categorized as DANGEROUS_HOST, but

[asanka, 2013/10/19 00:42:31]

Nit: DANGEROUS_FILE

[felt, 2013/10/19 02:51:50]

Done.

+  // TestDownloadManagerDelegate turns it into DANGEROUS_URL.
+  base::FilePath file(FILE_PATH_LITERAL("downloads/dangerous/dangerous.swf"));
+  GURL download_url(content::URLRequestMockHTTPJob::GetMockUrl(file));
+  content::DownloadTestObserverInterrupted observer(
+      content::BrowserContext::GetDownloadManager(browser()->profile()),
+      1,
+      content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_QUIT);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      GURL(download_url),
+      NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_NONE);
+  observer.WaitForFinished();
+
+  // Check that the download manager has the expected state.
+  EXPECT_EQ(1, content::BrowserContext::GetDownloadManager(
+      browser()->profile())->InProgressCount());
+  EXPECT_EQ(0, content::BrowserContext::GetDownloadManager(
+      browser()->profile())->NonMaliciousInProgressCount());
+
+  // Close the browser with no user action.
+  RepeatedNotificationObserver close_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSED, 1);
+  TestBrowserCloseManager::AttemptClose(
+      TestBrowserCloseManager::NO_USER_CHOICE);
+  close_observer.Wait();
+  EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
+  EXPECT_TRUE(chrome::BrowserIterator().done());
+}
+
 // Test shutdown with a download in progress.
 IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest, TestWithDownloads) {
   ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
   ASSERT_NO_FATAL_FAILURE(CreateStalledDownload(browser()));
   content::TestNavigationObserver navigation_observer(
       browser()->tab_strip_model()->GetActiveWebContents(), 1);
   TestBrowserCloseManager::AttemptClose(
       TestBrowserCloseManager::USER_CHOICE_USER_CANCELS_CLOSE);
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   navigation_observer.Wait();
@@ skipping 79 matching lines @@
   TestBrowserCloseManager::AttemptClose(
       TestBrowserCloseManager::USER_CHOICE_USER_ALLOWS_CLOSE);
   close_observer.Wait();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(chrome::BrowserIterator().done());
 }
 
 INSTANTIATE_TEST_CASE_P(BrowserCloseManagerBrowserTest,
                         BrowserCloseManagerBrowserTest,
                         testing::Bool());