Loading the Launcher with a Locked Profile launched UserManager.

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

 // Copyright 2013 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.
 
 #include "chrome/browser/ui/webui/signin/user_manager_screen_handler.h"
 
 #include "base/bind.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/value_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/screenlock_private/screenlock_private_api.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_info_cache_observer.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #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"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
@@ skipping 98 matching lines @@
   DCHECK(service);
   return service->GetBoolean(prefs::kBrowserGuestModeEnabled);
 }
 
 bool IsAddPersonEnabled() {
   PrefService* service = g_browser_process->local_state();
   DCHECK(service);
   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 ------------------------------------------------------
 
 class UserManagerScreenHandler::ProfileUpdateObserver
     : public ProfileInfoCacheObserver {
  public:
   ProfileUpdateObserver(
       ProfileManager* profile_manager, UserManagerScreenHandler* handler)
       : profile_manager_(profile_manager),
@@ skipping 551 matching lines @@
   // Unlock the profile after browser opens so startup can read the lock bit.
   // Any necessary authentication must have been successful to reach this point.
   if (!browser->profile()->IsGuestSession()) {
     ProfileInfoCache& info_cache =
         g_browser_process->profile_manager()->GetProfileInfoCache();
     size_t index = info_cache.GetIndexOfProfileWithPath(
         browser->profile()->GetPath());
     info_cache.SetProfileSigninRequiredAtIndex(index, false);
   }
 
-  if (url_hash_ == profiles::kUserManagerSelectProfileTaskManager) {
+  if (!url_hash_.empty()) {
     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) {
-    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.
   UserManager::Hide();
 }
 
 void UserManagerScreenHandler::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
@@ skipping 19 matching lines @@
     Profile* profile, Profile::CreateStatus profile_create_status) {
   Browser* browser = chrome::FindAnyBrowser(profile, false, desktop_type_);
   if (browser && browser->window()) {
     OnBrowserWindowReady(browser);
   } else {
     registrar_.Add(this,
                    chrome::NOTIFICATION_BROWSER_WINDOW_READY,
                    content::NotificationService::AllSources());
   }
 }