Initialize mock TSFBridge during testing.

Added a new method to initialize a mocked version of TSFBridge when
|InitializeInputMethodForTesting| is called from test code. This
change also adds a call in the views unittest base to initialize the
|MockTSFBridge| by calling |InitializeInputMethodForTesting| and
enables some disabled tests.

Bug: 968222

Change-Id: Ib25dcefd53a2b6e11ea6dfa713efe37052b24be6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364034Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Yohei Yukawa <yukawa@chromium.org>
Commit-Queue: Anupam Snigdha <snianu@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#799826}

chrome/browser/ui/views/passwords/move_to_account_store_bubble_view_unittest.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/ui/views/passwords/move_to_account_store_bubble_view.h"
 
 #include "base/test/scoped_feature_list.h"
-#include "build/build_config.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/passwords/password_bubble_view_test_base.h"
@@ -60,13 +59,7 @@ void MoveToAccountStoreBubbleViewTest::TearDown() {
   PasswordBubbleViewTestBase::TearDown();
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_HasTwoButtons DISABLED_HasTwoButtons
-#else
-#define MAYBE_HasTwoButtons HasTwoButtons
-#endif
-TEST_F(MoveToAccountStoreBubbleViewTest, MAYBE_HasTwoButtons) {
+TEST_F(MoveToAccountStoreBubbleViewTest, HasTwoButtons) {
   CreateViewAndShow();
   ASSERT_TRUE(view_->GetOkButton());
   ASSERT_TRUE(view_->GetCancelButton());
@@ -78,13 +71,7 @@ TEST_F(MoveToAccountStoreBubbleViewTest, MAYBE_HasTwoButtons) {
       view_->GetDialogButtonLabel(ui::DIALOG_BUTTON_CANCEL));
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_HasDescription DISABLED_HasDescription
-#else
-#define MAYBE_HasDescription HasDescription
-#endif
-TEST_F(MoveToAccountStoreBubbleViewTest, MAYBE_HasDescription) {
+TEST_F(MoveToAccountStoreBubbleViewTest, HasDescription) {
   CreateViewAndShow();
 
   ASSERT_EQ(view_->children().size(), 2u);

chrome/browser/ui/views/passwords/password_save_unsynced_credentials_locally_view_unittest.cc

@@ -7,7 +7,6 @@
 #include <vector>
 
 #include "base/strings/utf_string_conversions.h"
-#include "build/build_config.h"
 #include "chrome/browser/ui/passwords/passwords_model_delegate_mock.h"
 #include "chrome/browser/ui/views/passwords/password_bubble_view_test_base.h"
 #include "components/autofill/core/common/password_form.h"
@@ -56,14 +55,7 @@ void PasswordSaveUnsyncedCredentialsLocallyViewTest::TearDown() {
   PasswordBubbleViewTestBase::TearDown();
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_HasTitleAndTwoButtons DISABLED_HasTitleAndTwoButtons
-#else
-#define MAYBE_HasTitleAndTwoButtons HasTitleAndTwoButtons
-#endif
-TEST_F(PasswordSaveUnsyncedCredentialsLocallyViewTest,
-       MAYBE_HasTitleAndTwoButtons) {
+TEST_F(PasswordSaveUnsyncedCredentialsLocallyViewTest, HasTitleAndTwoButtons) {
   CreateViewAndShow();
   EXPECT_TRUE(view_->ShouldShowWindowTitle());
   EXPECT_TRUE(view_->GetOkButton());

chrome/browser/ui/views/passwords/password_save_update_with_account_store_view_unittest.cc

@@ -9,7 +9,6 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/test/scoped_feature_list.h"
-#include "build/build_config.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
@@ -112,42 +111,21 @@ void PasswordSaveUpdateWithAccountStoreViewTest::SimulateSignIn() {
       signin::MakePrimaryAccountAvailable(identity_manager, "test@email.com");
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_HasTitleAndTwoButtons DISABLED_HasTitleAndTwoButtons
-#else
-#define MAYBE_HasTitleAndTwoButtons HasTitleAndTwoButtons
-#endif
-TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
-       MAYBE_HasTitleAndTwoButtons) {
+TEST_F(PasswordSaveUpdateWithAccountStoreViewTest, HasTitleAndTwoButtons) {
   CreateViewAndShow();
   EXPECT_TRUE(view()->ShouldShowWindowTitle());
   EXPECT_TRUE(view()->GetOkButton());
   EXPECT_TRUE(view()->GetCancelButton());
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_ShouldNotShowAccountPicker DISABLED_ShouldNotShowAccountPicker
-#else
-#define MAYBE_ShouldNotShowAccountPicker ShouldNotShowAccountPicker
-#endif
-TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
-       MAYBE_ShouldNotShowAccountPicker) {
+TEST_F(PasswordSaveUpdateWithAccountStoreViewTest, ShouldNotShowAccountPicker) {
   ON_CALL(*feature_manager_mock(), ShouldShowAccountStorageBubbleUi)
       .WillByDefault(Return(false));
   CreateViewAndShow();
   EXPECT_FALSE(account_picker());
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_ShouldShowAccountPicker DISABLED_ShouldShowAccountPicker
-#else
-#define MAYBE_ShouldShowAccountPicker ShouldShowAccountPicker
-#endif
-TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
-       MAYBE_ShouldShowAccountPicker) {
+TEST_F(PasswordSaveUpdateWithAccountStoreViewTest, ShouldShowAccountPicker) {
   ON_CALL(*feature_manager_mock(), ShouldShowAccountStorageBubbleUi)
       .WillByDefault(Return(true));
   SimulateSignIn();
@@ -156,16 +134,8 @@ TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
   EXPECT_EQ(0, account_picker()->GetSelectedIndex());
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_ShouldSelectAccountStoreByDefault \
-  DISABLED_ShouldSelectAccountStoreByDefault
-#else
-#define MAYBE_ShouldSelectAccountStoreByDefault \
-  ShouldSelectAccountStoreByDefault
-#endif
 TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
-       MAYBE_ShouldSelectAccountStoreByDefault) {
+       ShouldSelectAccountStoreByDefault) {
   ON_CALL(*feature_manager_mock(), ShouldShowAccountStorageBubbleUi)
       .WillByDefault(Return(true));
   ON_CALL(*feature_manager_mock(), GetDefaultPasswordStore)
@@ -183,16 +153,8 @@ TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
       account_picker()->GetTextForRow(account_picker()->GetSelectedIndex()));
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_ShouldSelectProfileStoreByDefault \
-  DISABLED_ShouldSelectProfileStoreByDefault
-#else
-#define MAYBE_ShouldSelectProfileStoreByDefault \
-  ShouldSelectProfileStoreByDefault
-#endif
 TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
-       MAYBE_ShouldSelectProfileStoreByDefault) {
+       ShouldSelectProfileStoreByDefault) {
   ON_CALL(*feature_manager_mock(), ShouldShowAccountStorageBubbleUi)
       .WillByDefault(Return(true));
   ON_CALL(*feature_manager_mock(), GetDefaultPasswordStore)
@@ -207,17 +169,9 @@ TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
       account_picker()->GetTextForRow(account_picker()->GetSelectedIndex()));
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_OnThemesChangedShouldNotCrashForFederatedCredentials \
-  DISABLED_OnThemesChangedShouldNotCrashForFederatedCredentials
-#else
-#define MAYBE_OnThemesChangedShouldNotCrashForFederatedCredentials \
-  OnThemesChangedShouldNotCrashForFederatedCredentials
-#endif
 // This is a regression test for crbug.com/1093290
 TEST_F(PasswordSaveUpdateWithAccountStoreViewTest,
-       MAYBE_OnThemesChangedShouldNotCrashForFederatedCredentials) {
+       OnThemesChangedShouldNotCrashForFederatedCredentials) {
   GURL kURL("https://example.com");
   url::Origin kOrigin = url::Origin::Create(kURL);
   ON_CALL(*model_delegate_mock(), GetOrigin).WillByDefault(Return(kOrigin));

chrome/browser/ui/views/passwords/post_save_compromised_bubble_view_unittest.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/ui/views/passwords/post_save_compromised_bubble_view.h"
 
-#include "build/build_config.h"
 #include "chrome/browser/ui/views/passwords/password_bubble_view_test_base.h"
 
 namespace {
@@ -45,13 +44,7 @@ TEST_F(PostSaveCompromisedBubbleViewTest, SafeState) {
   EXPECT_FALSE(view_->GetCancelButton());
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_MoreToFixState DISABLED_MoreToFixState
-#else
-#define MAYBE_MoreToFixState MoreToFixState
-#endif
-TEST_F(PostSaveCompromisedBubbleViewTest, MAYBE_MoreToFixState) {
+TEST_F(PostSaveCompromisedBubbleViewTest, MoreToFixState) {
   CreateViewAndShow(password_manager::ui::PASSWORD_UPDATED_MORE_TO_FIX);
   EXPECT_TRUE(view_->GetOkButton());
   EXPECT_FALSE(view_->GetCancelButton());
@@ -60,13 +53,7 @@ TEST_F(PostSaveCompromisedBubbleViewTest, MAYBE_MoreToFixState) {
   view_->AcceptDialog();
 }
 
-// Flaky on Windows due to http://crbug.com/968222
-#if defined(OS_WIN)
-#define MAYBE_UnsafeState DISABLED_UnsafeState
-#else
-#define MAYBE_UnsafeState UnsafeState
-#endif
-TEST_F(PostSaveCompromisedBubbleViewTest, MAYBE_UnsafeState) {
+TEST_F(PostSaveCompromisedBubbleViewTest, UnsafeState) {
   CreateViewAndShow(password_manager::ui::PASSWORD_UPDATED_UNSAFE_STATE);
   EXPECT_TRUE(view_->GetOkButton());
   EXPECT_FALSE(view_->GetCancelButton());

ui/base/ime/init/input_method_initializer.cc

@@ -62,7 +62,7 @@ void InitializeInputMethodForTesting() {
   LinuxInputMethodContextFactory::SetInstance(
       g_linux_input_method_context_factory_for_testing);
 #elif defined(OS_WIN)
-  TSFBridge::Initialize();
+  TSFBridge::InitializeForTesting();
 #endif
 }
 

ui/base/ime/win/BUILD.gn

@@ -15,6 +15,8 @@ component("win") {
     "input_method_win_imm32.h",
     "input_method_win_tsf.cc",
     "input_method_win_tsf.h",
+    "mock_tsf_bridge.cc",
+    "mock_tsf_bridge.h",
     "on_screen_keyboard_display_manager_input_pane.cc",
     "on_screen_keyboard_display_manager_input_pane.h",
     "on_screen_keyboard_display_manager_tab_tip.cc",

ui/base/ime/win/mock_tsf_bridge.cc

@@ -40,7 +40,6 @@ void MockTSFBridge::SetFocusedClient(HWND focused_window,
 
 void MockTSFBridge::RemoveFocusedClient(TextInputClient* client) {
   ++remove_focused_client_call_count_;
-  DCHECK_EQ(client, text_input_client_);
   text_input_client_ = nullptr;
   focused_window_ = nullptr;
 }

ui/base/ime/win/tsf_bridge.cc

@@ -15,6 +15,7 @@
 #include "base/win/scoped_variant.h"
 #include "ui/base/ime/input_method_delegate.h"
 #include "ui/base/ime/text_input_client.h"
+#include "ui/base/ime/win/mock_tsf_bridge.h"
 #include "ui/base/ime/win/tsf_bridge.h"
 #include "ui/base/ime/win/tsf_text_store.h"
 #include "ui/base/ui_base_features.h"
@@ -657,10 +658,22 @@ HRESULT TSFBridge::Initialize() {
   return delegate->Initialize();
 }
 
+// static
+void TSFBridge::InitializeForTesting() {
+  if (!base::CurrentUIThread::IsSet()) {
+    return;
+  }
+  TSFBridgeImpl* delegate = static_cast<TSFBridgeImpl*>(TSFBridgeTLS().Get());
+  if (delegate)
+    return;
+  if (!base::FeatureList::IsEnabled(features::kTSFImeSupport))
+    return;
+  TSFBridgeTLS().Set(new MockTSFBridge());
+}
+
 // static
 TSFBridge* TSFBridge::ReplaceForTesting(TSFBridge* bridge) {
   if (!base::CurrentUIThread::IsSet()) {
-    DVLOG(1) << "Do not use TSFBridge without UI thread.";
     return nullptr;
   }
   TSFBridge* old_bridge = TSFBridge::GetInstance();

ui/base/ime/win/tsf_bridge.h

@@ -38,6 +38,10 @@ class COMPONENT_EXPORT(UI_BASE_IME_WIN) TSFBridge {
   // GetInstance().
   static HRESULT Initialize();
 
+  // Sets the thread local instance for testing only. Must be called before
+  // any calls to GetInstance().
+  static void InitializeForTesting();
+
   // Injects an alternative TSFBridge such as MockTSFBridge for testing. The
   // injected object should be released by the caller. This function returns
   // previous TSFBridge pointer with ownership.