Fix warning prompting on closing a window that will cancel downloads.

chrome/browser/ui/browser_close_browsertest.cc

+// Copyright (c) 2011 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/logging.h"
+#include "base/path_service.h"
+#include "base/stringprintf.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/download/download_test_observer.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/webui/active_downloads_ui.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/browser/download/download_item.h"
+#include "content/browser/net/url_request_slow_download_job.h"
+#include "content/browser/tab_contents/tab_contents.h"
+#include "content/common/page_transition_types.h"
+#include "content/common/url_constants.h"
+
+class BrowserCloseTest : public InProcessBrowserTest {
+ public:
+  // Structure defining test cases for DownloadsCloseCheck.
+  struct DownloadsCloseCheckCase {
+    std::string DebugString() const;
+
+    // Input
+    struct {

[achuithb, 2011/08/29 17:19:10]

Isn't struct profile_a {} more readable than struct {} profile_a? 

[Randy Smith (Not in Mondays), 2011/10/13 20:06:04]

Sure, but it doesn't mean the same thing.  In the first, profile_a is a type
name.  In the second it's the name of the member variable of the containing
structure.

+      struct {
+        int windows;
+        int downloads;
+      } regular;
+      struct {
+        int windows;
+        int downloads;
+      } incognito;
+    } profile_a;
+
+    struct {
+      struct {
+        int windows;
+        int downloads;
+      } regular;
+      struct {
+        int windows;
+        int downloads;
+      } incognito;
+    } profile_b;
+
+    // We always probe a window in profile A.
+    enum { REGULAR = 0, INCOGNITO = 1 } window_to_probe;
+
+    // Output
+    Browser::DownloadClosePreventionType type;
+
+    // Unchecked if type == DOWNLOAD_CLOSE_OK.
+    int num_blocking;
+  };
+
+ protected:
+  // Create a second profile to work within multi-profile.
+  Profile* CreateSecondProfile() {
+    FilePath user_data_dir;
+    PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+
+    if (!second_profile_data_dir_.CreateUniqueTempDirUnderPath(user_data_dir))
+      return NULL;
+
+    return g_browser_process->profile_manager()->GetProfile(
+        second_profile_data_dir_.path());
+  }
+
+  // Create |num_downloads| number of downloads that are stalled
+  // (will quickly get to a place where the server won't
+  // provide any more data) so that we can test closing the
+  // browser with active downloads.
+  void CreateStalledDownloads(Browser* browser, int num_downloads) {
+    GURL url(URLRequestSlowDownloadJob::kKnownSizeUrl);
+
+    if (num_downloads == 0)
+      return;
+
+    // Setup an observer waiting for the given number of downloads
+    // to get to IN_PROGRESS.
+    DownloadManager* download_manager =
+        browser->profile()->GetDownloadManager();
+    scoped_ptr<DownloadTestObserver> observer(
+        new DownloadTestObserver(
+            download_manager, num_downloads,
+            DownloadItem::IN_PROGRESS,
+            true,                           // Bail on select file

[achuithb, 2011/08/29 17:19:10]

nits: 2 space between comment and code? period at end of comment?

[Randy Smith (Not in Mondays), 2011/10/13 20:06:04]

Done.

+            DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+
+    // Set of that number of downloads.
+    while (num_downloads--)
+      ui_test_utils::NavigateToURLWithDisposition(
+          browser, url, NEW_BACKGROUND_TAB,
+          ui_test_utils::BROWSER_TEST_NONE);
+
+    // Wait for them.
+    observer->WaitForFinished();
+  }
+
+  // All all downloads created in CreateStalledDownloads() to
+  // complete, and block in this routine until they do complete.
+  void CompleteAllDownloads(Browser* browser) {
+    GURL finish_url(URLRequestSlowDownloadJob::kFinishDownloadUrl);
+    ui_test_utils::NavigateToURL(browser, finish_url);
+
+    // Go through and, for every single profile, wait until there are
+    // no active downloads on that download manager.
+    std::vector<Profile*> profiles(
+        g_browser_process->profile_manager()->GetLoadedProfiles());
+    for (std::vector<Profile*>::const_iterator pit = profiles.begin();
+         pit != profiles.end(); ++pit) {
+      if ((*pit)->HasCreatedDownloadManager()) {
+        DownloadManager *mgr = (*pit)->GetDownloadManager();
+        scoped_refptr<DownloadTestFlushObserver> observer(
+            new DownloadTestFlushObserver(mgr));
+        observer->WaitForFlush();
+      }
+    }
+  }
+
+  // Create a Browser (with associated window) on the specified profile.
+  Browser* CreateBrowserOnProfile(Profile* profile) {
+    Browser* new_browser = Browser::Create(profile);
+    new_browser->AddSelectedTabWithURL(GURL(chrome::kAboutBlankURL),
+                                       PageTransition::START_PAGE);
+    ui_test_utils::WaitForNavigation(
+        &new_browser->GetSelectedTabContents()->controller());
+    new_browser->window()->Show();
+    return new_browser;
+  }
+
+  // Adjust the number of browsers and associated windows up or down
+  // to |num_windows|.  This routine assumes that there is only a single
+  // browser associated with the profile on entry.  |*base_browser| contains
+  // this browser, and the profile is derived from that browser.  On output,
+  // if |*base_browser| was destroyed (because |num_windows == 0|), NULL
+  // is assigned to that memory location.
+  bool AdjustBrowsersOnProfile(Browser** base_browser, int num_windows) {
+    int num_downloads_blocking;
+    if (num_windows == 0) {
+      if (Browser::DOWNLOAD_CLOSE_OK !=
+          (*base_browser)->OkToCloseWithInProgressDownloads(
+              &num_downloads_blocking))
+        return false;
+      (*base_browser)->window()->Close();
+      *base_browser = 0;
+      return true;
+    }
+
+    // num_windows > 0
+    Profile* profile((*base_browser)->profile());
+    for (int w = 1; w < num_windows; w++) {

[achuithb, 2011/08/29 17:19:10]

nit: ++w

[Randy Smith (Not in Mondays), 2011/10/13 20:06:04]

Done.

+      CreateBrowserOnProfile(profile);
+    }
+    return true;
+  }
+
+  // Test a specific DownloadsCloseCheckCase.  Returns false if
+  // an assertion has failed and the test should be aborted.
+  bool CheckDownloadsClose(const DownloadsCloseCheckCase& check_case,
+                           Profile* profile_a,
+                           Profile* profile_b) {
+    // 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 profile_a 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.
+
+#if 0

[achuithb, 2011/08/29 17:19:10]

I'm assuming you're working on fixing this?

[Randy Smith (Not in Mondays), 2011/10/13 20:06:04]

Yep.  It's gone now.

+#if !defined(OS_MACOSX)
+    // Window closes are delayed until the outer run loop on the mac.
+    EXPECT_EQ(1u, BrowserList::size()) << "Case: " << check_case.DebugString();
+    if (BrowserList::size() != 1u)
+      return false;
+#endif
+#endif
+
+    Browser* entry_browser = *BrowserList::begin();
+    EXPECT_EQ(profile_a, entry_browser->profile()) << "Case: "
+                                                   << check_case.DebugString();
+    if (profile_a != entry_browser->profile())
+      return false;
+    int total_download_count =
+        g_browser_process->profile_manager()->TotalDownloadCount();
+    EXPECT_EQ(0, g_browser_process->profile_manager()->TotalDownloadCount())
+        << "Case: " << check_case.DebugString();
+    if (0 != total_download_count)
+      return false;
+
+    Profile* profile_a_incognito = profile_a->GetOffTheRecordProfile();
+    Profile* profile_b_incognito = profile_b->GetOffTheRecordProfile();
+
+    // For simplicty of coding, we create a window on each profile so that
+    // we can easily create downloads, then we destroy or create windows
+    // as necessary.
+    Browser* browser_a_regular(CreateBrowserOnProfile(profile_a));
+    Browser* browser_a_incognito(CreateBrowserOnProfile(profile_a_incognito));
+    Browser* browser_b_regular(CreateBrowserOnProfile(profile_b));
+    Browser* browser_b_incognito(CreateBrowserOnProfile(profile_b_incognito));
+
+    // Kill our entry browser.
+    entry_browser->window()->Close();
+    entry_browser = NULL;
+
+    // Create all downloads needed.
+    CreateStalledDownloads(
+        browser_a_regular, check_case.profile_a.regular.downloads);
+    CreateStalledDownloads(
+        browser_a_incognito, check_case.profile_a.incognito.downloads);
+    CreateStalledDownloads(
+        browser_b_regular, check_case.profile_b.regular.downloads);
+    CreateStalledDownloads(
+        browser_b_incognito, check_case.profile_b.incognito.downloads);
+
+    // Adjust the windows
+    Browser** browsers[] = {
+      &browser_a_regular, &browser_a_incognito,
+      &browser_b_regular, &browser_b_incognito
+    };
+    int window_counts[] = {
+      check_case.profile_a.regular.windows,
+      check_case.profile_a.incognito.windows,
+      check_case.profile_b.regular.windows,
+      check_case.profile_b.incognito.windows,
+    };
+    for (size_t i = 0; i < arraysize(browsers); i++) {

[achuithb, 2011/08/29 17:19:10]

nit: ++i

[Randy Smith (Not in Mondays), 2011/10/13 20:06:04]

Done.

+      bool result = AdjustBrowsersOnProfile(browsers[i], window_counts[i]);
+      EXPECT_TRUE(result);
+      if (!result)
+        return false;
+    }
+
+#if defined(OS_CHROMEOS)
+    // Get rid of downloads panel on ChromeOS
+    Browser* panel = ActiveDownloadsUI::GetPopup();
+    if (panel)
+      panel->CloseWindow();
+    ui_test_utils::RunAllPendingInMessageLoop();
+#endif
+
+    // All that work, for this one little test.
+    EXPECT_TRUE((check_case.window_to_probe ==
+                 DownloadsCloseCheckCase::REGULAR) ||
+                (check_case.window_to_probe ==
+                 DownloadsCloseCheckCase::INCOGNITO));
+    if (!((check_case.window_to_probe ==
+           DownloadsCloseCheckCase::REGULAR) ||
+          (check_case.window_to_probe ==
+           DownloadsCloseCheckCase::INCOGNITO)))
+      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) << check_case.DebugString();
+    if (check_case.type != Browser::DOWNLOAD_CLOSE_OK)
+      EXPECT_EQ(check_case.num_blocking, num_downloads_blocking)
+          << check_case.DebugString();
+
+    // Release all the downloads.
+    CompleteAllDownloads(browser_to_probe);
+
+    // Create a new main window and kill everything else.
+    entry_browser = CreateBrowserOnProfile(profile_a);
+    for (BrowserList::const_iterator bit = BrowserList::begin();
+         bit != BrowserList::end(); ++bit) {
+      if ((*bit) != entry_browser) {
+        EXPECT_TRUE((*bit)->window());
+        if (!(*bit)->window())
+          return false;
+        (*bit)->window()->Close();
+      }
+    }
+    ui_test_utils::RunAllPendingInMessageLoop();
+
+    return true;
+  }
+
+  ScopedTempDir second_profile_data_dir_;
+};
+
+std::string BrowserCloseTest::DownloadsCloseCheckCase::DebugString() const {
+  std::string result;
+  result += "Case: {";
+  if (profile_a.regular.windows || profile_a.regular.downloads)
+    result += base::StringPrintf("Regular profile A: (%d w, %d d), ",
+                                 profile_a.regular.windows,
+                                 profile_a.regular.downloads);
+  if (profile_a.incognito.windows || profile_a.incognito.downloads)
+    result += base::StringPrintf("Incognito profile A: (%d w, %d d), ",
+                                 profile_a.incognito.windows,
+                                 profile_a.incognito.downloads);
+  if (profile_b.regular.windows || profile_b.regular.downloads)
+    result += base::StringPrintf("Regular profile B: (%d w, %d d), ",
+                                 profile_b.regular.windows,
+                                 profile_b.regular.downloads);
+  if (profile_b.incognito.windows || profile_b.incognito.downloads)
+    result += base::StringPrintf("Incognito profile B: (%d w, %d d), ",
+                                 profile_b.incognito.windows,
+                                 profile_b.incognito.downloads);
+  result += (window_to_probe == REGULAR ? "Probe regular" :
+             window_to_probe == INCOGNITO ? "Probe incognito" :
+             "Probe unknown");
+  result += "} -> ";
+  if (type == Browser::DOWNLOAD_CLOSE_OK) {
+    result += "No warning";
+  } else {
+    result += base::StringPrintf(
+        "%s (%d downloads) warning",
+        (type == Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN ? "Browser shutdown" :
+         type == Browser::DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE ?
+         "Incognito close" : "Unknown"),
+        num_blocking);
+  }
+  return result;
+}
+
+static const BrowserCloseTest::DownloadsCloseCheckCase
+download_close_check_cases[] = {
+  // Top level nesting is {profile_a, profile_b}
+  // Second level nesting is {regular, incognito
+  // Third level (inner) nesting is {windows, downloads}
+
+  // Last window (incognito) triggers browser close warning.
+  {{{0, 0}, {1, 1}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO,
+   Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 1},
+
+  // Last incognito window triggers incognito close warning.
+  {{{1, 0}, {1, 1}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO,
+   Browser::DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE, 1},
+
+  // Last incognito window with no downloads triggers no warning.
+  {{{0, 0}, {1, 0}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Last incognito window with window+download on another incognito profile
+  // triggers no warning.
+  {{{0, 0}, {1, 0}}, {{0, 0}, {1, 1}},
+   BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Non-last incognito window triggers no warning.
+  {{{0, 0}, {2, 1}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Non-last regular window triggers no warning.
+  {{{2, 1}, {0, 0}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Last regular window triggers browser close.
+  {{{1, 1}, {0, 0}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 1},
+
+  // Last regular window triggers browser close for download on different
+  // profile.
+  {{{1, 0}, {0, 0}}, {{0, 1}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 1},
+
+  // Last regular window triggers no warning if incognito
+  // active (http://crbug.com/61257).
+  {{{1, 0}, {1, 1}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Last regular window triggers no warning if other profile window active.
+  {{{1, 1}, {0, 0}}, {{1, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Last regular window triggers no warning if other incognito window
+  // active.
+  {{{1, 0}, {0, 0}}, {{0, 0}, {1, 1}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Last regular window triggers no warning if incognito active.
+  {{{1, 1}, {1, 0}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_OK},
+
+  // Test plural for regular.
+  {{{1, 2}, {0, 0}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::REGULAR,
+   Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 2},
+
+  // Test plural for incognito.
+  {{{1, 0}, {1, 2}}, {{0, 0}, {0, 0}},
+   BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO,
+   Browser::DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE, 2},
+};
+
+
+// The following test is split into three chunks to reduce the chance
+// of hitting the 25s timeout.
+
+IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DownloadsCloseCheck_0) {
+  Profile* profile_a = browser()->profile();

[achuithb, 2011/08/29 17:19:10]

Any reason to not pull out the boiler-plate in DownloadsCloseCheck_* into a
separate function?

[Randy Smith (Not in Mondays), 2011/10/13 20:06:04]

Nope; hadn't gotten to it.  Done.

+  Profile* profile_b = CreateSecondProfile();
+  ASSERT_TRUE(profile_b);
+
+  ScopedTempDir profile_a_dir;
+  ASSERT_TRUE(profile_a_dir.CreateUniqueTempDir());
+  profile_a->GetPrefs()->SetFilePath(
+      prefs::kDownloadDefaultDirectory,
+      profile_a_dir.path());
+  ScopedTempDir profile_b_dir;
+  ASSERT_TRUE(profile_b_dir.CreateUniqueTempDir());
+  profile_b->GetPrefs()->SetFilePath(
+      prefs::kDownloadDefaultDirectory,
+      profile_b_dir.path());
+
+  for (size_t i = 0; i < arraysize(download_close_check_cases) / 3; ++i) {
+    const DownloadsCloseCheckCase& check_case(download_close_check_cases[i]);
+    ASSERT_TRUE(CheckDownloadsClose(check_case, profile_a, profile_b));
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DownloadsCloseCheck_1) {
+  Profile* profile_a = browser()->profile();
+  Profile* profile_b = CreateSecondProfile();
+  ASSERT_TRUE(profile_b);
+
+  ScopedTempDir profile_a_dir;
+  ASSERT_TRUE(profile_a_dir.CreateUniqueTempDir());
+  profile_a->GetPrefs()->SetFilePath(
+      prefs::kDownloadDefaultDirectory,
+      profile_a_dir.path());
+  ScopedTempDir profile_b_dir;
+  ASSERT_TRUE(profile_b_dir.CreateUniqueTempDir());
+  profile_b->GetPrefs()->SetFilePath(
+      prefs::kDownloadDefaultDirectory,
+      profile_b_dir.path());
+
+  for (size_t i = arraysize(download_close_check_cases) / 3;
+       i < 2 * arraysize(download_close_check_cases) / 3; ++i) {
+    const DownloadsCloseCheckCase& check_case(download_close_check_cases[i]);
+    ASSERT_TRUE(CheckDownloadsClose(check_case, profile_a, profile_b));
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DownloadsCloseCheck_2) {
+  Profile* profile_a = browser()->profile();
+  Profile* profile_b = CreateSecondProfile();
+  ASSERT_TRUE(profile_b);
+
+  ScopedTempDir profile_a_dir;
+  ASSERT_TRUE(profile_a_dir.CreateUniqueTempDir());
+  profile_a->GetPrefs()->SetFilePath(
+      prefs::kDownloadDefaultDirectory,
+      profile_a_dir.path());
+  ScopedTempDir profile_b_dir;
+  ASSERT_TRUE(profile_b_dir.CreateUniqueTempDir());
+  profile_b->GetPrefs()->SetFilePath(
+      prefs::kDownloadDefaultDirectory,
+      profile_b_dir.path());
+
+  for (size_t i = 2 * arraysize(download_close_check_cases) / 3;
+       i < arraysize(download_close_check_cases); ++i) {
+    const DownloadsCloseCheckCase& check_case(download_close_check_cases[i]);
+    ASSERT_TRUE(CheckDownloadsClose(check_case, profile_a, profile_b));
+  }
+}