ChromeOS: "Add New User" screen should enable all hardware keyboards.

chrome/browser/chromeos/login/login_ui_keyboard_browsertest.cc

+// Copyright 2014 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 "base/command_line.h"
+#include "base/prefs/pref_service.h"
+#include "base/timer/timer.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/chromeos/input_method/input_method_persistence.h"
+#include "chrome/browser/chromeos/language_preferences.h"
+#include "chrome/browser/chromeos/login/login_manager_test.h"
+#include "chrome/browser/chromeos/login/screenshot_tester.h"
+#include "chrome/browser/chromeos/login/startup_utils.h"
+#include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
+#include "chrome/browser/chromeos/login/ui/login_display_host_impl.h"
+#include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
+#include "chrome/common/pref_names.h"
+#include "chromeos/chromeos_switches.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"

[dzhioev (left Google), 2014/09/23 11:05:39]

Why do we need all this notifications_*.h included? I can't find where they are
used.

[Alexander Alekseev, 2014/09/23 13:31:40]

Done.

+#include "content/public/test/test_utils.h"
+#include "ui/compositor/compositor_switches.h"
+
+namespace chromeos {
+
+namespace {
+
+const char kTestUser1[] = "test-user1@gmail.com";
+const char kTestUser2[] = "test-user2@gmail.com";
+const char kTestUser3[] = "test-user3@gmail.com";
+
+class ENLocale {

[dzhioev (left Google), 2014/09/23 11:05:39]

nit: Why do we need a whole class for that? I think a function will be enough.
And ENLocale is not a good name. I don't understand what it means.

[Alexander Alekseev, 2014/09/23 13:31:40]

Done.

+ public:
+  ENLocale() {
+    en_locale_input_methods_.push_back("xkb:us::eng");
+    en_locale_input_methods_.push_back("xkb:us:intl:eng");
+    en_locale_input_methods_.push_back("xkb:us:altgr-intl:eng");
+    en_locale_input_methods_.push_back("xkb:us:dvorak:eng");
+    en_locale_input_methods_.push_back("xkb:us:colemak:eng");
+    chromeos::input_method::InputMethodManager::Get()->MigrateInputMethods(

[dzhioev (left Google), 2014/09/23 11:05:38]

Do we need to do a migration here? We are doing it in tests cases anyway.

[Alexander Alekseev, 2014/09/23 13:31:40]

I've removed Migration from tests.

+        &en_locale_input_methods_);
+  }
+
+  void AppendInputMethods(std::vector<std::string>* out) const {
+    out->insert(out->end(),
+                en_locale_input_methods_.begin(),
+                en_locale_input_methods_.end());
+  }
+
+ private:
+  std::vector<std::string> en_locale_input_methods_;
+};
+
+class FocusPODWaiter {
+ public:
+  FocusPODWaiter() : focused_(false), runner_(new content::MessageLoopRunner) {
+    GetOobeUI()
+        ->signin_screen_handler_for_test()
+        ->SetFocusPODCallbackForTesting(
+            base::Bind(&FocusPODWaiter::OnFocusPOD, base::Unretained(this)));
+  }
+  ~FocusPODWaiter() {
+    GetOobeUI()
+        ->signin_screen_handler_for_test()
+        ->SetFocusPODCallbackForTesting(base::Closure());
+  }
+
+  void OnFocusPOD() {
+    focused_ = true;
+    if (runner_.get())
+      base::MessageLoopForUI::current()->PostTask(
+          FROM_HERE,
+          base::Bind(&FocusPODWaiter::ExitMessageLoop, base::Unretained(this)));
+  }
+
+  void ExitMessageLoop() { runner_->Quit(); }
+
+  void Wait() {
+    if (focused_)
+      return;
+    runner_->Run();
+    GetOobeUI()
+        ->signin_screen_handler_for_test()
+        ->SetFocusPODCallbackForTesting(base::Closure());
+    runner_ = NULL;
+  }
+
+ private:
+  OobeUI* GetOobeUI() {
+    OobeUI* oobe_ui =
+        static_cast<chromeos::LoginDisplayHostImpl*>(
+            chromeos::LoginDisplayHostImpl::default_host())->GetOobeUI();
+    CHECK(oobe_ui);
+    return oobe_ui;
+  }
+
+  bool focused_;
+
+  scoped_refptr<content::MessageLoopRunner> runner_;
+};
+
+}  // anonymous namespace
+
+class LoginUIKeyboardTest : public chromeos::LoginManagerTest {
+ public:
+  LoginUIKeyboardTest() : LoginManagerTest(false) {}
+  virtual ~LoginUIKeyboardTest() {}
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    user_input_methods.push_back("xkb:fr::fra");
+    user_input_methods.push_back("xkb:de::ger");
+
+    chromeos::input_method::InputMethodManager::Get()->MigrateInputMethods(
+        &user_input_methods);
+
+    en_locale.reset(new ENLocale);

[dzhioev (left Google), 2014/09/23 11:05:38]

Why not in constructor?

[Alexander Alekseev, 2014/09/23 13:31:40]

Because of MigrateInputMethods.

+
+    LoginManagerTest::SetUpOnMainThread();
+  }
+
+  // Should be called from PRE_ test so that local_state is saved to disk, and
+  // reloaded in the main test.
+  void InitUserLRUInputMethod() {
+    PrefService* local_state = g_browser_process->local_state();
+
+    input_method::SetUserLRUInputMethodPreferenceForTesting(
+        kTestUser1, user_input_methods[0], local_state);
+    input_method::SetUserLRUInputMethodPreferenceForTesting(
+        kTestUser2, user_input_methods[1], local_state);
+  }
+
+ protected:
+  std::vector<std::string> user_input_methods;

[dzhioev (left Google), 2014/09/23 11:05:39]

Is this a mapping from user index to user's input method? 

[Alexander Alekseev, 2014/09/23 13:31:40]

Yes.

+  scoped_ptr<ENLocale> en_locale;
+};
+
+IN_PROC_BROWSER_TEST_F(LoginUIKeyboardTest, PRE_CheckPODScreenDefault) {
+  RegisterUser(kTestUser1);
+  RegisterUser(kTestUser2);
+
+  StartupUtils::MarkOobeCompleted();
+}
+
+// Check default IME initialization, when there is no IME configuration in
+// local_state.
+IN_PROC_BROWSER_TEST_F(LoginUIKeyboardTest, CheckPODScreenDefault) {
+  js_checker().ExpectEQ("$('pod-row').pods.length", 2);
+
+  std::vector<std::string> expected_input_methods;
+  en_locale->AppendInputMethods(&expected_input_methods);
+
+  const size_t expected_input_methods_number = expected_input_methods.size();
+  chromeos::input_method::InputMethodManager::Get()->MigrateInputMethods(
+      &expected_input_methods);
+  EXPECT_EQ(expected_input_methods_number, expected_input_methods.size());
+
+  EXPECT_EQ(expected_input_methods,
+            input_method::InputMethodManager::Get()
+                ->GetActiveIMEState()
+                ->GetActiveInputMethodIds());
+}
+
+IN_PROC_BROWSER_TEST_F(LoginUIKeyboardTest, PRE_CheckPODScreenWithUsers) {
+  RegisterUser(kTestUser1);
+  RegisterUser(kTestUser2);
+
+  InitUserLRUInputMethod();
+
+  StartupUtils::MarkOobeCompleted();
+}
+
+IN_PROC_BROWSER_TEST_F(LoginUIKeyboardTest, CheckPODScreenWithUsers) {
+  js_checker().ExpectEQ("$('pod-row').pods.length", 2);
+
+  EXPECT_EQ(user_input_methods[0],
+            input_method::InputMethodManager::Get()
+                ->GetActiveIMEState()
+                ->GetCurrentInputMethod()
+                .id());
+
+  std::vector<std::string> expected_input_methods;
+  en_locale->AppendInputMethods(&expected_input_methods);
+  // Active IM for the first user (active user POD).
+  expected_input_methods.push_back(user_input_methods[0]);
+
+  const size_t expected_input_methods_number = expected_input_methods.size();
+  chromeos::input_method::InputMethodManager::Get()->MigrateInputMethods(
+      &expected_input_methods);
+  EXPECT_EQ(expected_input_methods_number, expected_input_methods.size());
+
+  EXPECT_EQ(expected_input_methods,
+            input_method::InputMethodManager::Get()
+                ->GetActiveIMEState()
+                ->GetActiveInputMethodIds());

[dzhioev (left Google), 2014/09/23 11:05:38]

These lines (190-198) are repeated 4 times in this file. Could you please create
a method in the base class doing these things.

[Alexander Alekseev, 2014/09/23 13:31:40]

I've removed most of them.

+
+  FocusPODWaiter waiter;
+  js_checker().Evaluate("$('pod-row').focusPod($('pod-row').pods[1])");
+  waiter.Wait();
+
+  EXPECT_EQ(user_input_methods[1],
+            input_method::InputMethodManager::Get()
+                ->GetActiveIMEState()
+                ->GetCurrentInputMethod()
+                .id());
+}
+
+class LoginUIKeyboardTestWithUsersAndOwner : public chromeos::LoginManagerTest {

[dzhioev (left Google), 2014/09/23 11:05:39]

LoginUIKeyboardTestWithUsersAndOwner has a common parts with LoginUIKeyboardTest
(user_input_methods, en_locale). Could you please create a common base for them.

[Alexander Alekseev, 2014/09/23 13:31:40]

user_input_methods are different. That is why classes are different.

+ public:
+  LoginUIKeyboardTestWithUsersAndOwner() : LoginManagerTest(false) {}
+  virtual ~LoginUIKeyboardTestWithUsersAndOwner() {}
+
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    LoginManagerTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kStubCrosSettings);
+
+    LoginManagerTest::SetUpCommandLine(command_line);
+  }
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    user_input_methods.push_back("xkb:fr::fra");
+    user_input_methods.push_back("xkb:de::ger");
+    user_input_methods.push_back("xkb:pl::pol");
+
+    chromeos::input_method::InputMethodManager::Get()->MigrateInputMethods(
+        &user_input_methods);
+
+    CrosSettings::Get()->SetString(kDeviceOwner, kTestUser3);
+
+    en_locale.reset(new ENLocale);
+
+    LoginManagerTest::SetUpOnMainThread();
+  }
+
+  // Should be called from PRE_ test so that local_state is saved to disk, and
+  // reloaded in the main test.
+  void InitUserLRUInputMethod() {
+    PrefService* local_state = g_browser_process->local_state();
+
+    input_method::SetUserLRUInputMethodPreferenceForTesting(
+        kTestUser1, user_input_methods[0], local_state);
+    input_method::SetUserLRUInputMethodPreferenceForTesting(
+        kTestUser2, user_input_methods[1], local_state);
+    input_method::SetUserLRUInputMethodPreferenceForTesting(
+        kTestUser3, user_input_methods[2], local_state);
+
+    local_state->SetString(language_prefs::kPreferredKeyboardLayout,
+                           user_input_methods[2]);
+  }
+
+  void CheckGaiaKeyboard();
+
+ protected:
+  std::vector<std::string> user_input_methods;
+  scoped_ptr<ENLocale> en_locale;
+};
+
+void LoginUIKeyboardTestWithUsersAndOwner::CheckGaiaKeyboard() {
+  std::vector<std::string> expected_input_methods;
+  // kPreferredKeyboardLayout is now set to last focused POD.
+  expected_input_methods.push_back(user_input_methods[0]);
+  // Locale default input methods (the first one also is hardware IM).
+  en_locale->AppendInputMethods(&expected_input_methods);
+
+  const size_t expected_input_methods_number = expected_input_methods.size();
+  chromeos::input_method::InputMethodManager::Get()->MigrateInputMethods(
+      &expected_input_methods);
+  EXPECT_EQ(expected_input_methods_number, expected_input_methods.size());
+  scoped_refptr<input_method::InputMethodManager::State> ime_state =
+      input_method::InputMethodManager::Get()->GetActiveIMEState();

[dzhioev (left Google), 2014/09/23 11:05:39]

|ime_state| not used.

[Alexander Alekseev, 2014/09/23 13:31:40]

Done.

+
+  EXPECT_EQ(expected_input_methods,
+            input_method::InputMethodManager::Get()
+                ->GetActiveIMEState()
+                ->GetActiveInputMethodIds());
+}
+
+IN_PROC_BROWSER_TEST_F(LoginUIKeyboardTestWithUsersAndOwner,
+                       PRE_CheckPODScreenKeyboard) {
+  RegisterUser(kTestUser1);
+  RegisterUser(kTestUser2);
+  RegisterUser(kTestUser3);
+
+  InitUserLRUInputMethod();
+
+  StartupUtils::MarkOobeCompleted();
+}
+
+IN_PROC_BROWSER_TEST_F(LoginUIKeyboardTestWithUsersAndOwner,
+                       CheckPODScreenKeyboard) {
+  js_checker().ExpectEQ("$('pod-row').pods.length", 3);
+
+  std::vector<std::string> expected_input_methods;
+  // Owner input method.
+  expected_input_methods.push_back(user_input_methods[2]);
+  // Locale default input methods (the first one also is hardware IM).
+  en_locale->AppendInputMethods(&expected_input_methods);
+  // Active IM for the first user (active user POD).
+  expected_input_methods.push_back(user_input_methods[0]);
+
+  const size_t expected_input_methods_number = expected_input_methods.size();
+  chromeos::input_method::InputMethodManager::Get()->MigrateInputMethods(
+      &expected_input_methods);
+  EXPECT_EQ(expected_input_methods_number, expected_input_methods.size());
+
+  EXPECT_EQ(expected_input_methods,
+            input_method::InputMethodManager::Get()
+                ->GetActiveIMEState()
+                ->GetActiveInputMethodIds());
+
+  // Switch to Gaia.
+  js_checker().Evaluate("$('add-user-button').click()");
+  OobeScreenWaiter(OobeDisplay::SCREEN_GAIA_SIGNIN).Wait();
+  CheckGaiaKeyboard();
+
+  // Switch back.
+  js_checker().Evaluate("$('cancel-add-user-button').click()");
+  OobeScreenWaiter(OobeDisplay::SCREEN_ACCOUNT_PICKER).Wait();
+
+  EXPECT_EQ(expected_input_methods,
+            input_method::InputMethodManager::Get()
+                ->GetActiveIMEState()
+                ->GetActiveInputMethodIds());
+}
+}  // namespace chromeos