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

Unified Diff: chrome/browser/ui/webui/signin/user_manager_screen_handler.cc

Issue 564453003: Access to Chrome via the System Tray should go through the User Manager. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Minor pre-review cleanup Created 6 years, 3 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/ui/webui/signin/user_manager_screen_handler.cc
diff --git a/chrome/browser/ui/webui/signin/user_manager_screen_handler.cc b/chrome/browser/ui/webui/signin/user_manager_screen_handler.cc
index 5bd6b062699552bc62f748a4a5d665ae35c0d1e4..8ff0b3b22ca3d17a1df7b29befd933cd2271ced2 100644
--- a/chrome/browser/ui/webui/signin/user_manager_screen_handler.cc
+++ b/chrome/browser/ui/webui/signin/user_manager_screen_handler.cc
@@ -22,6 +22,8 @@
#include "chrome/browser/signin/local_auth.h"
#include "chrome/browser/ui/browser_dialogs.h"
#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/chrome_pages.h"
#include "chrome/browser/ui/singleton_tabs.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
@@ -84,18 +86,6 @@ void OpenNewWindowForProfile(
false);
}
-// This callback is run after switching to a new profile has finished. This
-// means either a new browser window has been opened, or an existing one
-// has been found, which means we can safely close the User Manager without
-// accidentally terminating the browser process. The task needs to be posted,
-// as HideUserManager will end up destroying its WebContents, which will
-// destruct the UserManagerScreenHandler as well.
-void OnSwitchToProfileComplete() {
- base::MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&chrome::HideUserManager));
-}
-
std::string GetAvatarImageAtIndex(
size_t index, const ProfileInfoCache& info_cache) {
bool is_gaia_picture =
@@ -287,6 +277,11 @@ void UserManagerScreenHandler::Unlock(const std::string& user_email) {
}
void UserManagerScreenHandler::HandleInitialize(const base::ListValue* args) {
+ // If the URL has a hash parameter, store it for later.
+ base::string16 url_hash;
+ if (args->GetString(0, &url_hash))
+ url_hash_ = url_hash;
+
SendUserList();
web_ui()->CallJavascriptFunction("cr.ui.Oobe.showUserManagerScreen",
base::FundamentalValue(IsGuestModeEnabled()));
@@ -297,9 +292,11 @@ void UserManagerScreenHandler::HandleInitialize(const base::ListValue* args) {
}
void UserManagerScreenHandler::HandleAddUser(const base::ListValue* args) {
- profiles::CreateAndSwitchToNewProfile(desktop_type_,
- base::Bind(&OnSwitchToProfileComplete),
- ProfileMetrics::ADD_NEW_USER_MANAGER);
+ profiles::CreateAndSwitchToNewProfile(
+ desktop_type_,
+ base::Bind(&UserManagerScreenHandler::OnSwitchToProfileComplete,
+ base::Unretained(this)),
+ ProfileMetrics::ADD_NEW_USER_MANAGER);
}
void UserManagerScreenHandler::HandleAuthenticatedLaunchUser(
@@ -378,8 +375,10 @@ void UserManagerScreenHandler::HandleRemoveUser(const base::ListValue* args) {
void UserManagerScreenHandler::HandleLaunchGuest(const base::ListValue* args) {
if (IsGuestModeEnabled()) {
- profiles::SwitchToGuestProfile(desktop_type_,
- base::Bind(&OnSwitchToProfileComplete));
+ profiles::SwitchToGuestProfile(
+ desktop_type_,
+ base::Bind(&UserManagerScreenHandler::OnSwitchToProfileComplete,
+ base::Unretained(this)));
ProfileMetrics::LogProfileSwitchUser(ProfileMetrics::SWITCH_PROFILE_GUEST);
} else {
// The UI should have prevented the user from allowing the selection of
@@ -418,11 +417,13 @@ void UserManagerScreenHandler::HandleLaunchUser(const base::ListValue* args) {
ProfileMetrics::LogProfileAuthResult(ProfileMetrics::AUTH_UNNECESSARY);
base::FilePath path = info_cache.GetPathOfProfileAtIndex(profile_index);
- profiles::SwitchToProfile(path,
- desktop_type_,
- false, /* reuse any existing windows */
- base::Bind(&OnSwitchToProfileComplete),
- ProfileMetrics::SWITCH_PROFILE_MANAGER);
+ profiles::SwitchToProfile(
+ path,
+ desktop_type_,
+ false, /* reuse any existing windows */
+ base::Bind(&UserManagerScreenHandler::OnSwitchToProfileComplete,
+ base::Unretained(this)),
+ ProfileMetrics::SWITCH_PROFILE_MANAGER);
}
void UserManagerScreenHandler::HandleAttemptUnlock(
@@ -666,9 +667,13 @@ void UserManagerScreenHandler::ReportAuthenticationResult(
authenticating_profile_index_, false);
base::FilePath path = info_cache.GetPathOfProfileAtIndex(
authenticating_profile_index_);
- profiles::SwitchToProfile(path, desktop_type_, true,
- base::Bind(&OnSwitchToProfileComplete),
- ProfileMetrics::SWITCH_PROFILE_UNLOCK);
+ profiles::SwitchToProfile(
+ path,
+ desktop_type_,
+ true,
+ base::Bind(&UserManagerScreenHandler::OnSwitchToProfileComplete,
+ base::Unretained(this)),
Evan Stade 2014/09/11 17:02:51 I don't think unretained is right here. Are you gu
Mike Lerman 2014/09/11 18:49:29 The User Manager will certainly always wait around
Evan Stade 2014/09/11 19:02:52 During shutdown is the time I'd be most worried ab
noms (inactive) 2014/09/11 19:05:41 I think we could get in a weird situation where yo
Mike Lerman 2014/09/11 20:51:51 Done.
+ ProfileMetrics::SWITCH_PROFILE_UNLOCK);
} else {
web_ui()->CallJavascriptFunction(
"cr.ui.Oobe.showSignInError",
@@ -679,3 +684,33 @@ void UserManagerScreenHandler::ReportAuthenticationResult(
base::FundamentalValue(0));
}
}
+
+// This callback is run after switching to a new profile has finished. This
+// means either a new browser window has been opened, or an existing one
+// has been found, which means we can safely close the User Manager without
+// accidentally terminating the browser process. The task needs to be posted,
+// as HideUserManager will end up destroying its WebContents, which will
+// destruct the UserManagerScreenHandler as well.
Evan Stade 2014/09/11 17:02:50 I don't quite understand. HideUserManager might de
Mike Lerman 2014/09/11 18:49:29 More in depth explanation is here, where the code
Evan Stade 2014/09/11 19:02:52 I don't see how L380 or any use is synchronous? E
Mike Lerman 2014/09/11 20:51:51 L380 can execute the following chain of calls sync
+void UserManagerScreenHandler::OnSwitchToProfileComplete(
+ Profile* profile, Profile::CreateStatus profile_create_status) {
+ base::MessageLoop::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&chrome::HideUserManager));
+ if (url_hash_ ==
+ base::UTF8ToUTF16(profiles::kUserManagerSelectProfileTaskManager)) {
+ Browser* browser = BrowserList::GetInstance(desktop_type_)->get(0);
+ base::MessageLoop::current()->PostTask(
+ FROM_HERE, base::Bind(&chrome::ShowTaskManager, browser));
Evan Stade 2014/09/11 17:02:51 I'm worried that |browser| might be invalid by the
Mike Lerman 2014/09/11 18:49:29 This is for the new browser object that's created
Evan Stade 2014/09/11 19:02:52 I do think you should call ShowTaskManager synchro
noms (inactive) 2014/09/11 19:05:41 I'm not sure why we need to PostTask this one. Wha
Mike Lerman 2014/09/11 20:51:51 The TaskManager requires the browser's window to b
Evan Stade 2014/09/11 21:30:00 Did you consider listening for NOTIFICATION_BROWSE
Mike Lerman 2014/09/12 14:26:42 New implementation! Huzzah. I now look at the bro
+ } else if (
+ url_hash_ ==
Evan Stade 2014/09/11 17:02:51 move to same line as if
Mike Lerman 2014/09/11 18:49:29 Done.
+ base::UTF8ToUTF16(profiles::kUserManagerSelectProfileAboutChrome)) {
+ // The new browser window may not have been created. Make a new one.
+ Browser* browser = chrome::FindAnyBrowser(profile, false, desktop_type_);
+ if (browser) {
+ base::MessageLoop::current()->PostTask(
+ FROM_HERE, base::Bind(&chrome::ShowAboutChrome, browser));
+ } else {
+ NOTREACHED();
+ }
+ }
+}

Powered by Google App Engine
This is Rietveld 408576698