Make UnloadController::on_close_confirmed_ explicitly RepeatingCallback

Unintuitively, it seems this can be called more than once if a close is
confirmed by the user but later aborted.

Bug: 1152284
Change-Id: I89d45ae0eadf3248fccd983406fe0dc5d050afd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570332Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833425}

chrome/browser/lifetime/browser_close_manager.cc

@@ -74,8 +74,8 @@ void BrowserCloseManager::TryToCloseBrowsers() {
   // this will trigger TryToCloseBrowsers to try again.
   for (auto* browser : *BrowserList::GetInstance()) {
     if (browser->TryToCloseWindow(
-            false,
-            base::Bind(&BrowserCloseManager::OnBrowserReportCloseable, this))) {
+            false, base::BindRepeating(
+                       &BrowserCloseManager::OnBrowserReportCloseable, this))) {
       current_browser_ = browser;
       return;
     }

chrome/browser/ui/browser.cc

@@ -861,7 +861,7 @@ bool Browser::ShouldCloseWindow() {
 
 bool Browser::TryToCloseWindow(
     bool skip_beforeunload,
-    const base::Callback<void(bool)>& on_close_confirmed) {
+    const base::RepeatingCallback<void(bool)>& on_close_confirmed) {
   cancel_download_confirmation_state_ = RESPONSE_RECEIVED;
   return unload_controller_.TryToCloseWindow(skip_beforeunload,
                                              on_close_confirmed);

chrome/browser/ui/browser.h

@@ -460,8 +460,9 @@ class Browser : public TabStripModelObserver,
   // Note that if the browser window has been used before, users should always
   // have a chance to save their work before the window is closed without
   // triggering beforeunload event.
-  bool TryToCloseWindow(bool skip_beforeunload,
-                        const base::Callback<void(bool)>& on_close_confirmed);
+  bool TryToCloseWindow(
+      bool skip_beforeunload,
+      const base::RepeatingCallback<void(bool)>& on_close_confirmed);
 
   // Clears the results of any beforeunload confirmation dialogs triggered by a
   // TryToCloseWindow call.

chrome/browser/ui/unload_controller.cc

@@ -166,7 +166,7 @@ bool UnloadController::ShouldCloseWindow() {
 
 bool UnloadController::TryToCloseWindow(
     bool skip_beforeunload,
-    const base::Callback<void(bool)>& on_close_confirmed) {
+    const base::RepeatingCallback<void(bool)>& on_close_confirmed) {
   // The devtools browser gets its beforeunload events as the results of
   // intercepting events from the inspected tab, so don't send them here as
   // well.
@@ -215,11 +215,8 @@ void UnloadController::CancelWindowClose() {
     DevToolsWindow::OnPageCloseCanceled(*it);
   }
   tabs_needing_unload_fired_.clear();
-  if (is_calling_before_unload_handlers()) {
-    base::Callback<void(bool)> on_close_confirmed = on_close_confirmed_;
-    on_close_confirmed_.Reset();
-    on_close_confirmed.Run(false);
-  }
+  if (is_calling_before_unload_handlers())
+    std::move(on_close_confirmed_).Run(false);
   is_attempting_to_close_browser_ = false;
 
   content::NotificationService::current()->Notify(
@@ -334,7 +331,8 @@ void UnloadController::ProcessPendingTabs(bool skip_beforeunload) {
       ClearUnloadState(web_contents, true);
     }
   } else if (is_calling_before_unload_handlers()) {
-    base::Callback<void(bool)> on_close_confirmed = on_close_confirmed_;
+    base::RepeatingCallback<void(bool)> on_close_confirmed =
+        on_close_confirmed_;
     // Reset |on_close_confirmed_| in case the callback tests
     // |is_calling_before_unload_handlers()|, we want to return that calling
     // is complete.

chrome/browser/ui/unload_controller.h

@@ -63,8 +63,9 @@ class UnloadController : public content::NotificationObserver,
   // Begins the process of confirming whether the associated browser can be
   // closed. Beforeunload events won't be fired if |skip_beforeunload|
   // is true.
-  bool TryToCloseWindow(bool skip_beforeunload,
-                        const base::Callback<void(bool)>& on_close_confirmed);
+  bool TryToCloseWindow(
+      bool skip_beforeunload,
+      const base::RepeatingCallback<void(bool)>& on_close_confirmed);
 
   // Clears the results of any beforeunload confirmation dialogs triggered by a
   // TryToCloseWindow call.
@@ -147,8 +148,13 @@ class UnloadController : public content::NotificationObserver,
 
   // A callback to call to report whether the user chose to close all tabs of
   // |browser_| that have beforeunload event handlers. This is set only if we
-  // are currently confirming that the browser is closable.
-  base::Callback<void(bool)> on_close_confirmed_;
+  // are currently confirming that the browser is closable. This can be called
+  // more than once if a user confirms all the beforeunload prompts (at which
+  // point it will be called with true) but the window close is later aborted
+  // (at which point it will be called with false). This can happen when
+  // multiple browser windows are being closed together. See
+  // BrowserList::TryToCloseBrowserList.
+  base::RepeatingCallback<void(bool)> on_close_confirmed_;
 
   base::WeakPtrFactory<UnloadController> weak_factory_{this};