Do not switch the ICU locale when interacting with public session pods

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 <set>
 #include <string>
 #include <vector>
 
 #include "apps/app_window_registry.h"
@@ skipping 110 matching lines @@
 #include "net/base/url_util.h"
 #include "net/http/http_status_code.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_status.h"
 #include "policy/policy_constants.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "third_party/icu/source/common/unicode/locid.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/views/widget/widget.h"
 #include "url/gurl.h"
-//#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace em = enterprise_management;
 
 using chromeos::LoginScreenContext;
 using testing::InvokeWithoutArgs;
 using testing::Return;
 using testing::_;
 
 namespace policy {
 
@@ skipping 306 matching lines @@
 
     InstallOwnerKey();
     MarkAsEnterpriseOwned();
 
     InitializePolicy();
   }
 
   virtual void SetUpOnMainThread() OVERRIDE {
     DevicePolicyCrosBrowserTest::SetUpOnMainThread();
 
+    initial_locale_ = g_browser_process->GetApplicationLocale();
+
     content::WindowedNotificationObserver(
         chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
         content::NotificationService::AllSources()).Wait();
 
     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);
@@ skipping 221 matching lines @@
     EXPECT_EQ(layouts_from_locale.front(),
               input_method_manager->GetActiveIMEState()
                   ->GetCurrentInputMethod()
                   .id());
   }
 
   const std::string user_id_1_;
   const std::string user_id_2_;
   const std::string public_session_input_method_id_;
 
+  std::string initial_locale_;
+
   scoped_ptr<base::RunLoop> run_loop_;
 
   UserPolicyBuilder device_local_account_policy_;
   LocalPolicyTestServer test_server_;
 
   content::WebContents* contents_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountTest);
 };
@@ skipping 676 matching lines @@
 
   // Deny the logout.
   dialog->GetWidget()->Close();
   dialog = NULL;
   base::RunLoop().RunUntilIdle();
 
   app_window_registry->RemoveObserver(this);
 };
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, NoRecommendedLocaleNoSwitch) {
-  const std::string initial_locale = g_browser_process->GetApplicationLocale();
-
   UploadAndInstallDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
 
   WaitForPolicy();
 
   // Click on the pod to expand it. Verify that the pod expands to its basic
   // form as there are no recommended locales.
   bool advanced = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       contents_,
       base::StringPrintf(
           "var pod ="
           "    document.getElementById('pod-row').getPodWithUsername_('%s');"
           "pod.click();"
           "domAutomationController.send(pod.classList.contains('advanced'));",
           user_id_1_.c_str()),
       &advanced));
   EXPECT_FALSE(advanced);
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),

[pneubeck (no reviews), 2014/08/19 15:09:30]

from reading this file and the function declaration of
BrowserProcess::GetApplicationLocale(), it's unclear why you would even mention
icu::Locale::getDefault() in this file.

Could you at least wrap the comparison like here (compare only initial_locale_
to Locale::getDefault)
and like in lines 1437-1439 into helper functions, where you document the reason
for the comparison to icu::Locale::getDefault() ?

[bartfab (slow), 2014/08/19 17:03:01]

I added comments and made this code a bit more concise. I specifically did not
want to move the comparisons to helper functions. This is because a failure in
the main test body gives you a useful line number while a failure in a helper
function only points you at the helper, making it impossible to tell which
invocation of the helper failed.

[pneubeck (no reviews), 2014/08/20 09:03:40]

FROM_HERE ? :-)
I suggested that to keep the redundancy low, now you have a 10x duplication of a
two-line comment. And if you have to ever change this...

Anyways it's not dramatic, so your choice.

[bartfab (slow), 2014/08/20 17:52:28]

OK, you got me convinced. I moved the locale checks into helper methods that
existed already anyway and one additional helper that I could have extracted a
while ago. Things are cleaner and shorter that way.

+            icu::Locale::getDefault().getLanguage());
 
   // Click the enter button to start the session.
   ASSERT_TRUE(content::ExecuteScript(
       contents_,
       base::StringPrintf(
           "document.getElementById('pod-row').getPodWithUsername_('%s')"
           "    .querySelector('.enter-button').click();",
           user_id_1_.c_str())));
 
   WaitForSessionStart();
 
   // Verify that the locale has not changed and the first keyboard layout
   // applicable to the locale was chosen.
-  EXPECT_EQ(initial_locale, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(initial_locale_, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),
+            icu::Locale::getDefault().getLanguage());
   VerifyKeyboardLayoutMatchesLocale();
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, NoRecommendedLocaleSwitch) {
   UploadAndInstallDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
 
   WaitForPolicy();
 
   // Click on the pod to expand it. Verify that the pod expands to its basic
   // form as there are no recommended locales.
   bool advanced = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       contents_,
       base::StringPrintf(
           "var pod ="
           "    document.getElementById('pod-row').getPodWithUsername_('%s');"
           "pod.click();"
           "domAutomationController.send(pod.classList.contains('advanced'));",
           user_id_1_.c_str()),
       &advanced));
   EXPECT_FALSE(advanced);
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),
+            icu::Locale::getDefault().getLanguage());
 
   // Click the link that switches the pod to its advanced form. Verify that the
   // pod switches from basic to advanced.
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       contents_,
       base::StringPrintf(
           "var pod ="
           "    document.getElementById('pod-row').getPodWithUsername_('%s');"
           "pod.querySelector('.language-and-input').click();"
           "domAutomationController.send(pod.classList.contains('advanced'));",
@@ skipping 10 matching lines @@
           "languageSelect.value = '%s';"
           "var event = document.createEvent('HTMLEvents');"
           "event.initEvent('change', false, true);"
           "languageSelect.dispatchEvent(event);",
           user_id_1_.c_str(),
           kPublicSessionLocale)));
 
   // The UI will have requested an updated list of keyboard layouts at this
   // point. Wait for the constructions of this list to finish.
   WaitForGetKeyboardLayoutsForLocaleToFinish();
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),
+            icu::Locale::getDefault().getLanguage());
 
   // Manually select a different keyboard layout and click the enter button to
   // start the session.
   ASSERT_TRUE(content::ExecuteScript(
       contents_,
       base::StringPrintf(
           "var pod ="
           "    document.getElementById('pod-row').getPodWithUsername_('%s');"
           "pod.querySelector('.keyboard-select').value = '%s';"
           "pod.querySelector('.enter-button').click();",
           user_id_1_.c_str(),
           public_session_input_method_id_.c_str())));
 
   WaitForSessionStart();
 
   // Verify that the locale and keyboard layout have been applied.
   EXPECT_EQ(kPublicSessionLocale, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(kPublicSessionLocale),
+            icu::Locale::getDefault().getLanguage());
   EXPECT_EQ(public_session_input_method_id_,
             chromeos::input_method::InputMethodManager::Get()
                 ->GetActiveIMEState()
                 ->GetCurrentInputMethod()
                 .id());
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, OneRecommendedLocale) {
   // Specify a recommended locale.
   SetRecommendedLocales(kSingleRecommendedLocale,
                         arraysize(kSingleRecommendedLocale));
   UploadAndInstallDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
 
   WaitForPolicy();
 
   // Click on the pod to expand it. Verify that the pod expands to its basic
   // form as there is only one recommended locale.
   bool advanced = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       contents_,
       base::StringPrintf(
           "var pod ="
           "    document.getElementById('pod-row').getPodWithUsername_('%s');"
           "pod.click();"
           "domAutomationController.send(pod.classList.contains('advanced'));",
           user_id_1_.c_str()),
       &advanced));
   EXPECT_FALSE(advanced);
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),
+            icu::Locale::getDefault().getLanguage());
 
   // Click the enter button to start the session.
   ASSERT_TRUE(content::ExecuteScript(
       contents_,
       base::StringPrintf(
           "document.getElementById('pod-row').getPodWithUsername_('%s')"
           "    .querySelector('.enter-button').click();",
           user_id_1_.c_str())));
 
   WaitForSessionStart();
 
   // Verify that the recommended locale has been applied and the first keyboard
   // layout applicable to the locale was chosen.
   EXPECT_EQ(kSingleRecommendedLocale[0],
             g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(kSingleRecommendedLocale[0]),
+            icu::Locale::getDefault().getLanguage());
   VerifyKeyboardLayoutMatchesLocale();
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, MultipleRecommendedLocales) {
   // Specify recommended locales.
   SetRecommendedLocales(kRecommendedLocales1, arraysize(kRecommendedLocales1));
   UploadAndInstallDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
   AddPublicSessionToDevicePolicy(kAccountId2);
 
   WaitForPolicy();
 
   // Click on the pod to expand it. Verify that the pod expands to its advanced
   // form directly as there are two or more recommended locales.
   bool advanced = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       contents_,
       base::StringPrintf(
           "var pod ="
           "    document.getElementById('pod-row').getPodWithUsername_('%s');"
           "pod.click();"
           "domAutomationController.send(pod.classList.contains('advanced'));",
           user_id_1_.c_str()),
       &advanced));
   EXPECT_TRUE(advanced);
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),
+            icu::Locale::getDefault().getLanguage());
 
   // Verify that the pod shows a list of locales beginning with the recommended
   // ones, followed by others.
   const std::string get_locale_list = base::StringPrintf(
       "var languageSelect = document.getElementById('pod-row')"
       "    .getPodWithUsername_('%s').querySelector('.language-select');"
       "var locales = [];"
       "for (var i = 0; i < languageSelect.length; ++i)"
       "  locales.push(languageSelect.options[i].value);"
       "domAutomationController.send(JSON.stringify(locales));",
@@ skipping 101 matching lines @@
 
   // Verify that the manually selected locale is still selected.
   ASSERT_TRUE(content::ExecuteScriptAndExtractString(contents_,
                                                      get_selected_locale,
                                                      &selected_locale));
   EXPECT_EQ(kPublicSessionLocale, selected_locale);
 
   // The UI will request an updated list of keyboard layouts at this point. Wait
   // for the constructions of this list to finish.
   WaitForGetKeyboardLayoutsForLocaleToFinish();
+  EXPECT_EQ(initial_locale_,
+            g_browser_process->GetApplicationLocale());
 
   // Manually select a different keyboard layout.
   ASSERT_TRUE(content::ExecuteScript(
       contents_,
       base::StringPrintf(
           "document.getElementById('pod-row').getPodWithUsername_('%s')"
           "    .querySelector('.keyboard-select').value = '%s';",
           user_id_1_.c_str(),
           public_session_input_method_id_.c_str())));
 
@@ skipping 41 matching lines @@
       contents_,
       base::StringPrintf(
           "document.getElementById('pod-row').getPodWithUsername_('%s')"
           "    .querySelector('.enter-button').click();",
           user_id_1_.c_str())));
 
   WaitForSessionStart();
 
   // Verify that the locale and keyboard layout have been applied.
   EXPECT_EQ(kPublicSessionLocale, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(kPublicSessionLocale),
+            icu::Locale::getDefault().getLanguage());
   EXPECT_EQ(public_session_input_method_id_,
             chromeos::input_method::InputMethodManager::Get()
                 ->GetActiveIMEState()
                 ->GetCurrentInputMethod()
                 .id());
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest,
                        AutoLoginWithoutRecommendedLocales) {
-  const std::string initial_locale = g_browser_process->GetApplicationLocale();
-
   UploadAndInstallDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
   EnableAutoLogin();
 
   WaitForPolicy();
 
   WaitForSessionStart();
 
   // Verify that the locale has not changed and the first keyboard layout
   // applicable to the locale was chosen.
-  EXPECT_EQ(initial_locale, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(initial_locale_, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),
+            icu::Locale::getDefault().getLanguage());
   VerifyKeyboardLayoutMatchesLocale();
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest,
                        AutoLoginWithRecommendedLocales) {
   // Specify recommended locales.
   SetRecommendedLocales(kRecommendedLocales1, arraysize(kRecommendedLocales1));
   UploadAndInstallDeviceLocalAccountPolicy();
   AddPublicSessionToDevicePolicy(kAccountId1);
   EnableAutoLogin();
 
   WaitForPolicy();
 
   WaitForSessionStart();
 
   // Verify that the first recommended locale has been applied and the first
   // keyboard layout applicable to the locale was chosen.
   EXPECT_EQ(kRecommendedLocales1[0], g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(kRecommendedLocales1[0]),
+            icu::Locale::getDefault().getLanguage());
   VerifyKeyboardLayoutMatchesLocale();
 }
 
 IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, TermsOfServiceWithLocaleSwitch) {
   // Specify Terms of Service URL.
   ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
   device_local_account_policy_.payload().mutable_termsofserviceurl()->set_value(
       embedded_test_server()->GetURL(
           std::string("/") + kExistentTermsOfServicePath).spec());
   UploadAndInstallDeviceLocalAccountPolicy();
@@ skipping 10 matching lines @@
           "languageSelect.value = '%s';"
           "var event = document.createEvent('HTMLEvents');"
           "event.initEvent('change', false, true);"
           "languageSelect.dispatchEvent(event);",
           user_id_1_.c_str(),
           kPublicSessionLocale)));
 
   // The UI will have requested an updated list of keyboard layouts at this
   // point. Wait for the constructions of this list to finish.
   WaitForGetKeyboardLayoutsForLocaleToFinish();
+  EXPECT_EQ(l10n_util::GetLanguage(initial_locale_),
+            icu::Locale::getDefault().getLanguage());
 
   // Set up an observer that will quit the message loop when login has succeeded
   // and the first wizard screen, if any, is being shown.
   base::RunLoop login_wait_run_loop;
   chromeos::MockAuthStatusConsumer login_status_consumer;
   EXPECT_CALL(login_status_consumer, OnAuthSuccess(_)).Times(1).WillOnce(
       InvokeWithoutArgs(&login_wait_run_loop, &base::RunLoop::Quit));
   chromeos::ExistingUserController* controller =
       chromeos::ExistingUserController::current_controller();
   ASSERT_TRUE(controller);
@@ skipping 37 matching lines @@
       "}"
       "var observer = new MutationObserver(SendReplyIfDownloadDone);"
       "if (!SendReplyIfDownloadDone()) {"
       "  var options = { attributes: true, attributeFilter: [ 'class' ] };"
       "  observer.observe(screenElement, options);"
       "}",
       &done));
 
   // Verify that the locale and keyboard layout have been applied.
   EXPECT_EQ(kPublicSessionLocale, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(kPublicSessionLocale),
+            icu::Locale::getDefault().getLanguage());
   EXPECT_EQ(public_session_input_method_id_,
             chromeos::input_method::InputMethodManager::Get()
                 ->GetActiveIMEState()
                 ->GetCurrentInputMethod()
                 .id());
 
   // Click the accept button.
   ASSERT_TRUE(content::ExecuteScript(contents_,
                                      "$('tos-accept-button').click();"));
 
   WaitForSessionStart();
 
   // Verify that the locale and keyboard layout are still in force.
   EXPECT_EQ(kPublicSessionLocale, g_browser_process->GetApplicationLocale());
+  EXPECT_EQ(l10n_util::GetLanguage(kPublicSessionLocale),
+            icu::Locale::getDefault().getLanguage());
   EXPECT_EQ(public_session_input_method_id_,
             chromeos::input_method::InputMethodManager::Get()
                 ->GetActiveIMEState()
                 ->GetCurrentInputMethod()
                 .id());
 }
 
 class TermsOfServiceDownloadTest : public DeviceLocalAccountTest,
                                    public testing::WithParamInterface<bool> {
 };
@@ skipping 119 matching lines @@
   ASSERT_TRUE(content::ExecuteScript(contents_,
                                      "$('tos-accept-button').click();"));
 
   WaitForSessionStart();
 }
 
 INSTANTIATE_TEST_CASE_P(TermsOfServiceDownloadTestInstance,
                         TermsOfServiceDownloadTest, testing::Bool());
 
 }  // namespace policy