cros: More cleanup on ScreenLockerTester

TBR=dtseng@chromium.org

Bug: 899777
Change-Id: Id56ad8bb68c95b4412fbc9ebacef7921216fe9ce
Reviewed-on: https://chromium-review.googlesource.com/c/1317924
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608415}

chrome/browser/chromeos/extensions/users_private/users_private_apitest.cc

@@ -240,8 +240,8 @@ IN_PROC_BROWSER_TEST_F(UsersPrivateApiLoginStatusTest, User) {
 // Screenlock - logged in, screen locked.
 IN_PROC_BROWSER_TEST_F(UsersPrivateApiLockStatusTest, ScreenLock) {
   chromeos::ScreenLocker::Show();
-  std::unique_ptr<chromeos::test::ScreenLockerTester> tester(
-      chromeos::ScreenLocker::GetTester());
+  std::unique_ptr<chromeos::ScreenLockerTester> tester =
+      chromeos::ScreenLockerTester::Create();
   content::WindowedNotificationObserver lock_state_observer(
       chrome::NOTIFICATION_SCREEN_LOCK_STATE_CHANGED,
       content::NotificationService::AllSources());

chrome/browser/chromeos/login/lock/screen_locker.cc

@@ -582,6 +582,14 @@ void ScreenLocker::SaveSyncPasswordHash(const UserContext& user_context) {
   if (profile)
     login::SaveSyncPasswordDataToProfile(user_context, profile);
 }
+
+void ScreenLocker::SetAuthenticatorsForTesting(
+    scoped_refptr<Authenticator> authenticator,
+    scoped_refptr<ExtendedAuthenticator> extended_authenticator) {
+  authenticator_ = std::move(authenticator);
+  extended_authenticator_ = std::move(extended_authenticator);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // ScreenLocker, private:
 
@@ -613,10 +621,6 @@ ScreenLocker::~ScreenLocker() {
   }
 }
 
-void ScreenLocker::SetAuthenticator(Authenticator* authenticator) {
-  authenticator_ = authenticator;
-}
-
 void ScreenLocker::ScreenLockReady() {
   locked_ = true;
   base::TimeDelta delta = base::Time::Now() - start_time_;

chrome/browser/chromeos/login/lock/screen_locker.h

@@ -38,12 +38,6 @@ class ScreenlockIconProvider;
 class WebUIScreenLocker;
 class ViewsScreenLocker;
 
-namespace test {
-class ScreenLockerTester;
-class ScreenLockerViewsTester;
-class WebUIScreenLockerTester;
-}  // namespace test
-
 // ScreenLocker creates a WebUIScreenLocker which will display the lock UI.
 // As well, it takes care of authenticating the user and managing a global
 // instance of itself which will be deleted when the system is unlocked.
@@ -166,18 +160,17 @@ class ScreenLocker : public AuthStatusConsumer,
   // Hide the screen locker.
   static void Hide();
 
-  // Returns the tester
-  static test::ScreenLockerTester* GetTester();
-
   // Saves sync password hash and salt to user profile prefs based on
   // |user_context|.
   void SaveSyncPasswordHash(const UserContext& user_context);
 
+  // Change the authenticators; should only be used by tests.
+  void SetAuthenticatorsForTesting(
+      scoped_refptr<Authenticator> authenticator,
+      scoped_refptr<ExtendedAuthenticator> extended_authenticator);
+
  private:
   friend class base::DeleteHelper<ScreenLocker>;
-  friend class test::ScreenLockerTester;
-  friend class test::ScreenLockerViewsTester;
-  friend class test::WebUIScreenLockerTester;
   friend class WebUIScreenLocker;
   friend class ViewsScreenLocker;
 
@@ -201,9 +194,6 @@ class ScreenLocker : public AuthStatusConsumer,
 
   void OnFingerprintAuthFailure(const user_manager::User& user);
 
-  // Sets the authenticator.
-  void SetAuthenticator(Authenticator* authenticator);
-
   // Called when the screen lock is ready.
   void ScreenLockReady();
 

chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc

@@ -43,6 +43,7 @@ using testing::_;
 using testing::AnyNumber;
 using testing::Return;
 
+namespace chromeos {
 namespace {
 
 // An object that wait for lock state and fullscreen state.
@@ -68,8 +69,7 @@ class Waiter : public content::NotificationObserver {
 
   // Wait until the two conditions are met.
   void Wait(bool locker_state, bool fullscreen) {
-    std::unique_ptr<chromeos::test::ScreenLockerTester> tester(
-        chromeos::ScreenLocker::GetTester());
+    std::unique_ptr<ScreenLockerTester> tester = ScreenLockerTester::Create();
     while (tester->IsLocked() != locker_state ||
            browser_->window()->IsFullscreen() != fullscreen) {
       base::RunLoop run_loop;
@@ -91,8 +91,6 @@ class Waiter : public content::NotificationObserver {
 
 }  // namespace
 
-namespace chromeos {
-
 class ScreenLockerTest : public InProcessBrowserTest {
  public:
   ScreenLockerTest() : fake_session_manager_client_(NULL) {}
@@ -100,7 +98,7 @@ class ScreenLockerTest : public InProcessBrowserTest {
  protected:
   FakeSessionManagerClient* fake_session_manager_client_;
 
-  void LockScreen(test::ScreenLockerTester* tester) {
+  void LockScreen(ScreenLockerTester* tester) {
     ScreenLocker::Show();
     content::WindowedNotificationObserver lock_state_observer(
         chrome::NOTIFICATION_SCREEN_LOCK_STATE_CHANGED,
@@ -156,7 +154,7 @@ IN_PROC_BROWSER_TEST_F(WebUiScreenLockerTest, TestBasic) {
   if (features::IsUsingWindowService())
     return;
   ScreenLocker::Show();
-  std::unique_ptr<test::ScreenLockerTester> tester(ScreenLocker::GetTester());
+  std::unique_ptr<ScreenLockerTester> tester = ScreenLockerTester::Create();
   content::WindowedNotificationObserver lock_state_observer(
       chrome::NOTIFICATION_SCREEN_LOCK_STATE_CHANGED,
       content::NotificationService::AllSources());
@@ -211,7 +209,7 @@ IN_PROC_BROWSER_TEST_F(WebUiScreenLockerTest, MAYBE_TestFullscreenExit) {
   // does not have all the pixels (e.g. the shelf is auto hidden instead of
   // hidden), locking the screen should not exit fullscreen. The shelf is
   // auto hidden when in immersive fullscreen.
-  std::unique_ptr<test::ScreenLockerTester> tester(ScreenLocker::GetTester());
+  std::unique_ptr<ScreenLockerTester> tester = ScreenLockerTester::Create();
   BrowserWindow* browser_window = browser()->window();
   ash::wm::WindowState* window_state =
       ash::wm::GetWindowState(browser_window->GetNativeWindow());
@@ -300,7 +298,7 @@ void UnlockKeyPress(views::Widget* widget) {
 }
 
 IN_PROC_BROWSER_TEST_F(ScreenLockerTest, TestShowTwice) {
-  std::unique_ptr<test::ScreenLockerTester> tester(ScreenLocker::GetTester());
+  std::unique_ptr<ScreenLockerTester> tester = ScreenLockerTester::Create();
   LockScreen(tester.get());
 
   // Calling Show again simply send LockCompleted signal.

chrome/browser/chromeos/login/lock/screen_locker_tester.cc

@@ -8,10 +8,8 @@
 
 #include "base/macros.h"
 #include "base/run_loop.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/chromeos/login/lock/screen_locker.h"
 #include "chrome/browser/chromeos/login/lock/webui_screen_locker.h"
 #include "chromeos/login/auth/auth_status_consumer.h"
@@ -22,177 +20,143 @@
 #include "content/public/browser/web_ui.h"
 #include "content/public/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "ui/views/controls/button/button.h"
-#include "ui/views/controls/label.h"
-#include "ui/views/controls/textfield/textfield.h"
-#include "ui/views/widget/root_view.h"
-
-using content::WebContents;
 
+namespace chromeos {
 namespace {
 
 // This class is used to observe state of the global ScreenLocker instance,
 // which can go away as a result of a successful authentication. As such,
 // it needs to directly reference the global ScreenLocker.
-class LoginAttemptObserver : public chromeos::AuthStatusConsumer {
+class LoginAttemptObserver : public AuthStatusConsumer {
  public:
-  LoginAttemptObserver();
-  ~LoginAttemptObserver() override;
-
-  void WaitForAttempt();
+  LoginAttemptObserver() : AuthStatusConsumer() {
+    ScreenLocker::default_screen_locker()->SetLoginStatusConsumer(this);
+  }
+  ~LoginAttemptObserver() override {
+    if (ScreenLocker::default_screen_locker())
+      ScreenLocker::default_screen_locker()->SetLoginStatusConsumer(nullptr);
+  }
 
-  // Overridden from AuthStatusConsumer:
-  void OnAuthFailure(const chromeos::AuthFailure& error) override {
-    LoginAttempted();
+  void WaitForAttempt() {
+    if (!login_attempted_) {
+      run_loop_ = std::make_unique<base::RunLoop>();
+      run_loop_->Run();
+      run_loop_.release();
+    }
+    ASSERT_TRUE(login_attempted_);
   }
 
-  void OnAuthSuccess(const chromeos::UserContext& credentials) override {
+  // AuthStatusConsumer:
+  void OnAuthFailure(const AuthFailure& error) override { LoginAttempted(); }
+  void OnAuthSuccess(const UserContext& credentials) override {
     LoginAttempted();
   }
 
  private:
-  void LoginAttempted();
+  void LoginAttempted() {
+    login_attempted_ = true;
+    if (run_loop_)
+      run_loop_->QuitWhenIdle();
+  }
 
-  bool login_attempted_;
-  bool waiting_;
+  bool login_attempted_ = false;
+  std::unique_ptr<base::RunLoop> run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginAttemptObserver);
 };
 
-LoginAttemptObserver::LoginAttemptObserver()
-    : chromeos::AuthStatusConsumer(), login_attempted_(false), waiting_(false) {
-  chromeos::ScreenLocker::default_screen_locker()->SetLoginStatusConsumer(this);
-}
-
-LoginAttemptObserver::~LoginAttemptObserver() {
-  chromeos::ScreenLocker* global_locker =
-      chromeos::ScreenLocker::default_screen_locker();
-  if (global_locker)
-    global_locker->SetLoginStatusConsumer(NULL);
-}
-
-void LoginAttemptObserver::WaitForAttempt() {
-  if (!login_attempted_) {
-    waiting_ = true;
-    content::RunMessageLoop();
-    waiting_ = false;
-  }
-  ASSERT_TRUE(login_attempted_);
-}
-
-void LoginAttemptObserver::LoginAttempted() {
-  login_attempted_ = true;
-  if (waiting_)
-    base::RunLoop::QuitCurrentWhenIdleDeprecated();
-}
-
-}  // namespace
-
-namespace chromeos {
-
-namespace test {
-
 class WebUIScreenLockerTester : public ScreenLockerTester {
  public:
-  // ScreenLockerTester overrides:
-  void SetPassword(const std::string& password) override;
-  std::string GetPassword() override;
-  void EnterPassword(const std::string& password) override;
+  WebUIScreenLockerTester() = default;
+  ~WebUIScreenLockerTester() override = default;
+
+  // ScreenLockerTester:
+  void EnterPassword(const std::string& password) override {
+    bool result;
+
+    SetPassword(password);
+
+    // Verify password is set.
+    ASSERT_EQ(password, GetPassword());
+
+    // Verify that "reauth" warning is hidden.
+    std::unique_ptr<base::Value> v = content::ExecuteScriptAndGetValue(
+        GetMainFrame(),
+        "window.getComputedStyle("
+        "    $('pod-row').pods[0].querySelector('.reauth-hint-container'))"
+        "        .display == 'none'");
+    ASSERT_TRUE(v->GetAsBoolean(&result));
+    ASSERT_TRUE(result);
+
+    // Attempt to sign in.
+    LoginAttemptObserver login;
+    v = content::ExecuteScriptAndGetValue(GetMainFrame(),
+                                          "$('pod-row').pods[0].activate();");
+    ASSERT_TRUE(v->GetAsBoolean(&result));
+    ASSERT_TRUE(result);
+
+    // Wait for login attempt.
+    login.WaitForAttempt();
+  }
 
  private:
-  friend class chromeos::ScreenLocker;
+  void SetPassword(const std::string& password) {
+    GetMainFrame()->ExecuteJavaScriptForTests(base::ASCIIToUTF16(
+        base::StringPrintf("$('pod-row').pods[0].passwordElement.value = '%s';",
+                           password.c_str())));
+  }
 
-  WebUIScreenLockerTester() {}
+  std::string GetPassword() {
+    std::string result;
+    std::unique_ptr<base::Value> v = content::ExecuteScriptAndGetValue(
+        GetMainFrame(), "$('pod-row').pods[0].passwordElement.value;");
+    CHECK(v->GetAsString(&result));
+    return result;
+  }
 
-  content::RenderFrameHost* GetMainFrame() const;
+  content::RenderFrameHost* GetMainFrame() {
+    return webui()->GetWebContents()->GetMainFrame();
+  }
 
   // Returns the ScreenLockerWebUI object.
-  WebUIScreenLocker* webui_screen_locker() const;
+  WebUIScreenLocker* webui_screen_locker() {
+    DCHECK(ScreenLocker::default_screen_locker());
+    return ScreenLocker::default_screen_locker()->web_ui_for_testing();
+  }
 
   // Returns the WebUI object from the screen locker.
-  content::WebUI* webui() const;
+  content::WebUI* webui() {
+    DCHECK(webui_screen_locker()->webui_ready_for_testing());
+    content::WebUI* webui = webui_screen_locker()->GetWebUI();
+    DCHECK(webui);
+    return webui;
+  }
 
   DISALLOW_COPY_AND_ASSIGN(WebUIScreenLockerTester);
 };
 
-void WebUIScreenLockerTester::SetPassword(const std::string& password) {
-  GetMainFrame()->ExecuteJavaScriptForTests(base::ASCIIToUTF16(
-      base::StringPrintf("$('pod-row').pods[0].passwordElement.value = '%s';",
-                         password.c_str())));
-}
-
-std::string WebUIScreenLockerTester::GetPassword() {
-  std::string result;
-  std::unique_ptr<base::Value> v = content::ExecuteScriptAndGetValue(
-      GetMainFrame(), "$('pod-row').pods[0].passwordElement.value;");
-  CHECK(v->GetAsString(&result));
-  return result;
-}
-
-void WebUIScreenLockerTester::EnterPassword(const std::string& password) {
-  bool result;
-  SetPassword(password);
-
-  // Verify password is set.
-  ASSERT_EQ(password, GetPassword());
-
-  // Verify that "reauth" warning is hidden.
-  std::unique_ptr<base::Value> v = content::ExecuteScriptAndGetValue(
-      GetMainFrame(),
-      "window.getComputedStyle("
-      "    $('pod-row').pods[0].querySelector('.reauth-hint-container'))"
-      "        .display == 'none'");
-  ASSERT_TRUE(v->GetAsBoolean(&result));
-  ASSERT_TRUE(result);
-
-  // Attempt to sign in.
-  LoginAttemptObserver login;
-  v = content::ExecuteScriptAndGetValue(GetMainFrame(),
-                                        "$('pod-row').pods[0].activate();");
-  ASSERT_TRUE(v->GetAsBoolean(&result));
-  ASSERT_TRUE(result);
-
-  // Wait for login attempt.
-  login.WaitForAttempt();
-}
-
-content::RenderFrameHost* WebUIScreenLockerTester::GetMainFrame() const {
-  return webui()->GetWebContents()->GetMainFrame();
-}
-
-WebUIScreenLocker* WebUIScreenLockerTester::webui_screen_locker() const {
-  DCHECK(ScreenLocker::screen_locker_);
-  return ScreenLocker::screen_locker_->web_ui_for_testing();
-}
+}  // namespace
 
-content::WebUI* WebUIScreenLockerTester::webui() const {
-  DCHECK(webui_screen_locker()->webui_ready_);
-  content::WebUI* webui = webui_screen_locker()->GetWebUI();
-  DCHECK(webui);
-  return webui;
+// static
+std::unique_ptr<ScreenLockerTester> ScreenLockerTester::Create() {
+  return std::make_unique<WebUIScreenLockerTester>();
 }
 
 ScreenLockerTester::ScreenLockerTester() {}
 
 ScreenLockerTester::~ScreenLockerTester() {}
 
-bool ScreenLockerTester::IsLocked() {
-  return ScreenLocker::screen_locker_ && ScreenLocker::screen_locker_->locked_;
-}
-
 void ScreenLockerTester::InjectStubUserContext(
     const UserContext& user_context) {
-  DCHECK(ScreenLocker::screen_locker_);
-  ScreenLocker::screen_locker_->SetAuthenticator(
-      new StubAuthenticator(ScreenLocker::screen_locker_, user_context));
-  ScreenLocker::screen_locker_->extended_authenticator_ =
-      new FakeExtendedAuthenticator(ScreenLocker::screen_locker_, user_context);
+  auto* locker = ScreenLocker::default_screen_locker();
+  locker->SetAuthenticatorsForTesting(
+      base::MakeRefCounted<StubAuthenticator>(locker, user_context),
+      base::MakeRefCounted<FakeExtendedAuthenticator>(locker, user_context));
 }
 
-}  // namespace test
-
-test::ScreenLockerTester* ScreenLocker::GetTester() {
-  return new test::WebUIScreenLockerTester();
+bool ScreenLockerTester::IsLocked() {
+  return ScreenLocker::default_screen_locker() &&
+         ScreenLocker::default_screen_locker()->locked();
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/login/lock/screen_locker_tester.h

@@ -5,18 +5,20 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_LOCK_SCREEN_LOCKER_TESTER_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_LOCK_SCREEN_LOCKER_TESTER_H_
 
+#include <memory>
 #include <string>
 
 namespace chromeos {
 
 class UserContext;
 
-namespace test {
-
 // ScreenLockerTester provides access to the private state/function
 // of ScreenLocker class. Used to implement unit tests.
 class ScreenLockerTester {
  public:
+  // Create a new tester.
+  static std::unique_ptr<ScreenLockerTester> Create();
+
   ScreenLockerTester();
   virtual ~ScreenLockerTester();
 
@@ -26,18 +28,10 @@ class ScreenLockerTester {
   // Injects StubAuthenticator that uses the credentials in |user_context|.
   virtual void InjectStubUserContext(const UserContext& user_context);
 
-  // Sets the password text.
-  virtual void SetPassword(const std::string& password) = 0;
-
-  // Gets the password text.
-  virtual std::string GetPassword() = 0;
-
-  // Emulates entring a password.
+  // Enters and submits the given password.
   virtual void EnterPassword(const std::string& password) = 0;
 };
 
-}  // namespace test
-
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_LOGIN_LOCK_SCREEN_LOCKER_TESTER_H_

chrome/browser/chromeos/login/lock/webui_screen_locker.h

@@ -38,10 +38,6 @@ namespace login {
 class NetworkStateHelper;
 }
 
-namespace test {
-class WebUIScreenLockerTester;
-}
-
 // Displays a WebUI lock screen based on the Oobe account picker screen.
 class WebUIScreenLocker : public WebUILoginView,
                           public ScreenLocker::Delegate,
@@ -62,8 +58,9 @@ class WebUIScreenLocker : public WebUILoginView,
   // ScreenLockReady is called when all initialization has finished.
   void LockScreen();
 
+  bool webui_ready_for_testing() const { return webui_ready_; }
+
  private:
-  friend class test::WebUIScreenLockerTester;
 
   // Returns true if the lock screen should be preloaded.
   static bool ShouldPreloadLockScreen();

chrome/browser/chromeos/shutdown_policy_browsertest.cc

@@ -238,7 +238,8 @@ class ShutdownPolicyLockerTest : public ShutdownPolicyBaseTest {
 
     // Bring up the locker screen.
     ScreenLocker::Show();
-    std::unique_ptr<test::ScreenLockerTester> tester(ScreenLocker::GetTester());
+    std::unique_ptr<chromeos::ScreenLockerTester> tester =
+        chromeos::ScreenLockerTester::Create();
     content::WindowedNotificationObserver lock_state_observer(
         chrome::NOTIFICATION_SCREEN_LOCK_STATE_CHANGED,
         content::NotificationService::AllSources());

chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc

@@ -31,8 +31,8 @@
 
 using chromeos::ProfileHelper;
 using chromeos::ScreenLocker;
+using chromeos::ScreenLockerTester;
 using user_manager::UserManager;
-using chromeos::test::ScreenLockerTester;
 using content::BrowserThread;
 
 namespace extensions {
@@ -338,7 +338,7 @@ class BrailleDisplayPrivateAPIUserTest : public BrailleDisplayPrivateApiTest {
 #endif
 IN_PROC_BROWSER_TEST_F(BrailleDisplayPrivateAPIUserTest,
                        MAYBE_KeyEventOnLockScreen) {
-  std::unique_ptr<ScreenLockerTester> tester(ScreenLocker::GetTester());
+  std::unique_ptr<ScreenLockerTester> tester = ScreenLockerTester::Create();
   // Log in.
   session_manager::SessionManager::Get()->CreateSession(
       AccountId::FromUserEmailGaiaId(kTestUserName, kTestUserGaiaId),