Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7044)

Unified Diff: chrome/browser/profiles/profile_window.cc

Issue 149883004: Fix sketchy User Manager code :( (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: moar better comments Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/profiles/profile_window.cc
diff --git a/chrome/browser/profiles/profile_window.cc b/chrome/browser/profiles/profile_window.cc
index 5dc41915e9792b8904908149f3316f0e56e14f81..49bd41f799e7c3f20ccd5b0a1d7ee51f7ec98442 100644
--- a/chrome/browser/profiles/profile_window.cc
+++ b/chrome/browser/profiles/profile_window.cc
@@ -28,23 +28,30 @@ using content::BrowserThread;
namespace {
// Handles running a callback when a new Browser has been completely created.
Alexei Svitkine (slow) 2014/02/07 15:44:43 Expand comment to say "a new Browser for the given
noms (inactive) 2014/02/07 16:30:58 Done.
-class BrowserAddedObserver : public chrome::BrowserListObserver {
+class BrowserAddedForProfileObserver : public chrome::BrowserListObserver {
public:
- explicit BrowserAddedObserver(
- profiles::ProfileSwitchingDoneCallback callback) : callback_(callback) {
+ BrowserAddedForProfileObserver(
+ Profile* profile, profiles::ProfileSwitchingDoneCallback callback)
Alexei Svitkine (slow) 2014/02/07 15:44:43 1 param per line
noms (inactive) 2014/02/07 16:30:58 Done.
+ : profile_(profile),
+ callback_(callback) {
+ DCHECK(!callback_.is_null());
BrowserList::AddObserver(this);
}
- virtual ~BrowserAddedObserver() {
- BrowserList::RemoveObserver(this);
+ virtual ~BrowserAddedForProfileObserver() {
}
private:
// Overridden from BrowserListObserver:
virtual void OnBrowserAdded(Browser* browser) OVERRIDE {
- DCHECK(!callback_.is_null());
- callback_.Run();
+ if (browser->profile() == profile_) {
+ BrowserList::RemoveObserver(this);
+ callback_.Run();
+ base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+ }
}
+ // Profile for which the browser should be opened.
+ Profile* profile_;
profiles::ProfileSwitchingDoneCallback callback_;
DISALLOW_COPY_AND_ASSIGN(BrowserAddedObserver);
@@ -93,10 +100,14 @@ void OpenBrowserWindowForProfile(
}
// If there is a callback, create an observer to make sure it is only
- // run when the browser has been completely created.
- scoped_ptr<BrowserAddedObserver> browser_added_observer;
+ // run when the browser has been completely created. This observer will
+ // delete itself once that happens. This should not leak, because we are
+ // passing |always_create| = true to FindOrCreateNewWindow below, which ends
+ // up calling LaunchBrowser and opens a new window. If for whatever reason
+ // that fails, either something has crashed, or the observer will be cleaned
+ // up when a different browser for this profile is opened.
if (!callback.is_null())
- browser_added_observer.reset(new BrowserAddedObserver(callback));
+ new BrowserAddedForProfileObserver(profile, callback);
// We already dealt with the case when |always_create| was false and a browser
// existed, which means that here a browser definitely needs to be created.

Powered by Google App Engine
This is Rietveld 408576698