Nicely handle OnBeforeUnloads with guest and lock mode.

chrome/browser/ui/browser_list.h

Index: chrome/browser/ui/browser_list.h
diff --git a/chrome/browser/ui/browser_list.h b/chrome/browser/ui/browser_list.h
index 11a736b12959f7a1b4b6eedae32f8ab215492767..65266f60eac73fd690691c919ff01686a230cf95 100644
--- a/chrome/browser/ui/browser_list.h
+++ b/chrome/browser/ui/browser_list.h
@@ -72,8 +72,16 @@ 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. If an OnBeforeUnload
+  // event causes any browser to not be closed, |on_close_aborted| is called.

[Peter Kasting, 2014/08/28 18:46:13]

This comment is now out-of-date.

[Mike Lerman, 2014/08/29 18:02:21]

Done.

+  // Otherwise, |on_close_success| is called.
+  static void CloseAllBrowsersWithProfile(
+      Profile* profile, const base::Callback<void(size_t)>& on_close_success);

[Peter Kasting, 2014/08/28 18:46:12]

Nit: One arg per line

[Mike Lerman, 2014/08/29 18:02:21]

Done.

+
   // Returns true if at least one incognito session is active across all
   // desktops.
   static bool IsOffTheRecordSessionActive();
@@ -89,6 +97,25 @@ class BrowserList {
   // Helper method to remove a browser instance from a list of browsers
   static void RemoveBrowserFrom(Browser* browser, BrowserVector* browser_list);
 
+  // Attempt to close |browser_to_close| while respecting OnBeforeUnload events.

[Peter Kasting, 2014/08/28 18:46:13]

Nit: Attempt -> Attempts; browser -> browsers

[Mike Lerman, 2014/08/29 18:02:21]

Done.

+  // If successful |on_close_confirmed| will be called, with a parameter of
+  // |profile_index|.
+  static void TryToCloseBrowserList(
+      const BrowserVector& browsers_to_close,
+      const base::Callback<void(size_t)>& on_close_confirmed,

[Peter Kasting, 2014/08/28 18:46:12]

Nit: Maybe rename this arg |on_close_completed| or |on_close_succeeded| or
|on_close_success|?  Make the args to CloseAllBrowsersWithProfile() and
PostBeforeUnloadHandlers() match the name

[Mike Lerman, 2014/08/29 18:02:21]

Done.

+      const size_t profile_index);

[Peter Kasting, 2014/08/28 18:46:12]

Nit: Pass by const value is unusual; probably should just pass by value
(multiple places)

[Mike Lerman, 2014/08/29 18:02:21]

This parameter has changed from a size_t to a base::FilePath& (as par noms's
comment), so I'll keep it as a const base::FilePath&.

+
+  // Called after handling an OnBeforeUnload event. |proceed| indicates if the
+  // user responded that closing the browser tab should proceed or not. If so,

[Peter Kasting, 2014/08/28 18:46:12]

Nit: Remove "or not" (makes the sentence technically mean something other than
what you intended)

[Mike Lerman, 2014/08/29 18:02:21]

Done.

+  // call |TryToCloseBrowserList|, passing the parameters |browsers_to_close|,

[Peter Kasting, 2014/08/28 18:46:12]

Nit: call -> calls

[Mike Lerman, 2014/08/29 18:02:21]

Done.

+  // |on_close_confirmed|, and |profile_index|. Otherwise, reset all the

[Peter Kasting, 2014/08/28 18:46:12]

Nit: reset -> resets (although perhaps this should be "restores"?)

[Mike Lerman, 2014/08/29 18:02:21]

Done. (the method called is ResetBeforeUnloadHandlers(), so I'll keep reset). 

+  // OnBeforeUnload events.

[Peter Kasting, 2014/08/28 18:46:12]

Nit: events -> handlers?  (You can't reset an event, can you?)

[Mike Lerman, 2014/08/29 18:02:21]

How about "event handlers"? It's really the event that's being handled, and the
handlers that's being reset.

+  static void PostBeforeUnloadHandlers(
+      const BrowserVector& browsers_to_close,
+      const base::Callback<void(size_t)>& on_close_confirmed,
+      const size_t profile_index,
+      bool proceed);
+
   // 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