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 257 matching lines @@
     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();
     if (first_profile_ != entry_browser->profile())
       return false;
+    int nondangerous_download_count =
+        DownloadService::NonDangerousDownloadCountAllProfiles();
+    EXPECT_EQ(0, nondangerous_download_count)
+        << "Case " << i
+        << ": " << check_case.DebugString();

[asanka, 2013/10/11 16:02:35]

Minor nit: Adding a SCOPED_TRACE at the top of the function block would be
better than generating these debug strings everywhere. It could look like this:

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

See
https://code.google.com/p/chromium/codesearch#chromium/src/testing/gtest/incl...

[felt, 2013/10/11 17:26:44]

I assume you mean for the whole file, not just for the few lines I touched?
Newest patchset refactors the whole test.

On 2013/10/11 16:02:35, asanka wrote:

[asanka, 2013/10/11 19:09:42]

Yup. However, my suggestion seems to have been misunderstood :)

The SCOPED_TRACE() is only needed at the top of the function block. So you can
avoid all the message generation everywhere else in the method. In other words:

bool ExecuteDownloadCloseCheckCase(size_t i) {
  SCOPED_TRACE(testing::Message() << "Case " << ... );

  ...
  EXPECT_EQ(first_profile_, ...);
  // Note: no message generation here.
  ...
  EXPECT_EQ(0, total_download_count);
  // Or here...
  ...
  ...
  EXPECT_...;
  // Or here.
  ...
}

If any of EXPECT_foo within the scope of the SCOPED_TRACE() fails, then it will
log the message. It won't log anything if all the expectations succeed.

[felt, 2013/10/11 23:41:10]

Done.

+    if (0 != nondangerous_download_count)
+      return false;
     int total_download_count = DownloadService::DownloadCountAllProfiles();
     EXPECT_EQ(0, total_download_count)
         << "Case " << i
         << ": " << check_case.DebugString();
     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 270 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);
   }
 }