Loading the Launcher with a Locked Profile launched UserManager.

chrome/browser/ui/webui/signin/user_manager_screen_handler.cc

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 91a5bce179edad88405345fd32f252e11d843bb6..8788df7033431810f7ee6f3349b26ae455c73dc6 100644
--- a/chrome/browser/ui/webui/signin/user_manager_screen_handler.cc
+++ b/chrome/browser/ui/webui/signin/user_manager_screen_handler.cc
@@ -21,10 +21,12 @@
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/signin/local_auth.h"
+#include "chrome/browser/ui/app_list/app_list_service.h"
 #include "chrome/browser/ui/browser_commands.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/browser_list_observer.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/user_manager.h"
@@ -143,6 +145,66 @@ bool IsAddPersonEnabled() {
   return service->GetBoolean(prefs::kBrowserAddPersonEnabled);
 }
 
+class UrlHashHelper : public chrome::BrowserListObserver {

[tapted, 2015/01/27 02:52:55]

add a brief comment, it should say something about the lifetime. (Also to me
`Hash` seems a but out of place and misleading. I guess it's historical here,
but I'd go maybe `KeyHander` or `IdHandler`)

[Mike Lerman, 2015/01/27 14:59:49]

Comment added.

It's called a "hash" because that's the variable used elsewhere in the
UserManagerScreenHandler to reference the parameter in the URL that follows the
#.

+ public:
+  UrlHashHelper(
+      Browser* browser, chrome::HostDesktopType desktop_type, std::string hash);

[tapted, 2015/01/27 02:52:55]

const reference `hash`

[Mike Lerman, 2015/01/27 14:59:49]

Done.

+  ~UrlHashHelper() override;
+
+  void ExecuteUrlHash();
+
+  // chrome::BrowserListObserver overrides:
+  void OnBrowserRemoved(Browser* browser) override;
+
+ private:
+  Browser* browser_;
+  Profile* profile_;
+  chrome::HostDesktopType desktop_type_;
+  std::string hash_;
+};

[tapted, 2015/01/27 02:52:55]

nit: DISALLOW_COPY_AND_ASSIGN

[Mike Lerman, 2015/01/27 14:59:49]

Done.

+
+UrlHashHelper::UrlHashHelper(
+    Browser* browser, chrome::HostDesktopType desktop_type, std::string hash)
+    : browser_(browser),
+      profile_(browser->profile()),
+      desktop_type_(desktop_type),

[tapted, 2015/01/27 02:52:55]

Note there's also browser->host_desktop_type(), which I'd say would be
marginally better than passing in |desktop_type| separately.

The question is raises would be whether OnBrowserWindowReady is ever called with
a browser on a different desktop type to
UserManagerScreenHandler::desktop_type_. On Mac, no. But on Windows 8 I can
imagine

 - Show App Launcher in Desktop Mode (appears in desktop mode regardless of
whether Chrome is in metro mode)
 - *locked or something* -> need to show UserManager
 - UserManager shows a Browser window
 - Browser windows are banned from desktop mode when user is in metro mode
 - Chrome opens Metro mode to show the browser window
 - ShowAppList....... but now it should show in Metro mode (since I'm pretty
sure we don't try to flip win8 out of metro mode when showing stuff on the
desktop [just the other way around]).

[Mike Lerman, 2015/01/27 14:59:49]

SGTM. Done.

+      hash_(hash) {
+  BrowserList::AddObserver(this);
+}
+
+UrlHashHelper::~UrlHashHelper() {
+  BrowserList::RemoveObserver(this);
+}
+
+void UrlHashHelper::OnBrowserRemoved(Browser* browser) {
+  if (browser == browser_)
+    browser_ = NULL;

[tapted, 2015/01/27 02:52:55]

nit: NULL -> nullptr. 

[Mike Lerman, 2015/01/27 14:59:49]

Done.

+}
+
+void UrlHashHelper::ExecuteUrlHash() {
+  if (browser_ == NULL)

[tapted, 2015/01/27 02:52:55]

NULL -> nullptr;

or, I'd probably go just `if (!browser_)`, but I'm not an OWNER here :)

[Mike Lerman, 2015/01/27 14:59:49]

Done. I like that better, too :)

+    browser_ = chrome::FindLastActiveWithProfile(profile_, desktop_type_);
+
+  if (browser_ != NULL) {
+    if (hash_ == profiles::kUserManagerSelectProfileTaskManager) {
+      chrome::OpenTaskManager(browser_);
+    } else if (hash_ == profiles::kUserManagerSelectProfileAboutChrome) {
+      chrome::ShowAboutChrome(browser_);
+    } else if (hash_ == profiles::kUserManagerSelectProfileChromeSettings) {
+      chrome::ShowSettings(browser_);
+    } else if (hash_ == profiles::kUserManagerSelectProfileChromeMemory) {
+      chrome::ShowMemory(browser_);
+    }
+  }
+
+  if (hash_ == profiles::kUserManagerSelectProfileAppLauncher) {
+    AppListService* app_list_service = AppListService::Get(desktop_type_);

[tapted, 2015/01/27 02:52:55]

note this desktop_type_ can be different to |browser_| now. Perhaps move this
whole `if` to the 

[tapted, 2015/01/27 02:56:36]

uh.. to the `top` that was going to say (but I thought I'd deleted this since
it's covered by the next comment)

[Mike Lerman, 2015/01/27 14:59:49]

Done.

[Mike Lerman, 2015/01/27 14:59:49]

Done.

+    app_list_service->ShowForProfile(profile_);
+  }
+
+  delete this;

[tapted, 2015/01/27 02:52:55]

Using base::Owned(..) rather than base::Unretained(..) might be a better fit for
this, but I guess this works too.

base::Owned(..) would also have the benefit of being able to return early for
some nicer control flow here:
i.e.

if (app_launcher) {
  .. ShowForProfile(
  return;
}

if (!browser_)
  browser_ =

if (!browser_)
  return;

etc.

[Mike Lerman, 2015/01/27 14:59:49]

So base::Owned will take care of deleting the object once the posted task is
done execution? Sounds ideal for me! I can't find any documentation about that
class.

[tapted, 2015/01/27 22:55:01]

btw this is documented in base/bind_helpers.h

but you're right that actually finding `Owned` is tricky since it's a 1-line
template function returning an OwnedWrapper

+}
+
 }  // namespace
 
 // ProfileUpdateObserver ------------------------------------------------------
@@ -714,18 +776,13 @@ void UserManagerScreenHandler::OnBrowserWindowReady(Browser* browser) {
     info_cache.SetProfileSigninRequiredAtIndex(index, false);
   }
 
-  if (url_hash_ == profiles::kUserManagerSelectProfileTaskManager) {
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE, base::Bind(&chrome::OpenTaskManager, browser));
-  } else if (url_hash_ == profiles::kUserManagerSelectProfileAboutChrome) {
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE, base::Bind(&chrome::ShowAboutChrome, browser));
-  } else if (url_hash_ == profiles::kUserManagerSelectProfileChromeSettings) {
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE, base::Bind(&chrome::ShowSettings, browser));
-  } else if (url_hash_ == profiles::kUserManagerSelectProfileChromeMemory) {
+  if (!url_hash_.empty()) {
     base::MessageLoop::current()->PostTask(
-        FROM_HERE, base::Bind(&chrome::ShowMemory, browser));
+        FROM_HERE,
+        base::Bind(&UrlHashHelper::ExecuteUrlHash,
+                   base::Unretained(new UrlHashHelper(browser,
+                                                      desktop_type_,
+                                                      url_hash_))));
   }
 
   // This call is last as it deletes this object.