Fix Download In Progress prompt on browser close (regression)

Before reviewing, please read this brief history of the is_attempting_to_close_browser_ branch.

https://chromiumcodereview.appspot.com/10749002
12 months ago by Ben Goodger
Before this CL, is_attempting_to_close_browser_ was set to true in ShouldCloseWindow() *after* CanCloseWithInProgressDownloads() was called, so that the `if (is_attempting_to_close_browser_)return...` branch in OkToCloseWithInProgressDownloads() would only be taken while the Download In Progress dialog is displaying.
After this CL, UnloadController::is_attempting_to_close_browser_ is set to true in TabStripEmpty(), which is called *before* ShouldCloseWindow(), so that the is_attempting_to_close_browser_ branch in OkToCloseWithInProgressDownloads() would *always* (or perhaps just more often?) be taken.

https://codereview.chromium.org/7466033
23 months ago by Randy
This CL moved the `if (is_attempting_to_close_browser_) return...` branch from CanCloseWithInProgressDownloads() to its current location in OkToCloseWithInProgressDownloads(). This branch was not documented either before or after this CL.

https://codereview.chromium.org/1654011
39 months ago by georgey
This CL adds the is_attempting_to_close_browser_ branch to CanCloseWithInProgressDownloads(). In this revision, that flag is only true between ShouldCloseWindow() (after CanCloseWithInProgressDownloads() has a chance to prevent the window from closing) and CancelWindowClose(), so it seems that the intended purpose of the branch is to make CanCloseWithInProgressDownloads() return true while the Download In Progress dialog is displaying.

Why should CanCloseWithInProgressDownloads() return true while the Download In Progress dialog is displaying? How could it be called, if there's a modal dialog preventing the user from interacting with the browser? Even if it can be called from javascript or somewhere else, wouldn't it be acceptable to close the browser if all downloads are complete without waiting for the user to deal with the dialog?

I'm happy to find another way to make OkToCloseWithInProgressDownloads() return OK while the Download In Progress dialog is displaying if there's a reason for it. This might be a liberal interpretation of the 'when in doubt, rip it out' philosophy, but I don't think it is.

PS1@r211430

BUG=153294
R=asanka@chromium.org, ben@chromium.org, rdsmith@chromium.org

Review URL: https://codereview.chromium.org/19209002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212403 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/browser.cc

@@ -679,9 +679,6 @@ Browser::DownloadClosePreventionType Browser::OkToCloseWithInProgressDownloads(
   DCHECK(num_downloads_blocking);
   *num_downloads_blocking = 0;
 
-  if (IsAttemptingToCloseBrowser())
-    return DOWNLOAD_CLOSE_OK;
-
   // If we're not running a full browser process with a profile manager
   // (testing), it's ok to close the browser.
   if (!g_browser_process->profile_manager())
@@ -720,7 +717,8 @@ Browser::DownloadClosePreventionType Browser::OkToCloseWithInProgressDownloads(
   // those downloads would be cancelled by our window (-> profile) close.
   DownloadService* download_service =
       DownloadServiceFactory::GetForProfile(profile());
-  if (profile_window_count == 0 && download_service->DownloadCount() > 0 &&
+  if ((profile_window_count == 0) &&
+      (download_service->DownloadCount() > 0) &&
       profile()->IsOffTheRecord()) {
     *num_downloads_blocking = download_service->DownloadCount();
     return DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE;