Nicely handle OnBeforeUnloads with guest and lock mode.

chrome/browser/ui/browser_list.h

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_UI_BROWSER_LIST_H_
 #define CHROME_BROWSER_UI_BROWSER_LIST_H_
 
 #include <vector>
 
 #include "base/basictypes.h"
@@ skipping 54 matching lines @@
   static void AddObserver(chrome::BrowserListObserver* observer);
   static void RemoveObserver(chrome::BrowserListObserver* observer);
 
   // Called by Browser objects when their window is activated (focused).  This
   // allows us to determine what the last active Browser was on each desktop.
   // Note: This only takes effect on the appropriate browser list as determined
   // by |browser->host_desktop_type()|.
   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();
 
   // Returns true if at least one incognito session is active for |profile|
   // across all desktops.
   static bool IsOffTheRecordSessionActiveForProfile(Profile* profile);
 
  private:
   BrowserList();
   ~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
   // reverse order in which they were last activated.
   BrowserVector last_active_browsers_;
 
   // A list of observers which will be notified of every browser addition and
   // removal across all BrowserLists.
   static base::LazyInstance<ObserverList<chrome::BrowserListObserver> >::Leaky
       observers_;
 
   // Nothing fancy, since we only have two HDTs.
   static BrowserList* native_instance_;
   static BrowserList* ash_instance_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserList);
 };
 
 #endif  // CHROME_BROWSER_UI_BROWSER_LIST_H_