The Lock and Guest functions currently try to close all browser windows for...

The Lock and Guest functions currently try to close all browser windows for their profiles, and any OnBeforeUnload dialogs can cause that closing to fail. This CL takes inspiration from the BrowserCloseManager that is used when an entire browser window is closed.

Methods are added to BrowserList that will trigger all OnBeforeUpload handlers, wait for the user's response, and then close the profile's browsers and perform an appropriate success callback. A abort callback is permitted and will be used when ProfileDeletion also uses this flow.

Guest and Lock will now only open the User Manager (and lock the profile, for Lock) if no OnBeforeUnload event is canceled.

BUG=368497, 289390
TEST=Open a guest profile. Navigate to http://www.4guysfromrolla.com/demos/OnBeforeUnloadDemo1.htm. Click "Exit Guest" in the User Menu. Select "Stay on this page". The window should stay open and the User Manager should not be shown.
Open a signed-in and lockable profile. Navigate to the above URL. Lock the profile from the User Menu. Select "Stay on this page". The User Manager should not be shown. Manually opening the User Manager should show that the Profile has not been locked.

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

Cr-Commit-Position: refs/heads/master@{#293535}

chrome/browser/profiles/profile_window.cc

@@ -259,25 +259,36 @@ void CreateAndSwitchToNewProfile(chrome::HostDesktopType desktop_type,
   ProfileMetrics::LogProfileAddNewUser(metric);
 }
 
+void GuestBrowserCloseSuccess(const base::FilePath& profile_path) {
+  chrome::ShowUserManager(profile_path);
+}
+
 void CloseGuestProfileWindows() {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   Profile* profile = profile_manager->GetProfileByPath(
       ProfileManager::GetGuestProfilePath());
 
   if (profile) {
-    BrowserList::CloseAllBrowsersWithProfile(profile);
+    BrowserList::CloseAllBrowsersWithProfile(
+        profile, base::Bind(&GuestBrowserCloseSuccess));
   }
 }
 
+void LockBrowserCloseSuccess(const base::FilePath& profile_path) {
+  ProfileInfoCache* cache =
+      &g_browser_process->profile_manager()->GetProfileInfoCache();
+
+  cache->SetProfileSigninRequiredAtIndex(
+      cache->GetIndexOfProfileWithPath(profile_path), true);
+  chrome::ShowUserManager(profile_path);
+}
+
 void LockProfile(Profile* profile) {
   DCHECK(profile);
-  ProfileInfoCache& cache =
-      g_browser_process->profile_manager()->GetProfileInfoCache();
-
-  size_t index = cache.GetIndexOfProfileWithPath(profile->GetPath());
-  cache.SetProfileSigninRequiredAtIndex(index, true);
-  chrome::ShowUserManager(profile->GetPath());
-  BrowserList::CloseAllBrowsersWithProfile(profile);
+  if (profile) {
+    BrowserList::CloseAllBrowsersWithProfile(
+        profile, base::Bind(&LockBrowserCloseSuccess));
+  }
 }
 
 void CreateGuestProfileForUserManager(

chrome/browser/ui/browser_list.cc

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "base/auto_reset.h"
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
@@ -116,6 +117,7 @@ void BrowserList::RemoveObserver(chrome::BrowserListObserver* observer) {
   observers_.Get().RemoveObserver(observer);
 }
 
+// static
 void BrowserList::CloseAllBrowsersWithProfile(Profile* profile) {
   BrowserVector browsers_to_close;
   for (chrome::BrowserIterator it; !it.done(); it.Next()) {
@@ -129,6 +131,64 @@ void BrowserList::CloseAllBrowsersWithProfile(Profile* profile) {
   }
 }
 
+// static
+void BrowserList::CloseAllBrowsersWithProfile(Profile* profile,
+    const base::Callback<void(const base::FilePath&)>& on_close_success) {
+  BrowserVector browsers_to_close;
+  for (chrome::BrowserIterator it; !it.done(); it.Next()) {
+    if (it->profile()->GetOriginalProfile() == profile->GetOriginalProfile())
+      browsers_to_close.push_back(*it);
+  }
+
+  TryToCloseBrowserList(browsers_to_close,
+                        on_close_success,
+                        profile->GetPath());
+}
+
+// static
+void BrowserList::TryToCloseBrowserList(const BrowserVector& browsers_to_close,
+    const base::Callback<void(const base::FilePath&)>& on_close_success,
+    const base::FilePath& profile_path) {
+  for (BrowserVector::const_iterator it = browsers_to_close.begin();
+       it != browsers_to_close.end(); ++it) {
+    if ((*it)->CallBeforeUnloadHandlers(
+            base::Bind(&BrowserList::PostBeforeUnloadHandlers,
+                       browsers_to_close,
+                       on_close_success,
+                       profile_path))) {
+      return;
+    }
+  }
+
+  on_close_success.Run(profile_path);
+
+  for (BrowserVector::const_iterator it = browsers_to_close.begin();
+       it != browsers_to_close.end(); ++it)
+    (*it)->window()->Close();
+}
+
+// static
+void BrowserList::PostBeforeUnloadHandlers(
+    const BrowserVector& browsers_to_close,
+    const base::Callback<void(const base::FilePath&)>& on_close_success,
+    const base::FilePath& profile_path,
+    bool tab_close_confirmed) {
+  // We need this bool to avoid infinite recursion when resetting the
+  // BeforeUnload handlers, since doing that will trigger calls back to this
+  // method for each affected window.
+  static bool resetting_handlers = false;
+
+  if (tab_close_confirmed) {
+    TryToCloseBrowserList(browsers_to_close, on_close_success, profile_path);
+  } else if (!resetting_handlers) {
+    base::AutoReset<bool> resetting_handlers_scoper(&resetting_handlers, true);
+    for (BrowserVector::const_iterator it = browsers_to_close.begin();
+         it != browsers_to_close.end(); ++it) {
+      (*it)->ResetBeforeUnloadHandlers();
+    }
+  }
+}
+
 // static
 void BrowserList::SetLastActive(Browser* browser) {
   content::RecordAction(UserMetricsAction("ActiveBrowserChanged"));

chrome/browser/ui/browser_list.h

@@ -15,6 +15,10 @@
 class Browser;
 class Profile;
 
+namespace base {
+class FilePath;
+}
+
 namespace chrome {
 class BrowserListObserver;
 }
@@ -72,8 +76,18 @@ class BrowserList {
   static void SetLastActive(Browser* browser);
 
   // Closes all browsers for |profile| across all desktops.
+  // TODO(mlerman): Move the Profile Deletion flow to use the overloaded
+  // version of this method with a callback, then remove this method.
   static void CloseAllBrowsersWithProfile(Profile* profile);
 
+  // Closes all browsers for |profile| across all desktops. Uses
+  // TryToCloseBrowserList() to do the actual closing and trigger any
+  // OnBeforeUnload events. If all OnBeforeUnload events are confirmed,
+  // |on_close_success| is called.
+  static void CloseAllBrowsersWithProfile(
+      Profile* profile,
+      const base::Callback<void(const base::FilePath&)>& on_close_success);
+
   // Returns true if at least one incognito session is active across all
   // desktops.
   static bool IsOffTheRecordSessionActive();
@@ -89,6 +103,28 @@ class BrowserList {
   // Helper method to remove a browser instance from a list of browsers
   static void RemoveBrowserFrom(Browser* browser, BrowserVector* browser_list);
 
+  // Attempts to close |browsers_to_close| while respecting OnBeforeUnload
+  // events. If there are no OnBeforeUnload events to be called,
+  // |on_close_confirmed| will be called, with a parameter of |profile_path|,
+  // and the Browsers will then be closed. If at least one unfired
+  // OnBeforeUnload event is found, handle it with a callback to
+  // PostBeforeUnloadHandlers, which upon success will recursively call this
+  // method to handle any other OnBeforeUnload events.
+  static void TryToCloseBrowserList(
+      const BrowserVector& browsers_to_close,
+      const base::Callback<void(const base::FilePath&)>& on_close_success,
+      const base::FilePath& profile_path);
+
+  // Called after handling an OnBeforeUnload event. If |tab_close_confirmed| is
+  // true, calls |TryToCloseBrowserList()|, passing the parameters
+  // |browsers_to_close|, |on_close_confirmed|, and |profile_path|. Otherwise,
+  // resets all the OnBeforeUnload event handlers.
+  static void PostBeforeUnloadHandlers(
+      const BrowserVector& browsers_to_close,
+      const base::Callback<void(const base::FilePath&)>& on_close_success,
+      const base::FilePath& profile_path,
+      bool tab_close_confirmed);
+
   // A vector of the browsers in this list, in the order they were added.
   BrowserVector browsers_;
   // A vector of the browsers in this list that have been activated, in the

chrome/browser/ui/unload_controller.cc

@@ -300,7 +300,13 @@ void UnloadController::ProcessPendingTabs() {
       ClearUnloadState(web_contents, true);
     }
   } else if (is_calling_before_unload_handlers()) {
-    on_close_confirmed_.Run(true);
+    base::Callback<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.
+    if (tabs_needing_unload_fired_.empty())
+      on_close_confirmed_.Reset();
+    on_close_confirmed.Run(true);
   } else if (!tabs_needing_unload_fired_.empty()) {
     // We've finished firing all beforeunload events and can proceed with unload
     // events.

chrome/browser/ui/views/profiles/profile_chooser_view.cc

@@ -706,13 +706,11 @@ void ProfileChooserView::ButtonPressed(views::Button* sender,
     sender->SetEnabled(false);
 
   if (sender == users_button_) {
-    // If this is a guest session, also close all the guest browser windows.
-    if (browser_->profile()->IsGuestSession()) {
-      chrome::ShowUserManager(base::FilePath());
+    // If this is a guest session, close all the guest browser windows.
+    if (browser_->profile()->IsGuestSession())
       profiles::CloseGuestProfileWindows();
-    } else {
+    else
       chrome::ShowUserManager(browser_->profile()->GetPath());
-    }
     PostActionPerformed(ProfileMetrics::PROFILE_DESKTOP_MENU_OPEN_USER_MANAGER);
   } else if (sender == go_incognito_button_) {
     DCHECK(ShouldShowGoIncognito());