Don't prompt to save malicious downloads on exit

chrome/browser/ui/browser_close_browsertest.cc

Index: chrome/browser/ui/browser_close_browsertest.cc
diff --git a/chrome/browser/ui/browser_close_browsertest.cc b/chrome/browser/ui/browser_close_browsertest.cc
index 4f2b14503671a1d1a37b75478829cea3db9a3379..7dbd19d97b206ce76d386af825731bb809a1347b 100644
--- a/chrome/browser/ui/browser_close_browsertest.cc
+++ b/chrome/browser/ui/browser_close_browsertest.cc
@@ -255,6 +255,8 @@ class BrowserCloseTest : public InProcessBrowserTest {
   // 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
@@ -270,18 +272,14 @@ class BrowserCloseTest : public InProcessBrowserTest {
       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();
@@ -356,12 +354,9 @@ class BrowserCloseTest : public InProcessBrowserTest {
     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);