Implement a policy to autologin a public account.

chrome/browser/chromeos/login/existing_user_controller_browsertest.cc

Index: chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
diff --git a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
index 03995c36f40c67d512da2cdab54a31a041b5c9de..7c905c3d1cfc0e38a58f61e48f26a97ac71b82e5 100644
--- a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
@@ -2,10 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ash/shell.h"

[bartfab (slow), 2013/02/25 16:51:23]

I trust you checked that the list of includes matches what your current code
uses? It is pretty hard to verify this as part of the review :).

[dconnelly, 2013/02/26 18:04:15]

Done.

+#include "ash/wm/user_activity_detector.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
+#include "base/stl_util.h"
 #include "chrome/browser/chromeos/cros/cros_in_process_browser_test.h"
 #include "chrome/browser/chromeos/cros/cros_mock.h"
 #include "chrome/browser/chromeos/cros/mock_network_library.h"
@@ -21,12 +25,25 @@
 #include "chrome/browser/chromeos/login/mock_url_fetchers.h"
 #include "chrome/browser/chromeos/login/mock_user_manager.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chrome/browser/policy/browser_policy_connector.h"
+#include "chrome/browser/policy/cloud_policy_constants.h"
+#include "chrome/browser/policy/cloud_policy_store.h"
+#include "chrome/browser/policy/device_local_account_policy_service.h"
+#include "chrome/browser/policy/policy_builder.h"
+#include "chrome/browser/policy/proto/chrome_device_policy.pb.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "chromeos/dbus/fake_session_manager_client.h"
 #include "chromeos/dbus/mock_dbus_thread_manager.h"
 #include "chromeos/dbus/mock_session_manager_client.h"
 #include "chromeos/dbus/mock_shill_manager_client.h"
+#include "content/public/test/mock_notification_observer.h"
 #include "google_apis/gaia/mock_url_fetcher_factory.h"
 #include "grit/generated_resources.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -39,10 +56,13 @@ using ::testing::AnyOf;
 using ::testing::Invoke;
 using ::testing::InvokeWithoutArgs;
 using ::testing::Return;
+using ::testing::ReturnRef;
 using ::testing::ReturnNull;
 using ::testing::Sequence;
 using ::testing::WithArg;
 
+namespace em = enterprise_management;
+
 namespace chromeos {
 
 namespace {
@@ -51,6 +71,11 @@ const char kUsername[] = "test_user@gmail.com";
 const char kNewUsername[] = "test_new_user@gmail.com";
 const char kPassword[] = "test_password";
 
+const char kAutoLoginUsername[] = "public_session_user@localhost";
+const int kAutoLoginNoDelay = 0;
+const int kAutoLoginShortDelay = 100;
+const int kAutoLoginLongDelay = 10000;
+
 class MockLoginDisplay : public LoginDisplay {
  public:
   MockLoginDisplay()
@@ -101,6 +126,18 @@ class MockLoginDisplayHost : public LoginDisplayHost {
   DISALLOW_COPY_AND_ASSIGN(MockLoginDisplayHost);
 };
 
+class MockCloudPolicyStoreObserver : public policy::CloudPolicyStore::Observer {
+ public:
+  MockCloudPolicyStoreObserver() {
+  }
+
+  MOCK_METHOD1(OnStoreLoaded, void(policy::CloudPolicyStore*));
+  MOCK_METHOD1(OnStoreError, void(policy::CloudPolicyStore*));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockCloudPolicyStoreObserver);
+};
+
 scoped_refptr<Authenticator> CreateAuthenticator(
     LoginStatusConsumer* consumer) {
   return new MockAuthenticator(consumer, kUsername, kPassword);
@@ -111,6 +148,41 @@ scoped_refptr<Authenticator> CreateAuthenticatorNewUser(
   return new MockAuthenticator(consumer, kNewUsername, kPassword);
 }
 
+scoped_refptr<Authenticator> CreateAuthenticatorPublicSession(
+    LoginStatusConsumer* consumer) {
+  return new MockAuthenticator(consumer, kAutoLoginUsername, "");
+}
+
+// Observes user list change notifications and quits the message loop once a

[bartfab (slow), 2013/02/25 16:51:23]

Is this not a straight copy & paste from Mattias' test as well? Code duplication
makes me :...(

[dconnelly, 2013/02/26 18:04:15]

He refactored it into a different thing and I missed it when I looked at his
submitted CL.  I've now refactored that out into its own file.

+// specified user appears.
+class UserListObserver : public content::NotificationObserver {
+ public:
+  explicit UserListObserver(const std::string& user_to_watch_for)
+      : user_to_watch_for_(user_to_watch_for) {}
+
+  void Run() {
+    if (chromeos::UserManager::Get()->IsKnownUser(user_to_watch_for_))
+      return;
+
+    content::NotificationRegistrar registrar;
+    registrar.Add(this, chrome::NOTIFICATION_USER_LIST_CHANGED,
+                  content::NotificationService::AllSources());
+    run_loop_.Run();
+  }
+
+  // content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE {
+    if (chromeos::UserManager::Get()->IsKnownUser(user_to_watch_for_))
+      run_loop_.Quit();
+  }
+
+ private:
+  const std::string user_to_watch_for_;
+  base::RunLoop run_loop_;
+};
+
 }  // namespace
 
 class ExistingUserControllerTest : public CrosInProcessBrowserTest {
@@ -143,6 +215,9 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
     EXPECT_CALL(*mock_dbus_thread_manager->mock_shill_manager_client(),
                 RemovePropertyChangedObserver(_))
         .Times(AnyNumber());
+
+    SetUpSessionManager(mock_dbus_thread_manager);
+
     DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager);
     CrosInProcessBrowserTest::SetUpInProcessBrowserTestFixture();
     cros_mock_->InitStatusAreaMocks();
@@ -154,13 +229,6 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
     EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _))
         .WillRepeatedly(Return(true));
 
-    MockSessionManagerClient* mock_session_manager_client =
-        mock_dbus_thread_manager->mock_session_manager_client();
-    EXPECT_CALL(*mock_session_manager_client, EmitLoginPromptReady())
-        .Times(1);
-    EXPECT_CALL(*mock_session_manager_client, RetrieveDevicePolicy(_))
-        .Times(AnyNumber());
-
     mock_login_utils_ = new MockLoginUtils();
     LoginUtils::Set(mock_login_utils_);
     EXPECT_CALL(*mock_login_utils_, PrewarmAuthentication())
@@ -171,32 +239,52 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
         .Times(1);
 
     mock_login_display_host_.reset(new MockLoginDisplayHost());
+    mock_login_display_ = new MockLoginDisplay();
+    SetUpLoginDisplay();
+  }
 
-    EXPECT_CALL(*mock_user_manager_.user_manager(), IsKnownUser(kUsername))
+  virtual void SetUpSessionManager(
+      MockDBusThreadManager* mock_dbus_thread_manager) {
+    mock_user_manager_.reset(new ScopedMockUserManagerEnabler);
+    EXPECT_CALL(*mock_user_manager_->user_manager(), IsKnownUser(kUsername))
         .Times(AnyNumber())
         .WillRepeatedly(Return(true));
-    EXPECT_CALL(*mock_user_manager_.user_manager(), IsKnownUser(kNewUsername))
+    EXPECT_CALL(*mock_user_manager_->user_manager(), IsKnownUser(kNewUsername))
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
-    EXPECT_CALL(*mock_user_manager_.user_manager(), IsUserLoggedIn())
+    EXPECT_CALL(*mock_user_manager_->user_manager(), IsUserLoggedIn())
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
-    EXPECT_CALL(*mock_user_manager_.user_manager(), IsLoggedInAsGuest())
+    EXPECT_CALL(*mock_user_manager_->user_manager(), IsLoggedInAsGuest())
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
-    EXPECT_CALL(*mock_user_manager_.user_manager(), IsLoggedInAsDemoUser())
+    EXPECT_CALL(*mock_user_manager_->user_manager(), IsLoggedInAsDemoUser())
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
-    EXPECT_CALL(*mock_user_manager_.user_manager(), IsLoggedInAsPublicAccount())
+    EXPECT_CALL(*mock_user_manager_->user_manager(),
+                IsLoggedInAsPublicAccount())
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
-    EXPECT_CALL(*mock_user_manager_.user_manager(), IsSessionStarted())
+    EXPECT_CALL(*mock_user_manager_->user_manager(), IsSessionStarted())
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
+    EXPECT_CALL(*mock_user_manager_->user_manager(), IsCurrentUserNew())
+        .Times(AnyNumber())
+        .WillRepeatedly(Return(false));
+    EXPECT_CALL(*mock_user_manager_->user_manager(), Shutdown())
+        .Times(1);
 
+    MockSessionManagerClient* mock_session_manager_client =
+        mock_dbus_thread_manager->mock_session_manager_client();
+    EXPECT_CALL(*mock_session_manager_client, EmitLoginPromptReady())
+        .Times(1);
+    EXPECT_CALL(*mock_session_manager_client, RetrieveDevicePolicy(_))
+        .Times(AnyNumber());
+  }
+
+  virtual void SetUpLoginDisplay() {
     // |mock_login_display_| is owned by the ExistingUserController, which calls
     // CreateLoginDisplay() on the |mock_login_display_host_| to get it.
-    mock_login_display_ = new MockLoginDisplay();
     EXPECT_CALL(*mock_login_display_host_.get(), CreateLoginDisplay(_))
         .Times(1)
         .WillOnce(Return(mock_login_display_));
@@ -226,6 +314,10 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
   }
 
   virtual void CleanUpOnMainThread() OVERRIDE {
+    // ExistingUserController must be cleaned up before
+    // CrosInProcessBrowserTest::CleanUpOnMainThread or it sometimes

[bartfab (slow), 2013/02/25 16:51:23]

"or it sometimes crashes" sounds like something is going on that we do not
understand. If this is a crash we can reproduce and understand, better state it
here (e.g. "because it is an observer of singleton X"). If this is an
unexplicable crash, it would be good to get a backtrace and understand why it
happens only sometimes.

[dconnelly, 2013/02/26 18:04:15]

Done.

+    // crashes in the destructor.
+    existing_user_controller_.reset(NULL);
     CrosInProcessBrowserTest::CleanUpOnMainThread();
     testing_profile_.reset(NULL);
   }
@@ -235,6 +327,32 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
     DBusThreadManager::Shutdown();
   }
 
+  // ExistingUserController private member accessors.
+  base::OneShotTimer<ExistingUserController>* auto_login_timer() {
+    return &existing_user_controller()->auto_login_timer_;
+  }
+
+  std::string auto_login_username() {
+    return existing_user_controller()->public_session_auto_login_username_;
+  }
+  void set_auto_login_username(const std::string& username) {
+    existing_user_controller()->public_session_auto_login_username_ = username;
+  }
+
+  int auto_login_delay() {
+    return existing_user_controller()->public_session_auto_login_delay_;
+  }
+  void set_auto_login_delay(int delay) {
+    existing_user_controller()->public_session_auto_login_delay_ = delay;
+  }
+
+  bool is_login_in_progress() {
+    return existing_user_controller()->is_login_in_progress_;
+  }
+  void set_is_login_in_progress(bool is_login_in_progress) {
+    existing_user_controller()->is_login_in_progress_ = is_login_in_progress;
+  }
+
   scoped_ptr<ExistingUserController> existing_user_controller_;
 
   // These mocks are owned by CrosLibrary class.
@@ -243,7 +361,7 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
   MockLoginDisplay* mock_login_display_;
   scoped_ptr<MockLoginDisplayHost> mock_login_display_host_;
 
-  ScopedMockUserManagerEnabler mock_user_manager_;
+  scoped_ptr<ScopedMockUserManagerEnabler> mock_user_manager_;
 
   // Owned by LoginUtilsWrapper.
   MockLoginUtils* mock_login_utils_;
@@ -285,7 +403,7 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerTest, ExistingUserLogin) {
   EXPECT_CALL(*mock_login_display_host_,
               StartWizard(WizardController::kTermsOfServiceScreenName, NULL))
       .Times(0);
-  EXPECT_CALL(*mock_user_manager_.user_manager(), IsCurrentUserNew())
+  EXPECT_CALL(*mock_user_manager_->user_manager(), IsCurrentUserNew())
       .Times(AnyNumber())
       .WillRepeatedly(Return(false));
   existing_user_controller()->Login(kUsername, kPassword);
@@ -300,7 +418,7 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerTest, AutoEnrollAfterSignIn) {
       .Times(1);
   EXPECT_CALL(*mock_login_display_host_.get(), OnCompleteLogin())
       .Times(1);
-  EXPECT_CALL(*mock_user_manager_.user_manager(), IsCurrentUserNew())
+  EXPECT_CALL(*mock_user_manager_->user_manager(), IsCurrentUserNew())
       .Times(AnyNumber())
       .WillRepeatedly(Return(false));
   // The order of these expected calls matters: the UI if first disabled
@@ -345,7 +463,7 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerTest,
       .Times(1);
   EXPECT_CALL(*mock_login_display_host_.get(), OnCompleteLogin())
       .Times(1);
-  EXPECT_CALL(*mock_user_manager_.user_manager(), IsCurrentUserNew())
+  EXPECT_CALL(*mock_user_manager_->user_manager(), IsCurrentUserNew())
       .Times(AnyNumber())
       .WillRepeatedly(Return(true));
 
@@ -365,4 +483,526 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerTest,
   content::RunAllPendingInMessageLoop();
 }
 
+MATCHER_P(HasDetails, expected, "") {
+  std::string actual = *content::Details<const std::string>(arg).ptr();
+  return expected == actual;

[bartfab (slow), 2013/02/25 16:51:23]

Why not do the dereferencing and comparison all on one line?

[dconnelly, 2013/02/26 18:04:15]

Done.

+}
+
+class ExistingUserControllerPublicSessionTest
+  : public ExistingUserControllerTest {
+ protected:
+  ExistingUserControllerPublicSessionTest() {
+  }
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    ExistingUserControllerTest::SetUpOnMainThread();
+
+    // Wait for the public session user to be created.
+    UserListObserver(kAutoLoginUsername).Run();
+
+    // Wait for the device local account policy to be installed.
+    MockCloudPolicyStoreObserver observer;
+    content::MessageLoopRunner* runner = new content::MessageLoopRunner;
+    policy::CloudPolicyStore* store = TestingBrowserProcess::GetGlobal()->
+      browser_policy_connector()->GetDeviceLocalAccountPolicyService()->

[bartfab (slow), 2013/02/25 16:51:23]

Indent 4 spaces.

[dconnelly, 2013/02/26 18:04:15]

Done.

+      GetBrokerForAccount(kAutoLoginUsername)->core()->store();

[bartfab (slow), 2013/02/25 16:51:23]

Indent 4 spaces.

[dconnelly, 2013/02/26 18:04:15]

Done.

+    store->AddObserver(&observer);
+    EXPECT_CALL(observer, OnStoreLoaded(store))
+        .Times(1)
+        .WillOnce(InvokeWithoutArgs(runner, &content::MessageLoopRunner::Quit));
+    runner->Run();
+    store->RemoveObserver(&observer);
+  }
+
+  virtual void SetUpSessionManager(
+      MockDBusThreadManager* mock_dbus_thread_manager) OVERRIDE {
+    EXPECT_CALL(*mock_dbus_thread_manager, GetSessionManagerClient())
+      .WillRepeatedly(Return(&session_manager_client_));

[bartfab (slow), 2013/02/25 16:51:23]

Indent 4 spaces.

[dconnelly, 2013/02/26 18:04:15]

Done.

+
+    // Install the owner key.
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    base::FilePath owner_key_file = temp_dir_.path().AppendASCII("owner.key");
+    std::vector<uint8> owner_key_bits;
+    ASSERT_TRUE(device_policy_.signing_key()->ExportPublicKey(&owner_key_bits));
+    ASSERT_EQ(
+        file_util::WriteFile(
+            owner_key_file,
+            reinterpret_cast<const char*>(vector_as_array(&owner_key_bits)),
+            owner_key_bits.size()),
+        static_cast<int>(owner_key_bits.size()));
+    ASSERT_TRUE(PathService::Override(chrome::FILE_OWNER_KEY, owner_key_file));
+
+    // Setup the device policy.
+    em::ChromeDeviceSettingsProto& proto(device_policy_.payload());
+    proto.mutable_device_local_accounts()->add_account()->set_id(
+        kAutoLoginUsername);
+    RefreshDevicePolicy();
+
+    // Setup the device local account policy.
+    policy::UserPolicyBuilder device_local_account_policy;
+    device_local_account_policy.policy_data().set_username(kAutoLoginUsername);
+    device_local_account_policy.policy_data().set_policy_type(
+        policy::dm_protocol::kChromePublicAccountPolicyType);
+    device_local_account_policy.policy_data().set_settings_entity_id(
+        kAutoLoginUsername);
+    device_local_account_policy.Build();
+    session_manager_client_.set_device_local_account_policy(
+        kAutoLoginUsername,
+        device_local_account_policy.GetBlob());
+  }
+
+  virtual void SetUpLoginDisplay() {
+    EXPECT_CALL(*mock_login_display_host_.get(), CreateLoginDisplay(_))
+        .Times(1)
+        .WillOnce(Return(mock_login_display_));
+    EXPECT_CALL(*mock_login_display_host_.get(), GetNativeWindow())
+      .Times(AnyNumber())
+      .WillRepeatedly(ReturnNull());
+    EXPECT_CALL(*mock_login_display_host_.get(), OnPreferencesChanged())
+      .Times(AnyNumber());
+    EXPECT_CALL(*mock_login_display_, Init(_, _, _, _))
+      .Times(AnyNumber());
+  }
+
+  void DisableAutoLoginPolicyNotifications() {
+    CrosSettings::Get()->RemoveSettingsObserver(
+        kAccountsPrefDeviceLocalAccountAutoLoginId,
+        existing_user_controller());
+    CrosSettings::Get()->RemoveSettingsObserver(
+        kAccountsPrefDeviceLocalAccountAutoLoginDelay,
+        existing_user_controller());
+  }
+
+  void PrepareLogin(
+      const std::string& username, const std::string& password,

[bartfab (slow), 2013/02/25 16:51:23]

The arguments should either *all* fit on one line or *all* be placed on
individual lines, not a combination of both.

[dconnelly, 2013/02/26 18:04:15]

Done.

+      scoped_refptr<Authenticator> create_authenticator(
+          LoginStatusConsumer* consumer)) {
+    EXPECT_CALL(*mock_login_display_, SetUIEnabled(false))
+        .Times(AnyNumber());
+    EXPECT_CALL(*mock_login_utils_, CreateAuthenticator(_))
+        .Times(1)
+        .WillOnce(WithArg<0>(Invoke(create_authenticator)));
+    EXPECT_CALL(*mock_login_utils_,
+                PrepareProfile(username, _, password, _, _, _))
+        .Times(1)
+        .WillOnce(InvokeWithoutArgs(&profile_prepared_cb_,
+                                    &base::Callback<void(void)>::Run));
+    EXPECT_CALL(*mock_login_utils_,
+                DoBrowserLaunch(testing_profile_.get(),
+                                mock_login_display_host_.get()))
+        .Times(1);
+    EXPECT_CALL(*mock_login_display_, OnLoginSuccess(username))
+        .Times(1);
+    EXPECT_CALL(*mock_login_display_, SetUIEnabled(true))
+        .Times(1);
+    EXPECT_CALL(*mock_login_display_, OnFadeOut())
+        .Times(1);
+    EXPECT_CALL(*mock_login_display_host_,
+                StartWizard(WizardController::kTermsOfServiceScreenName, NULL))
+        .Times(0);
+  }
+
+  content::MessageLoopRunner* CreateSettingsObserverRunLoop(
+      content::MockNotificationObserver& observer, const char* setting) {
+    content::MessageLoopRunner* runner = new content::MessageLoopRunner;
+    EXPECT_CALL(observer, Observe(chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED,
+                                  _, HasDetails(setting)))
+        .Times(1)
+        .WillOnce(InvokeWithoutArgs(runner, &content::MessageLoopRunner::Quit));
+    CrosSettings::Get()->AddSettingsObserver(setting, &observer);
+    return runner;
+  }
+
+  void RefreshDevicePolicy() {
+    // Reset the key to its original state.
+    device_policy_.set_signing_key(
+        policy::PolicyBuilder::CreateTestSigningKey());
+    device_policy_.Build();
+    // Clear the key so the device can't write owner settings.
+    device_policy_.set_signing_key(
+        make_scoped_ptr<crypto::RSAPrivateKey>(NULL));
+    device_policy_.set_new_signing_key(
+        make_scoped_ptr<crypto::RSAPrivateKey>(NULL));
+    session_manager_client_.set_device_policy(device_policy_.GetBlob());
+  }
+
+  void SetAutoLoginPolicy(const std::string& username, int delay) {
+    // Wait until ExistingUserController has finished auto-login
+    // configuration by observing the same settings that trigger
+    // ConfigurePublicSessionAutoLogin.
+    content::MockNotificationObserver observer;
+
+    em::ChromeDeviceSettingsProto& proto(device_policy_.payload());
+
+    // If both settings have changed we need to wait for both to
+    // propagate, so check the new values against the old ones.
+    content::MessageLoopRunner* runner1 = NULL;
+    if (!proto.has_device_local_accounts() ||
+        !proto.device_local_accounts().has_auto_login_id() ||
+        proto.device_local_accounts().auto_login_id() != username) {
+      runner1 = CreateSettingsObserverRunLoop(
+          observer, kAccountsPrefDeviceLocalAccountAutoLoginId);
+    }
+    content::MessageLoopRunner* runner2 = NULL;
+    if (!proto.has_device_local_accounts() ||
+        !proto.device_local_accounts().has_auto_login_delay() ||
+        proto.device_local_accounts().auto_login_delay() != delay) {
+      runner2 = CreateSettingsObserverRunLoop(
+          observer, kAccountsPrefDeviceLocalAccountAutoLoginDelay);
+    }
+
+    // Update the policy.
+    proto.mutable_device_local_accounts()->set_auto_login_id(username);
+    proto.mutable_device_local_accounts()->set_auto_login_delay(delay);
+    RefreshDevicePolicy();
+
+    // Wait for ExistingUserController.
+    if (runner1)
+      runner1->Run();
+    if (runner2)
+      runner2->Run();
+
+    // Cleanup.

[bartfab (slow), 2013/02/25 16:51:23]

To match the other comments, this should be a verb in the imperative form ->
"Clean up." instead of "Cleanup." (which is a noun)

[dconnelly, 2013/02/26 18:04:15]

Done.

+    CrosSettings::Get()->RemoveSettingsObserver(
+        kAccountsPrefDeviceLocalAccountAutoLoginId,
+        &observer);
+    CrosSettings::Get()->RemoveSettingsObserver(
+        kAccountsPrefDeviceLocalAccountAutoLoginDelay,
+        &observer);
+  }
+
+  // Enables public sessions in tests.
+  FakeSessionManagerClient session_manager_client_;
+
+  // Stores the device owner key.
+  base::ScopedTempDir temp_dir_;
+
+  // Carries Chrome OS device policies for tests.
+  policy::DevicePolicyBuilder device_policy_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ExistingUserControllerPublicSessionTest);
+};
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       StartAutoLoginTimer) {
+  ExistingUserController* euc = existing_user_controller();

[bartfab (slow), 2013/02/25 16:51:23]

The style guide discourages abbreviations. |euc| is hard to read and remember.
Better use verbose variable names.

[dconnelly, 2013/02/26 18:04:15]

Done.

+  DisableAutoLoginPolicyNotifications();
+
+  // Timer shouldn't start until signin screen is ready.
+  set_auto_login_username(kAutoLoginUsername);
+  set_auto_login_delay(kAutoLoginLongDelay);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());

[bartfab (slow), 2013/02/25 16:51:23]

Here and in general: You should be using ASSERT_* only when it is critical to
abort the test if the condition is not met (e.g. if the timer is not running,
the test would deadlock, so abort). Otherwise, you should be using EXPECT_*.

[dconnelly, 2013/02/26 18:04:15]

Done.

+
+  // Timer shouldn't start if the policy isn't set.
+  set_auto_login_username("");
+  euc->OnSigninScreenReady();
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  // Timer shouldn't fire in the middle of a login attempt.
+  set_auto_login_username(kAutoLoginUsername);
+  set_is_login_in_progress(true);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  // Otherwise start.
+  set_is_login_in_progress(false);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(),
+            kAutoLoginLongDelay);
+
+  content::RunAllPendingInMessageLoop();

[bartfab (slow), 2013/02/25 16:51:23]

Could you document what tasks this drains from the message loop's queue and why
that is needed?

[dconnelly, 2013/02/26 18:04:15]

Done.

+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       StopAutoLoginTimer) {
+  ExistingUserController* euc = existing_user_controller();
+  euc->OnSigninScreenReady();
+  set_auto_login_username(kAutoLoginUsername);
+  set_auto_login_delay(kAutoLoginLongDelay);
+
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+
+  euc->StopPublicSessionAutoLoginTimer();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  content::RunAllPendingInMessageLoop();
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       UserActivityRestartsAutoLogin) {
+  ExistingUserController* euc = existing_user_controller();
+  euc->OnSigninScreenReady();
+  set_auto_login_username(kAutoLoginUsername);
+
+  // Timer starts off not running.
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  // When the timer isn't running, nothing should happen.
+  euc->OnUserActivity();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  // Start the timer.
+  set_auto_login_delay(kAutoLoginLongDelay);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(),
+            kAutoLoginLongDelay);
+
+  // User activity should restart the timer.
+  set_auto_login_delay(kAutoLoginShortDelay);

[bartfab (slow), 2013/02/25 16:51:23]

If I understand correctly, you detect that the timer has restarted by the fact
that it picked up the modified login delay. Could you document this in a
comment?

[dconnelly, 2013/02/26 18:04:15]

Done.

+  euc->OnUserActivity();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(),
+            kAutoLoginShortDelay);
+
+  content::RunAllPendingInMessageLoop();
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       ConfigureAutoLogin) {
+  ExistingUserController* euc = existing_user_controller();
+  ash::UserActivityDetector* detector =
+    ash::Shell::GetInstance()->user_activity_detector();
+  euc->OnSigninScreenReady();
+
+  // Turn off settings notifications to EUC.

[bartfab (slow), 2013/02/25 16:51:23]

Just as in variable names, you should not be using abbreviations in comments.

[dconnelly, 2013/02/26 18:04:15]

Done.

+  DisableAutoLoginPolicyNotifications();
+
+  // Timer shouldn't start when the policy is disabled.
+  SetAutoLoginPolicy("", 0);
+  euc->ConfigurePublicSessionAutoLogin();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(), 0);
+  ASSERT_EQ(auto_login_delay(), 0);
+  ASSERT_EQ(auto_login_username(), "");
+  ASSERT_FALSE(detector->HasObserver(euc));
+
+  // Timer shouldn't start when the delay alone is set.
+  SetAutoLoginPolicy("", kAutoLoginShortDelay);
+  euc->ConfigurePublicSessionAutoLogin();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(), 0);
+  ASSERT_EQ(auto_login_delay(), kAutoLoginShortDelay);
+  ASSERT_EQ(auto_login_username(), "");
+  ASSERT_FALSE(detector->HasObserver(euc));
+
+  // Timer should start when the username is set.
+  SetAutoLoginPolicy(kAutoLoginUsername, kAutoLoginShortDelay);
+  euc->ConfigurePublicSessionAutoLogin();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(),
+            kAutoLoginShortDelay);
+  ASSERT_EQ(auto_login_delay(), kAutoLoginShortDelay);
+  ASSERT_EQ(auto_login_username(), kAutoLoginUsername);
+  ASSERT_TRUE(detector->HasObserver(euc));
+
+  // Timer should restart when the delay is changed.
+  SetAutoLoginPolicy(kAutoLoginUsername, kAutoLoginLongDelay);
+  euc->ConfigurePublicSessionAutoLogin();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(),
+            kAutoLoginLongDelay);
+  ASSERT_EQ(auto_login_delay(), kAutoLoginLongDelay);
+  ASSERT_EQ(auto_login_username(), kAutoLoginUsername);
+  ASSERT_TRUE(detector->HasObserver(euc));
+
+  // Timer should stop when the username is unset.
+  SetAutoLoginPolicy("", kAutoLoginLongDelay);
+  euc->ConfigurePublicSessionAutoLogin();
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+  ASSERT_EQ(auto_login_timer()->GetCurrentDelay().InMilliseconds(),
+            kAutoLoginLongDelay);
+  ASSERT_EQ(auto_login_username(), "");
+  ASSERT_EQ(auto_login_delay(), kAutoLoginLongDelay);
+  ASSERT_FALSE(detector->HasObserver(euc));
+
+  content::RunAllPendingInMessageLoop();
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       ConfigureAutoLoginUsingPolicy) {
+  ExistingUserController* euc = existing_user_controller();
+  ash::UserActivityDetector* detector =
+    ash::Shell::GetInstance()->user_activity_detector();
+  euc->OnSigninScreenReady();
+  ASSERT_EQ("", auto_login_username());
+  ASSERT_EQ(0, auto_login_delay());
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+  ASSERT_FALSE(detector->HasObserver(euc));
+
+  // Set the policy.
+  SetAutoLoginPolicy(kAutoLoginUsername, kAutoLoginLongDelay);
+  ASSERT_EQ(kAutoLoginUsername, auto_login_username());
+  ASSERT_EQ(kAutoLoginLongDelay, auto_login_delay());
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+  ASSERT_TRUE(detector->HasObserver(euc));
+
+  // Unset the policy.
+  SetAutoLoginPolicy("", 0);
+  ASSERT_EQ("", auto_login_username());
+  ASSERT_EQ(0, auto_login_delay());
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+  ASSERT_FALSE(detector->HasObserver(euc));
+
+  content::RunAllPendingInMessageLoop();
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       OnAutoLoginFire) {
+  PrepareLogin(kAutoLoginUsername, "", CreateAuthenticatorPublicSession);
+  existing_user_controller()->OnSigninScreenReady();
+
+  // Directly set auto-login parameters to avoid starting the timer.
+  set_auto_login_username(kAutoLoginUsername);
+  set_auto_login_delay(kAutoLoginLongDelay);
+
+  // Manually fire auto-login.
+  existing_user_controller()->OnPublicSessionAutoLoginTimerFire();
+  content::RunAllPendingInMessageLoop();

[bartfab (slow), 2013/02/25 16:51:23]

Nit: You have an emtpy line before the content::RunAllPendingInMessageLoop() at
the end in some tests and not in others. It would be nice to be consistent here.

[dconnelly, 2013/02/26 18:04:15]

Done.

+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       AutoLoginNoDelay) {
+  PrepareLogin(kAutoLoginUsername, "", CreateAuthenticatorPublicSession);
+  existing_user_controller()->OnSigninScreenReady();
+  SetAutoLoginPolicy(kAutoLoginUsername, kAutoLoginNoDelay);
+  // Timer should fire immediately.

[bartfab (slow), 2013/02/25 16:51:23]

Here and in the other tests: Can you document that PrepareLogin() sets up an
expectation that verifies the login happened?

[dconnelly, 2013/02/26 18:04:15]

Done.

+  content::RunAllPendingInMessageLoop();
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       AutoLoginShortDelay) {
+  PrepareLogin(kAutoLoginUsername, "", CreateAuthenticatorPublicSession);
+  existing_user_controller()->OnSigninScreenReady();
+  SetAutoLoginPolicy(kAutoLoginUsername, kAutoLoginShortDelay);
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+
+  // Wait for the timer to fire.
+  content::MessageLoopRunner* runner = new content::MessageLoopRunner;
+  base::OneShotTimer<content::MessageLoopRunner> timer;
+  timer.Start(FROM_HERE,
+              base::TimeDelta::FromMilliseconds(2 * kAutoLoginShortDelay),

[bartfab (slow), 2013/02/25 16:51:23]

Boo, waiting 200ms :(. Could you inspect the time the timer is set to and then
fast-forward the message loop by 100ms?

[dconnelly, 2013/02/26 18:04:15]

There's no real point to this test, then.  There's already a test that verifies
that updating the policy updates the timer and a test when the timer is zero. 
Should I just delete this one?

[bartfab (slow), 2013/02/28 10:21:33]

No, I think there is value in it - I would keep the test. In the light of all
our slow and flaky tests, 200ms is not going to hurt anyone :).

What I meant was to cheat the timer and make it think that 200ms have passed
already. But this is quite tricky to do.

+              base::Bind(&content::MessageLoopRunner::Quit,
+                         base::Unretained(runner)));
+  runner->Run();
+  content::RunAllPendingInMessageLoop();
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       LoginStopsAutoLogin) {
+  PrepareLogin(kUsername, kPassword, CreateAuthenticator);
+
+  ExistingUserController* euc = existing_user_controller();
+  euc->OnSigninScreenReady();
+  set_auto_login_username(kAutoLoginUsername);
+  set_auto_login_delay(kAutoLoginLongDelay);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+
+  // Login and check that it stopped the timer.
+  euc->Login(kUsername, kPassword);
+  ASSERT_TRUE(is_login_in_progress());
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  content::RunAllPendingInMessageLoop();
+
+  // Timer should still be stopped after login completes.
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       RetailModeLoginStopsAutoLogin) {

[bartfab (slow), 2013/02/25 16:51:23]

This is not necessary. Public sessions and retail mode cannot be mixed.

[dconnelly, 2013/02/26 18:04:15]

Done.

+  PrepareLogin(kRetailModeUserEMail, "", CreateAuthenticator);
+
+  ExistingUserController* euc = existing_user_controller();
+  euc->OnSigninScreenReady();
+  set_auto_login_username(kAutoLoginUsername);
+  set_auto_login_delay(kAutoLoginLongDelay);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+
+  // Login and check that it stopped the timer.
+  euc->LoginAsRetailModeUser();
+  ASSERT_TRUE(is_login_in_progress());
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  content::RunAllPendingInMessageLoop();
+
+  // Timer should still be stopped after login completes.
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       GuestModeLoginStopsAutoLogin) {
+  EXPECT_CALL(*mock_login_display_, SetUIEnabled(false))
+      .Times(1);
+  EXPECT_CALL(*mock_login_utils_, CreateAuthenticator(_))
+      .Times(1)
+      .WillOnce(WithArg<0>(Invoke(CreateAuthenticator)));
+  ScopedMockUserManagerEnabler mock_user_manager;
+  EXPECT_CALL(*mock_login_utils_, CompleteOffTheRecordLogin(_))
+      .Times(1);
+
+  ExistingUserController* euc = existing_user_controller();
+  euc->OnSigninScreenReady();
+  set_auto_login_username(kAutoLoginUsername);
+  set_auto_login_delay(kAutoLoginLongDelay);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+
+  // Login and check that it stopped the timer.
+  euc->LoginAsGuest();
+  ASSERT_TRUE(is_login_in_progress());
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+
+  content::RunAllPendingInMessageLoop();
+
+  // Timer should still be stopped after login completes.
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       CompleteLoginStopsAutoLogin) {
+  PrepareLogin(kUsername, kPassword, CreateAuthenticator);
+  EXPECT_CALL(*mock_login_display_host_, OnCompleteLogin())
+      .Times(1);
+
+  ExistingUserController* euc = existing_user_controller();
+  euc->OnSigninScreenReady();
+  set_auto_login_username(kAutoLoginUsername);
+  set_auto_login_delay(kAutoLoginLongDelay);
+  euc->StartPublicSessionAutoLoginTimer();
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+
+  // Check that login completes and stops the timer.
+  euc->CompleteLogin(kUsername, kPassword);
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+  content::RunAllPendingInMessageLoop();
+
+  // Timer should still be stopped after login completes.
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+}
+
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
+                       PublicSessionLoginStopsAutoLogin) {
+  PrepareLogin(kAutoLoginUsername, "", CreateAuthenticatorPublicSession);
+  existing_user_controller()->OnSigninScreenReady();
+  SetAutoLoginPolicy(kAutoLoginUsername, kAutoLoginShortDelay);
+  ASSERT_TRUE(auto_login_timer()->IsRunning());
+
+  // Login and check that it stopped the timer.
+  existing_user_controller()->LoginAsPublicAccount(kAutoLoginUsername);
+  ASSERT_TRUE(is_login_in_progress());
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+  content::RunAllPendingInMessageLoop();
+
+  // Timer should still be stopped after login completes.
+  ASSERT_FALSE(auto_login_timer()->IsRunning());
+}
+
 }  // namespace chromeos