Do not reload account picker when device-local account policy changes

chrome/browser/chromeos/policy/device_local_account_browsertest.cc

 // Copyright (c) 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 <map>
 #include <string>
 
 #include "apps/app_window_registry.h"
 #include "apps/ui/native_app_window.h"
 #include "ash/shell.h"
 #include "ash/system/chromeos/session/logout_confirmation_controller.h"
 #include "ash/system/chromeos/session/logout_confirmation_dialog.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/location.h"
+#include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/path_service.h"
+#include "base/prefs/pref_change_registrar.h"
 #include "base/prefs/pref_service.h"
 #include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 96 matching lines @@
 using testing::Return;
 using testing::_;
 
 namespace policy {
 
 namespace {
 
 const char kDomain[] = "example.com";
 const char kAccountId1[] = "dla1@example.com";
 const char kAccountId2[] = "dla2@example.com";
-const char kDisplayName[] = "display name";
+const char kDisplayName1[] = "display name 1";
+const char kDisplayName2[] = "display name 2";
 const char* kStartupURLs[] = {
   "chrome://policy",
   "chrome://about",
 };
 const char kExistentTermsOfServicePath[] = "chromeos/enterprise/tos.txt";
 const char kNonexistentTermsOfServicePath[] = "chromeos/enterprise/tos404.txt";
 const char kRelativeUpdateURL[] = "/service/update2/crx";
 const char kUpdateManifestHeader[] =
     "<?xml version='1.0' encoding='UTF-8'?>\n"
     "<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>\n";
@@ skipping 46 matching lines @@
     GURL crx_url;
   };
   typedef std::map<std::string, Update> UpdateMap;
   UpdateMap updates_;
 
   const std::string relative_update_url_;
 
   DISALLOW_COPY_AND_ASSIGN(TestingUpdateManifestProvider);
 };
 
+// Helper that observes the dictionary |pref| in local state and waits until the
+// value stored for |key| matches |expected_value|.
+class DictionaryPrefValueWaiter {
+ public:
+  DictionaryPrefValueWaiter(const std::string& pref,
+                            const std::string& key,
+                            const std::string& expected_value);
+  ~DictionaryPrefValueWaiter();
+
+  void Wait();
+
+ private:
+  void QuitLoopIfExpectedValueFound();
+
+  const std::string pref_;
+  const std::string key_;
+  const std::string expected_value_;
+
+  base::RunLoop run_loop_;
+  PrefChangeRegistrar pref_change_registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(DictionaryPrefValueWaiter);
+};
+
 TestingUpdateManifestProvider::Update::Update(const std::string& version,
                                               const GURL& crx_url)
     : version(version),
       crx_url(crx_url) {
 }
 
 TestingUpdateManifestProvider::Update::Update() {
 }
 
 TestingUpdateManifestProvider::TestingUpdateManifestProvider(
@@ skipping 36 matching lines @@
   }
   content += kUpdateManifestFooter;
   scoped_ptr<net::test_server::BasicHttpResponse>
       http_response(new net::test_server::BasicHttpResponse);
   http_response->set_code(net::HTTP_OK);
   http_response->set_content(content);
   http_response->set_content_type("text/xml");
   return http_response.PassAs<net::test_server::HttpResponse>();
 }
 
+DictionaryPrefValueWaiter::DictionaryPrefValueWaiter(
+    const std::string& pref,
+    const std::string& key,
+    const std::string& expected_value)
+    : pref_(pref),
+      key_(key),
+      expected_value_(expected_value) {
+  pref_change_registrar_.Init(g_browser_process->local_state());
+}
+
+DictionaryPrefValueWaiter::~DictionaryPrefValueWaiter() {
+}
+
+void DictionaryPrefValueWaiter::Wait() {
+  pref_change_registrar_.Add(
+      pref_.c_str(),
+      base::Bind(&DictionaryPrefValueWaiter::QuitLoopIfExpectedValueFound,
+                 base::Unretained(this)));
+  QuitLoopIfExpectedValueFound();
+  run_loop_.Run();
+}
+
+void DictionaryPrefValueWaiter::QuitLoopIfExpectedValueFound() {
+  const base::DictionaryValue* pref =
+      pref_change_registrar_.prefs()->GetDictionary(pref_.c_str());
+  ASSERT_TRUE(pref);
+  std::string actual_value;
+  if (pref->GetStringWithoutPathExpansion(key_, &actual_value) &&
+      actual_value == expected_value_) {
+    run_loop_.Quit();
+  }
+}
+
 bool DoesInstallSuccessReferToId(const std::string& id,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   return content::Details<const extensions::InstalledExtensionInfo>(details)->
       extension->id() == id;
 }
 
 bool DoesInstallFailureReferToId(const std::string& id,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   return content::Details<const base::string16>(details)->
       find(base::UTF8ToUTF16(id)) != base::string16::npos;
 }
 
 scoped_ptr<net::FakeURLFetcher> RunCallbackAndReturnFakeURLFetcher(
     scoped_refptr<base::SequencedTaskRunner> task_runner,
     const base::Closure& callback,
     const GURL& url,
     net::URLFetcherDelegate* delegate,
     const std::string& response_data,
     net::HttpStatusCode response_code,
     net::URLRequestStatus::Status status) {
   task_runner->PostTask(FROM_HERE, callback);
   return make_scoped_ptr(new net::FakeURLFetcher(
       url, delegate, response_data, response_code, status));
 }
 
-bool DisplayNameMatches(const std::string& account_id,
-                        const std::string& display_name) {
-  const user_manager::User* user =
-      chromeos::UserManager::Get()->FindUser(account_id);
-  if (!user || user->display_name().empty())
-    return false;
-  EXPECT_EQ(base::UTF8ToUTF16(display_name), user->display_name());
-  return true;
-}
-
 bool IsSessionStarted() {
   return chromeos::UserManager::Get()->IsSessionStarted();
 }
 
 }  // namespace
 
 class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest,
                                public chromeos::UserManager::Observer,
                                public chrome::BrowserListObserver,
                                public apps::AppWindowRegistry::Observer {
@@ skipping 87 matching lines @@
     em::ChromeDeviceSettingsProto& proto(device_policy()->payload());
     proto.mutable_show_user_names()->set_show_user_names(true);
 
     device_local_account_policy_.policy_data().set_policy_type(
         dm_protocol::kChromePublicAccountPolicyType);
     device_local_account_policy_.policy_data().set_username(kAccountId1);
     device_local_account_policy_.policy_data().set_settings_entity_id(
         kAccountId1);
     device_local_account_policy_.policy_data().set_public_key_version(1);
     device_local_account_policy_.payload().mutable_userdisplayname()->set_value(
-        kDisplayName);
+        kDisplayName1);
   }
 
   void BuildDeviceLocalAccountPolicy() {
     device_local_account_policy_.SetDefaultSigningKey();
     device_local_account_policy_.Build();
   }
 
   void UploadDeviceLocalAccountPolicy() {
     BuildDeviceLocalAccountPolicy();
     test_server_.UpdatePolicy(
@@ skipping 43 matching lines @@
     return GetExtensionCacheDirectoryForAccountID(account_id)
         .Append(base::StringPrintf("%s-%s.crx", id.c_str(), version.c_str()));
   }
 
   // Returns a profile which can be used for testing.
   Profile* GetProfileForTest() {
     // Any profile can be used here since this test does not test multi profile.
     return ProfileManager::GetActiveUserProfile();
   }
 
+  void WaitForDisplayName(const std::string& user_id,
+                          const std::string& expected_display_name) {
+    DictionaryPrefValueWaiter("UserDisplayName",
+                              user_id,
+                              expected_display_name).Wait();
+  }
+
   void WaitForPolicy() {
-    // This observes the display name becoming available as this indicates
+    // Wait for the display name becoming available as that indicates
     // device-local account policy is fully loaded, which is a prerequisite for
     // successful login.
-    content::WindowedNotificationObserver(
-        chrome::NOTIFICATION_USER_LIST_CHANGED,
-        base::Bind(&DisplayNameMatches, user_id_1_, kDisplayName)).Wait();
+    WaitForDisplayName(user_id_1_, kDisplayName1);
   }
 
   void WaitForLoginUI() {
     // Wait for the login UI to be ready.
     chromeos::LoginDisplayHostImpl* host =
         reinterpret_cast<chromeos::LoginDisplayHostImpl*>(
             chromeos::LoginDisplayHostImpl::default_host());
     ASSERT_TRUE(host);
     chromeos::OobeUI* oobe_ui = host->GetOobeUI();
     ASSERT_TRUE(oobe_ui);
@@ skipping 46 matching lines @@
   return chromeos::UserManager::Get()->IsKnownUser(account_id);
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, LoginScreen) {
   AddPublicSessionToDevicePolicy(kAccountId1);
   AddPublicSessionToDevicePolicy(kAccountId2);
 
   content::WindowedNotificationObserver(chrome::NOTIFICATION_USER_LIST_CHANGED,
                                         base::Bind(&IsKnownUser, user_id_1_))
       .Wait();
-  content::WindowedNotificationObserver(chrome::NOTIFICATION_USER_LIST_CHANGED,
-                                        base::Bind(&IsKnownUser, user_id_2_))
-      .Wait();
+  EXPECT_TRUE(IsKnownUser(user_id_2_));
 
   CheckPublicSessionPresent(user_id_1_);
   CheckPublicSessionPresent(user_id_2_);
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, DisplayName) {
   UploadAndInstallDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
 
   WaitForPolicy();
+
+  // Skip to the login screen.
+  chromeos::WizardController* wizard_controller =
+      chromeos::WizardController::default_controller();
+  ASSERT_TRUE(wizard_controller);
+  wizard_controller->SkipToLoginForTesting(LoginScreenContext());
+  content::WindowedNotificationObserver(
+      chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
+      content::NotificationService::AllSources()).Wait();
+
+  // Verify that the display name is shown in the UI.
+  chromeos::LoginDisplayHostImpl* host =
+      reinterpret_cast<chromeos::LoginDisplayHostImpl*>(
+          chromeos::LoginDisplayHostImpl::default_host());
+  ASSERT_TRUE(host);
+  chromeos::WebUILoginView* web_ui_login_view = host->GetWebUILoginView();
+  ASSERT_TRUE(web_ui_login_view);
+  content::WebUI* web_ui = web_ui_login_view->GetWebUI();
+  ASSERT_TRUE(web_ui);
+  content::WebContents* contents = web_ui->GetWebContents();
+  ASSERT_TRUE(contents);
+  const std::string get_compact_pod_display_name = base::StringPrintf(
+      "domAutomationController.send(document.getElementById('pod-row')"
+      "    .getPodWithUsername_('%s').nameElement.textContent);",
+      user_id_1_.c_str());
+  std::string display_name;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      contents,
+      get_compact_pod_display_name,
+      &display_name));
+  EXPECT_EQ(kDisplayName1, display_name);
+  const std::string get_expanded_pod_display_name = base::StringPrintf(
+      "domAutomationController.send(document.getElementById('pod-row')"
+      "    .getPodWithUsername_('%s').querySelector('.expanded-pane-name')"
+      "        .textContent);",
+      user_id_1_.c_str());
+  display_name.clear();
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      contents,
+      get_expanded_pod_display_name,
+      &display_name));
+  EXPECT_EQ(kDisplayName1, display_name);
+
+  // Click on the pod to expand it.
+  ASSERT_TRUE(content::ExecuteScript(
+      contents,
+      base::StringPrintf(
+          "document.getElementById('pod-row').getPodWithUsername_('%s')"
+          "    .click();",
+      user_id_1_.c_str())));
+
+  // Change the display name.
+  device_local_account_policy_.payload().mutable_userdisplayname()->set_value(
+      kDisplayName2);
+  UploadAndInstallDeviceLocalAccountPolicy();
+  policy::BrowserPolicyConnectorChromeOS* connector =
+      g_browser_process->platform_part()->browser_policy_connector_chromeos();
+  DeviceLocalAccountPolicyBroker* broker =
+      connector->GetDeviceLocalAccountPolicyService()->GetBrokerForUser(
+          user_id_1_);
+  ASSERT_TRUE(broker);
+  broker->core()->store()->Load();
+  WaitForDisplayName(user_id_1_, kDisplayName2);
+
+  // Verify that the new display name is shown in the UI.
+  display_name.clear();
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      contents,
+      get_compact_pod_display_name,
+      &display_name));
+  EXPECT_EQ(kDisplayName2, display_name);
+  display_name.clear();
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      contents,
+      get_expanded_pod_display_name,
+      &display_name));
+  EXPECT_EQ(kDisplayName2, display_name);
+
+  // Verify that the pod is still expanded. This indicates that the UI updated
+  // without reloading and losing state.
+  bool expanded = false;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+      contents,
+      base::StringPrintf(
+          "domAutomationController.send(document.getElementById('pod-row')"
+          "    .getPodWithUsername_('%s').expanded);",
+          user_id_1_.c_str()),
+      &expanded));
+  EXPECT_TRUE(expanded);
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, PolicyDownload) {
   UploadDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
 
   WaitForPolicy();
 
   // Sanity check: The policy should be present now.
   ASSERT_FALSE(session_manager_client()->device_local_account_policy(
       kAccountId1).empty());
 }
 
 static bool IsNotKnownUser(const std::string& account_id) {
   return !IsKnownUser(account_id);
 }
 
-IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, DevicePolicyChange) {
+IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, AccountListChange) {
   AddPublicSessionToDevicePolicy(kAccountId1);
   AddPublicSessionToDevicePolicy(kAccountId2);
 
-  // Wait until the login screen is up.
   content::WindowedNotificationObserver(chrome::NOTIFICATION_USER_LIST_CHANGED,
                                         base::Bind(&IsKnownUser, user_id_1_))
       .Wait();
-  content::WindowedNotificationObserver(chrome::NOTIFICATION_USER_LIST_CHANGED,
-                                        base::Bind(&IsKnownUser, user_id_2_))
-      .Wait();
+  EXPECT_TRUE(IsKnownUser(user_id_2_));
 
   // Update policy to remove kAccountId2.
   em::ChromeDeviceSettingsProto& proto(device_policy()->payload());
   proto.mutable_device_local_accounts()->clear_account();
   AddPublicSessionToDevicePolicy(kAccountId1);
 
   em::ChromeDeviceSettingsProto policy;
   policy.mutable_show_user_names()->set_show_user_names(true);
   em::DeviceLocalAccountInfoProto* account1 =
       policy.mutable_device_local_accounts()->add_account();
   account1->set_account_id(kAccountId1);
   account1->set_type(
       em::DeviceLocalAccountInfoProto::ACCOUNT_TYPE_PUBLIC_SESSION);
 
   test_server_.UpdatePolicy(dm_protocol::kChromeDevicePolicyType, std::string(),
                             policy.SerializeAsString());
   g_browser_process->policy_service()->RefreshPolicies(base::Closure());
 
   // Make sure the second device-local account disappears.
-  content::WindowedNotificationObserver(
-      chrome::NOTIFICATION_USER_LIST_CHANGED,
-      base::Bind(&IsNotKnownUser, user_id_2_)).Wait();
+  content::WindowedNotificationObserver(chrome::NOTIFICATION_USER_LIST_CHANGED,
+                                        base::Bind(&IsNotKnownUser, user_id_2_))
+      .Wait();
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, StartSession) {
   // Specify startup pages.
   device_local_account_policy_.payload().mutable_restoreonstartup()->set_value(
       SessionStartupPref::kPrefValueURLs);
   em::StringListPolicyProto* startup_urls_proto =
       device_local_account_policy_.payload().mutable_restoreonstartupurls();
   for (size_t i = 0; i < arraysize(kStartupURLs); ++i)
     startup_urls_proto->mutable_value()->add_entries(kStartupURLs[i]);
@@ skipping 803 matching lines @@
   ASSERT_TRUE(content::ExecuteScript(contents,
                                      "$('tos-accept-button').click();"));
 
   WaitForSessionStart();
 }
 
 INSTANTIATE_TEST_CASE_P(TermsOfServiceDownloadTestInstance,
                         TermsOfServiceDownloadTest, testing::Bool());
 
 }  // namespace policy