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 56 matching lines @@
 
   // 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.
   static void CloseAllBrowsersWithProfile(Profile* profile);
 
+  // Closes all browsers for |profile| across all desktops. If an OnBeforeUnload

[noms (inactive), 2014/08/26 20:17:32]

nit: maybe add "event"? 

[Mike Lerman, 2014/08/27 14:11:39]

Done.

+  // causes any browser to not be closed, |on_close_aborted| is called.
+  // Otherwise, |on_close_success| is called.
+  static void CloseAllBrowsersWithProfile(Profile* profile,

[Peter Kasting, 2014/08/26 20:26:15]

Nit: Align all args.  (Several places)

[Mike Lerman, 2014/08/27 14:11:39]

Done (several places).

+      const base::Callback<void(size_t)>& on_close_success,
+      const base::Callback<void(size_t)>& on_close_aborted);
+
   // 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);
 
+  // Null operation method for use with CloseAllBrowsersWithProfile.
+  static void DoNothing(size_t var) {}

[noms (inactive), 2014/08/26 20:17:32]

Hmm, I'm not super keen on this going in the public interface. Perhaps the
function caller should be the one to define it locally? Though I'm looking at
where you call CloseAllBrowsersWithProfile, and you always pass DoNothing for
on_close_aborted: maybe then this parameter isn't needed at all?

[Peter Kasting, 2014/08/26 20:26:15]

Yeah, don't define something like this in the class API.  If you don't use the
parameter, just remove it entirely.

[Mike Lerman, 2014/08/27 14:11:38]

I'll remove the on_close_aborted parameter of CloseAllBrowsersWithProfile until
I'm sure I'll need it, when I use that new method for Profile Deletion.
That way, I won't need this placeholder method at all.

+
  private:
   BrowserList();
   ~BrowserList();
 
   // Helper method to remove a browser instance from a list of browsers
   static void RemoveBrowserFrom(Browser* browser, BrowserVector* browser_list);
 
+  // Attempts to close |browser_to_close| while respecting OnBeforeUnloads.

[noms (inactive), 2014/08/26 20:17:32]

nit: OnBeforeUnload events?

[Mike Lerman, 2014/08/27 14:11:38]

Done.

+  static void TryToCloseBrowserList(const BrowserVector& browsers_to_close,
+      const base::Callback<void(size_t)>& on_close_confirmed,
+      const base::Callback<void(size_t)>& on_close_aborted,
+      const size_t profile_index);
+
+  // Called after handling an OnBeforeUnload. Will either try to close more

[noms (inactive), 2014/08/26 20:17:32]

nit: an OnBeforeUnload event?

[Mike Lerman, 2014/08/27 14:11:39]

Done.

+  // browsers or will call |on_close_aborted|.
+  static void PostBeforeUnloadHandlers(const BrowserVector& browsers_to_close,
+      const base::Callback<void(size_t)>& on_close_confirmed,
+      const base::Callback<void(size_t)>& on_close_aborted,
+      const size_t profile_index,
+      bool proceed);

[noms (inactive), 2014/08/26 20:17:32]

Please document all parameters

[Mike Lerman, 2014/08/27 14:11:38]

Done.

+
   // 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_