[Mac] Check if a profile is locked before allowing a new window to be opened.

chrome/browser/app_controller_mac_browsertest.mm

Index: chrome/browser/app_controller_mac_browsertest.mm
diff --git a/chrome/browser/app_controller_mac_browsertest.mm b/chrome/browser/app_controller_mac_browsertest.mm
index 4493a6f0f908f1818855d9f5dc8333e3c67b4d13..bd8dd9ebf4c945bc5d60692816f45274f6d10046 100644
--- a/chrome/browser/app_controller_mac_browsertest.mm
+++ b/chrome/browser/app_controller_mac_browsertest.mm
@@ -7,16 +7,22 @@
 #include "apps/shell_window_registry.h"
 #include "base/command_line.h"
 #include "base/mac/scoped_nsobject.h"
+#include "base/prefs/pref_service.h"
 #include "chrome/app/chrome_command_ids.h"
 #import "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/ui/browser_window.h"
+#import "chrome/browser/ui/cocoa/user_manager_mac.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#import "chrome/common/chrome_switches.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/web_contents.h"
@@ -113,4 +119,92 @@ IN_PROC_BROWSER_TEST_F(AppControllerWebAppBrowserTest,
   EXPECT_EQ(GetAppURL(), current_url.spec());
 }
 
+class AppControllerNewProfileManagementBrowserTest
+    : public InProcessBrowserTest {
+ protected:
+  AppControllerNewProfileManagementBrowserTest()
+      : active_browser_list_(BrowserList::GetInstance(
+                                chrome::GetActiveDesktop())) {
+  }
+
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    command_line->AppendSwitch(switches::kNewProfileManagement);
+  }
+
+  const BrowserList* active_browser_list_;
+};
+
+// Test that for a regular last profile, a reopen event opens a browser.
+IN_PROC_BROWSER_TEST_F(AppControllerNewProfileManagementBrowserTest,
+                       RegularProfileReopenWithNoWindows) {
+  base::scoped_nsobject<AppController> ac([[AppController alloc] init]);
+  EXPECT_EQ(1u, active_browser_list_->size());
+  BOOL result = [ac applicationShouldHandleReopen:NSApp hasVisibleWindows:NO];
+
+  EXPECT_FALSE(result);
+  EXPECT_EQ(2u, active_browser_list_->size());
+  EXPECT_FALSE(UserManagerMac::IsShowing());
+}
+
+// Test that for a locked last profile, a reopen event opens the user manager.
+IN_PROC_BROWSER_TEST_F(AppControllerNewProfileManagementBrowserTest,
+                       LockedProfileReopenWithNoWindows) {
+  base::scoped_nsobject<AppController> ac([[AppController alloc] init]);
+
+  // Lock the active profile.
+  Profile* profile = [ac lastProfile];
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+  size_t profile_index = cache.GetIndexOfProfileWithPath(profile->GetPath());
+  cache.SetProfileSigninRequiredAtIndex(profile_index, true);

[Nico, 2014/01/14 02:16:04]

This is an in-memory thing and doesn't need to be undone after the test, right?

[noms (inactive), 2014/01/14 17:59:50]

Correct. The browser process gets cleaned up at the beginning of a test.
On 2014/01/14 02:16:04, Nico wrote:

+  EXPECT_TRUE(cache.ProfileIsSigninRequiredAtIndex(profile_index));
+
+  EXPECT_EQ(1u, active_browser_list_->size());
+  BOOL result = [ac applicationShouldHandleReopen:NSApp hasVisibleWindows:NO];
+
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Opening the user manager requires creating a guest profile, which is
+  // asynchronous.
+  if (!UserManagerMac::IsShowing())
+    base::MessageLoop::current()->RunUntilIdle();

[Nico, 2014/01/14 02:16:04]

Huh, you're doing RunUntilIdle() twice in succession. Is more work added to the
message pump while UserManagerMac::IsShowing() is executed, or why is this
needed?

[noms (inactive), 2014/01/14 17:59:50]

IsShowing() doesn't do anything, but without the extra check the test is flaky
-- I can do RunUntilIdle, it will complete, and IsShowing() won't return true.
This also happens on Windows, and my only convincing argument is that creating a
guest profile has a bunch of nested callbacks, some of which can get enqueued
after the MessageLoop enqueues its "I'm done" task. Is this complete nonsense?

On 2014/01/14 02:16:04, Nico wrote:

[Nico, 2014/01/23 21:15:40]

As far as I understand (after patching this in, playing around a bit, and
reading code), this is still flaky:

* app controller calls UserManagerMac::Show
* that calls profile_manager->CreateProfileAsync
* that posts a task to the file thread
* when the file thread is done, it adds more tasks to the ui thread

So what happens is:
* the first RunUntilIdle() drains all UI thread tasks
* then UI gets descheduled, filethread runs, does its stuff, posts a new UI
thread task, gets descheduled
* the second RunUntilIdle() drains the new UI thread tasks.

This happens to work if the deschedules happen in the right order, but they
won't always.

(What do you mean with "its I'm done" task?)

+
+  EXPECT_FALSE(result);

[Nico, 2014/01/14 02:16:04]

Maybe check this right after you assign something to `result`

(The 2 comments on this test also apply to the next)

[noms (inactive), 2014/01/14 17:59:50]

Oops. Bad copy paste. Done.
On 2014/01/14 02:16:04, Nico wrote:

+  EXPECT_EQ(1u, active_browser_list_->size());
+  EXPECT_TRUE(UserManagerMac::IsShowing());
+  UserManagerMac::Hide();
+}
+
+// Test that for a guest last profile, a reopen event opens the user manager.
+IN_PROC_BROWSER_TEST_F(AppControllerNewProfileManagementBrowserTest,
+                       GuestProfileReopenWithNoWindows) {
+  // Create the guest profile, and set it as the last used profile so the
+  // app controller can use it on init.
+  scoped_ptr<Profile> guest_profile(Profile::CreateProfile(
+      ProfileManager::GetGuestProfilePath(),
+      NULL, Profile::CREATE_MODE_SYNCHRONOUS));
+  PrefService* local_state = g_browser_process->local_state();
+  local_state->SetString(prefs::kProfileLastUsed, chrome::kGuestProfileDir);
+
+  base::scoped_nsobject<AppController> ac([[AppController alloc] init]);
+
+  Profile* profile = [ac lastProfile];
+  EXPECT_EQ(ProfileManager::GetGuestProfilePath(), profile->GetPath());
+  EXPECT_TRUE(profile->IsGuestSession());
+
+  EXPECT_EQ(1u, active_browser_list_->size());
+  BOOL result = [ac applicationShouldHandleReopen:NSApp hasVisibleWindows:NO];
+
+  base::MessageLoop::current()->RunUntilIdle();
+  // Opening the user manager requires creating a guest profile, which is
+  // asynchronous.
+  if (!UserManagerMac::IsShowing())
+    base::MessageLoop::current()->RunUntilIdle();
+
+  EXPECT_FALSE(result);
+  EXPECT_EQ(1u, active_browser_list_->size());
+  EXPECT_TRUE(UserManagerMac::IsShowing());
+  UserManagerMac::Hide();
+}
+
 }  // namespace