Don't prompt to save malicious downloads on exit

chrome/browser/ui/browser_close_browsertest.cc

 // Copyright (c) 2012 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 "base/command_line.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 237 matching lines @@
 
     DownloadTestFileActivityObserver(first_profile_) .EnableFileChooser(false);
     DownloadTestFileActivityObserver(second_profile_).EnableFileChooser(false);
     return true;
   }
 
   // Test a specific DownloadsCloseCheckCase.  Returns false if
   // an assertion has failed and the test should be aborted.
   bool ExecuteDownloadCloseCheckCase(size_t i) {
     const DownloadsCloseCheckCase& check_case(download_close_check_cases[i]);
+    SCOPED_TRACE(testing::Message() << "Case" << i
+        << ": " << check_case.DebugString());

[Peter Kasting, 2013/10/15 19:10:28]

Nit: <<s should line up; try

    SCOPED_TRACE(testing::Message() << "Case" << i << ": "
                                    << check_case.DebugString());

[felt, 2013/10/18 01:50:53]

Done.

 
     // Test invariant: so that we don't actually try and close the browser,
     // we always enter the function with a single browser window open on the
     // main profile.  That means we need to exit the function the same way.
     // So we setup everything except for the |first_profile_| regular, and then
     // flip the bit on the main window.
     // Note that this means that browser() is unreliable in the context
     // of this function or its callers; we'll be killing that main window
     // and recreating it fairly frequently.
     int unclosed_browsers = TotalUnclosedBrowsers();
     EXPECT_EQ(1, unclosed_browsers);
     if (1 != unclosed_browsers)
       return false;
 
     Browser* entry_browser = FirstUnclosedBrowser();
-    EXPECT_EQ(first_profile_, entry_browser->profile())
-        << "Case" << i
-        << ": " << check_case.DebugString();
+    EXPECT_EQ(first_profile_, entry_browser->profile());
     if (first_profile_ != entry_browser->profile())
       return false;

[Peter Kasting, 2013/10/15 19:10:28]

Don't EXPECT_EQ and then handle failure; if you truly need to abort the test on
failure, just use an ASSERT_EQ instead.  (Generally, you should only abort the
test if proceeding will result in a NULL deref or some other crash.)  (2 places)

[felt, 2013/10/18 01:50:53]

This isn't my test -- I am just tweaking someone else's test to use the new
method instead of the old method. rdsmith@ wrote the test and I trust him, so I
am wondering if there is some reason for it. I've pinged him to ask why he is
returning false instead of ASSERT_EQ-ing. I'll go back and refactor if he says
there's no reason.

-    int total_download_count = DownloadService::DownloadCountAllProfiles();
-    EXPECT_EQ(0, total_download_count)
-        << "Case " << i
-        << ": " << check_case.DebugString();
+    int total_download_count =
+        DownloadService::NonDangerousDownloadCountAllProfiles();
+    EXPECT_EQ(0, total_download_count);
     if (0 != total_download_count)
       return false;
-
     Profile* first_profile_incognito = first_profile_->GetOffTheRecordProfile();
     Profile* second_profile_incognito =
         second_profile_->GetOffTheRecordProfile();
     DownloadTestFileActivityObserver(first_profile_incognito)
         .EnableFileChooser(false);
     DownloadTestFileActivityObserver(second_profile_incognito)
         .EnableFileChooser(false);
 
     // For simplicty of coding, we create a window on each profile so that
     // we can easily create downloads, then we destroy or create windows
@@ skipping 54 matching lines @@
       return false;
 
     int num_downloads_blocking;
     Browser* browser_to_probe =
         (check_case.window_to_probe == DownloadsCloseCheckCase::REGULAR ?
          browser_a_regular :
          browser_a_incognito);
     Browser::DownloadClosePreventionType type =
         browser_to_probe->OkToCloseWithInProgressDownloads(
             &num_downloads_blocking);
-    EXPECT_EQ(check_case.type, type) << "Case " << i
-                                     << ": " << check_case.DebugString();
+    EXPECT_EQ(check_case.type, type);
     if (type != Browser::DOWNLOAD_CLOSE_OK)
-      EXPECT_EQ(check_case.num_blocking, num_downloads_blocking)
-          << "Case " << i
-          << ": " << check_case.DebugString();
+      EXPECT_EQ(check_case.num_blocking, num_downloads_blocking);
 
     // Release all the downloads.
     CompleteAllDownloads(browser_to_probe);
 
     // Create a new main window and kill everything else.
     entry_browser = CreateBrowserOnProfile(first_profile_, host_desktop_type);
     for (chrome::BrowserIterator it; !it.done(); it.Next()) {
       if ((*it) != entry_browser) {
         if (!it->window()) {
           ADD_FAILURE();
@@ skipping 190 matching lines @@
   }
 }
 
 IN_PROC_BROWSER_TEST_F(BrowserCloseTest, MAYBE_DownloadsCloseCheck_5) {
   ASSERT_TRUE(SetupForDownloadCloseCheck());
   for (size_t i = 5 * arraysize(download_close_check_cases) / 6;
        i < 6 * arraysize(download_close_check_cases) / 6; ++i) {
     ExecuteDownloadCloseCheckCase(i);
   }
 }