Moving the COM processing to a background thread
Avoid to process COM events when the input method class for windows is
in the process of been destructed, we should create an inner class that
moves the COM processing to a background thread, which controls the
lifetime of all the COM objects.
Bug: 852386, 497381
Change-Id: Ia2517fb8ad813924d45ce58535b09360114d2f30
Reviewed-on: https://chromium-review.googlesource.com/c/1258182
Commit-Queue: Lan Wei <lanwei@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622887}diff --git a/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.cc b/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.cc
index ed7c451..80899ee 100644
--- a/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.cc
+++ b/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.cc
@@ -5,6 +5,7 @@
#include "ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
#include "base/win/com_init_util.h"
#include "base/win/core_winrt_util.h"
#include "base/win/windows_version.h"
@@ -12,30 +13,206 @@
namespace ui {
+// VirtualKeyboardInputPane class is used to store all the COM objects and
+// control their lifetime, so all the COM processing is on a background
+// thread.
+class OnScreenKeyboardDisplayManagerInputPane::VirtualKeyboardInputPane
+ : public base::RefCountedThreadSafe<VirtualKeyboardInputPane> {
+ public:
+ explicit VirtualKeyboardInputPane(
+ const scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+ : main_task_runner_(task_runner) {}
+
+ void InitVirtualKeyboardInputPaneInstance(
+ base::WeakPtr<OnScreenKeyboardDisplayManagerInputPane>
+ input_pane_weak_ptr) {
+ keyboard_input_pane_weak_ptr_ = input_pane_weak_ptr;
+ }
+
+ // Set the virtual keyboard input pane for |OnScreenKeyboardTest| tests.
+ void SetInputPaneForTestingInBackgroundThread(
+ Microsoft::WRL::ComPtr<ABI::Windows::UI::ViewManagement::IInputPane>
+ pane) {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ DCHECK(!input_pane_);
+ input_pane_ = pane;
+ HRESULT hr = input_pane_.As(&input_pane2_);
+ DCHECK(SUCCEEDED(hr));
+
+ AddCallbacksOnInputPaneShownOrHiddenInBackgroundThread();
+ }
+
+ void TryShowInBackgroundThread(HWND hwnd) {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ if (!EnsureInputPanePointersInBackgroundThread(hwnd))
+ return;
+ boolean res;
+ input_pane2_->TryShow(&res);
+ }
+
+ void TryHideInBackgroundThread() {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ if (!input_pane2_)
+ return;
+ boolean res;
+ input_pane2_->TryHide(&res);
+ }
+
+ private:
+ friend class base::RefCountedThreadSafe<VirtualKeyboardInputPane>;
+
+ ~VirtualKeyboardInputPane() {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ }
+
+ bool EnsureInputPanePointersInBackgroundThread(HWND hwnd) {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ if (input_pane2_)
+ return true;
+ if (!base::win::ResolveCoreWinRTDelayload() ||
+ !base::win::ScopedHString::ResolveCoreWinRTStringDelayload()) {
+ return false;
+ }
+
+ base::win::AssertComApartmentType(base::win::ComApartmentType::STA);
+
+ base::win::ScopedHString input_pane_guid = base::win::ScopedHString::Create(
+ RuntimeClass_Windows_UI_ViewManagement_InputPane);
+ Microsoft::WRL::ComPtr<IInputPaneInterop> input_pane_interop;
+ HRESULT hr = base::win::RoGetActivationFactory(
+ input_pane_guid.get(), IID_PPV_ARGS(&input_pane_interop));
+ if (FAILED(hr))
+ return false;
+
+ hr = input_pane_interop->GetForWindow(hwnd, IID_PPV_ARGS(&input_pane_));
+ if (FAILED(hr))
+ return false;
+
+ if (FAILED(input_pane_.As(&input_pane2_)))
+ return false;
+
+ AddCallbacksOnInputPaneShownOrHiddenInBackgroundThread();
+ return true;
+ }
+
+ // Add callbacks to notify virtual keyboard observers when the virtual
+ // keyboard is visible or hidden.
+ void AddCallbacksOnInputPaneShownOrHiddenInBackgroundThread() {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ input_pane_->add_Showing(
+ Microsoft::WRL::Callback<
+ OnScreenKeyboardDisplayManagerInputPane::VirtualKeyboardInputPane::
+ InputPaneEventHandler>(
+ this, &OnScreenKeyboardDisplayManagerInputPane::
+ VirtualKeyboardInputPane::OnInputPaneShown)
+ .Get(),
+ &show_event_token_);
+
+ input_pane_->add_Hiding(
+ Microsoft::WRL::Callback<
+ OnScreenKeyboardDisplayManagerInputPane::VirtualKeyboardInputPane::
+ InputPaneEventHandler>(
+ this, &OnScreenKeyboardDisplayManagerInputPane::
+ VirtualKeyboardInputPane::OnInputPaneHidden)
+ .Get(),
+ &hide_event_token_);
+ }
+
+ HRESULT OnInputPaneShown(
+ ABI::Windows::UI::ViewManagement::IInputPane* pane,
+ ABI::Windows::UI::ViewManagement::IInputPaneVisibilityEventArgs* args) {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ ABI::Windows::Foundation::Rect rect;
+ input_pane_->get_OccludedRect(&rect);
+ gfx::Rect dip_rect(rect.X, rect.Y, rect.Width, rect.Height);
+
+ main_task_runner_->PostTask(
+ FROM_HERE, base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::
+ NotifyObserversOnKeyboardShown,
+ keyboard_input_pane_weak_ptr_, dip_rect));
+ return S_OK;
+ }
+
+ HRESULT OnInputPaneHidden(
+ ABI::Windows::UI::ViewManagement::IInputPane* pane,
+ ABI::Windows::UI::ViewManagement::IInputPaneVisibilityEventArgs* args) {
+ DCHECK(!main_task_runner_->BelongsToCurrentThread());
+ main_task_runner_->PostTask(
+ FROM_HERE, base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::
+ NotifyObserversOnKeyboardHidden,
+ keyboard_input_pane_weak_ptr_));
+ return S_OK;
+ }
+
+ using InputPaneEventHandler = ABI::Windows::Foundation::ITypedEventHandler<
+ ABI::Windows::UI::ViewManagement::InputPane*,
+ ABI::Windows::UI::ViewManagement::InputPaneVisibilityEventArgs*>;
+
+ // InputPane objects are owned by VirtualKeyboardInputPane class and their
+ // functions are ran on a background thread.
+ Microsoft::WRL::ComPtr<ABI::Windows::UI::ViewManagement::IInputPane>
+ input_pane_;
+ Microsoft::WRL::ComPtr<ABI::Windows::UI::ViewManagement::IInputPane2>
+ input_pane2_;
+
+ EventRegistrationToken show_event_token_;
+ EventRegistrationToken hide_event_token_;
+
+ // |main_task_runner_| and |keyboard_input_pane_weak_ptr_| are owned by
+ // OnScreenKeyboardDisplayManagerInputPane class, and they are running on the
+ // main thread, which are used to post task to the main thread from the
+ // background thread.
+ scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+ base::WeakPtr<OnScreenKeyboardDisplayManagerInputPane>
+ keyboard_input_pane_weak_ptr_;
+
+ DISALLOW_COPY_AND_ASSIGN(VirtualKeyboardInputPane);
+};
+
OnScreenKeyboardDisplayManagerInputPane::
OnScreenKeyboardDisplayManagerInputPane(HWND hwnd)
: hwnd_(hwnd),
main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+ background_task_runner_(
+ base::CreateCOMSTATaskRunnerWithTraits({base::MayBlock()})),
+ virtual_keyboard_input_pane_(
+ base::MakeRefCounted<OnScreenKeyboardDisplayManagerInputPane::
+ VirtualKeyboardInputPane>(
+ main_task_runner_)),
+ is_keyboard_visible_(false),
weak_factory_(this) {
DCHECK_GE(base::win::GetVersion(), base::win::VERSION_WIN10_RS1);
+ DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+ // We post the initiation of |virtual_keyboard_input_pane_| to the background
+ // thread first, and any other tasks posted to the background thread are
+ // executed after its initiation.
+ background_task_runner_->PostTask(
+ FROM_HERE,
+ base::BindOnce(
+ &OnScreenKeyboardDisplayManagerInputPane::VirtualKeyboardInputPane::
+ InitVirtualKeyboardInputPaneInstance,
+ base::RetainedRef(virtual_keyboard_input_pane_),
+ weak_factory_.GetWeakPtr()));
}
-OnScreenKeyboardDisplayManagerInputPane::
- ~OnScreenKeyboardDisplayManagerInputPane() = default;
-
bool OnScreenKeyboardDisplayManagerInputPane::DisplayVirtualKeyboard() {
- main_task_runner_->PostTask(
+ DCHECK(main_task_runner_->BelongsToCurrentThread());
+ background_task_runner_->PostTask(
FROM_HERE,
- base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::TryShow,
- weak_factory_.GetWeakPtr()));
+ base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::
+ VirtualKeyboardInputPane::TryShowInBackgroundThread,
+ base::RetainedRef(virtual_keyboard_input_pane_), hwnd_));
return true;
}
void OnScreenKeyboardDisplayManagerInputPane::DismissVirtualKeyboard() {
- main_task_runner_->PostTask(
+ DCHECK(main_task_runner_->BelongsToCurrentThread());
+ background_task_runner_->PostTask(
FROM_HERE,
- base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::TryHide,
- weak_factory_.GetWeakPtr()));
+ base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::
+ VirtualKeyboardInputPane::TryHideInBackgroundThread,
+ base::RetainedRef(virtual_keyboard_input_pane_)));
}
void OnScreenKeyboardDisplayManagerInputPane::AddObserver(
@@ -51,102 +228,26 @@
}
bool OnScreenKeyboardDisplayManagerInputPane::IsKeyboardVisible() {
- if (!EnsureInputPanePointers())
- return false;
- ABI::Windows::Foundation::Rect rect;
- input_pane_->get_OccludedRect(&rect);
- // Height == 0 is special indicating it is floating on only check width.
- return rect.Width != 0;
+ DCHECK(main_task_runner_->BelongsToCurrentThread());
+ return is_keyboard_visible_;
}
void OnScreenKeyboardDisplayManagerInputPane::SetInputPaneForTesting(
- ABI::Windows::UI::ViewManagement::IInputPane* input_pane) {
- DCHECK(!input_pane_);
- input_pane_ = input_pane;
- HRESULT hr = input_pane_.As(&input_pane2_);
- DCHECK(SUCCEEDED(hr));
-
- input_pane_->add_Showing(
- Microsoft::WRL::Callback<InputPaneEventHandler>(
- this, &OnScreenKeyboardDisplayManagerInputPane::InputPaneShown)
- .Get(),
- &show_event_token_);
- input_pane_->add_Hiding(
- Microsoft::WRL::Callback<InputPaneEventHandler>(
- this, &OnScreenKeyboardDisplayManagerInputPane::InputPaneHidden)
- .Get(),
- &hide_event_token_);
-}
-
-bool OnScreenKeyboardDisplayManagerInputPane::EnsureInputPanePointers() {
- if (input_pane2_)
- return true;
- if (!base::win::ResolveCoreWinRTDelayload() ||
- !base::win::ScopedHString::ResolveCoreWinRTStringDelayload()) {
- return false;
- }
-
- // TODO(dtapuska): https://crbug.com/852386. Use TaskScheduler to access the
- // WinRT APIs.
- base::win::AssertComApartmentType(base::win::ComApartmentType::STA);
-
- base::win::ScopedHString input_pane_guid = base::win::ScopedHString::Create(
- RuntimeClass_Windows_UI_ViewManagement_InputPane);
- Microsoft::WRL::ComPtr<IInputPaneInterop> input_pane_interop;
- HRESULT hr = base::win::RoGetActivationFactory(
- input_pane_guid.get(), IID_PPV_ARGS(&input_pane_interop));
- if (FAILED(hr))
- return false;
-
- hr = input_pane_interop->GetForWindow(hwnd_, IID_PPV_ARGS(&input_pane_));
- if (FAILED(hr))
- return false;
-
- hr = input_pane_.As(&input_pane2_);
- if (FAILED(hr))
- return false;
-
- input_pane_->add_Showing(
- Microsoft::WRL::Callback<InputPaneEventHandler>(
- this, &OnScreenKeyboardDisplayManagerInputPane::InputPaneShown)
- .Get(),
- &show_event_token_);
- input_pane_->add_Hiding(
- Microsoft::WRL::Callback<InputPaneEventHandler>(
- this, &OnScreenKeyboardDisplayManagerInputPane::InputPaneHidden)
- .Get(),
- &hide_event_token_);
- return true;
-}
-
-HRESULT OnScreenKeyboardDisplayManagerInputPane::InputPaneShown(
- ABI::Windows::UI::ViewManagement::IInputPane* pane,
- ABI::Windows::UI::ViewManagement::IInputPaneVisibilityEventArgs* args) {
- // get_OccludedRect is in DIPs already.
- ABI::Windows::Foundation::Rect rect;
- input_pane_->get_OccludedRect(&rect);
- gfx::Rect dip_rect(rect.X, rect.Y, rect.Width, rect.Height);
-
- main_task_runner_->PostTask(
- FROM_HERE, base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::
- NotifyObserversOnKeyboardShown,
- weak_factory_.GetWeakPtr(), dip_rect));
- return S_OK;
-}
-
-HRESULT OnScreenKeyboardDisplayManagerInputPane::InputPaneHidden(
- ABI::Windows::UI::ViewManagement::IInputPane* pane,
- ABI::Windows::UI::ViewManagement::IInputPaneVisibilityEventArgs* args) {
- main_task_runner_->PostTask(
- FROM_HERE, base::BindOnce(&OnScreenKeyboardDisplayManagerInputPane::
- NotifyObserversOnKeyboardHidden,
- weak_factory_.GetWeakPtr()));
- return S_OK;
+ Microsoft::WRL::ComPtr<ABI::Windows::UI::ViewManagement::IInputPane> pane) {
+ DCHECK(main_task_runner_->BelongsToCurrentThread());
+ base::CreateCOMSTATaskRunnerWithTraits({base::MayBlock()})
+ ->PostTask(FROM_HERE,
+ base::BindOnce(
+ &OnScreenKeyboardDisplayManagerInputPane::
+ VirtualKeyboardInputPane::
+ SetInputPaneForTestingInBackgroundThread,
+ base::RetainedRef(virtual_keyboard_input_pane_), pane));
}
void OnScreenKeyboardDisplayManagerInputPane::NotifyObserversOnKeyboardShown(
gfx::Rect dip_rect) {
DCHECK(main_task_runner_->BelongsToCurrentThread());
+ is_keyboard_visible_ = true;
for (InputMethodKeyboardControllerObserver& observer : observers_)
observer.OnKeyboardVisible(dip_rect);
}
@@ -154,22 +255,18 @@
void OnScreenKeyboardDisplayManagerInputPane::
NotifyObserversOnKeyboardHidden() {
DCHECK(main_task_runner_->BelongsToCurrentThread());
+ is_keyboard_visible_ = false;
for (InputMethodKeyboardControllerObserver& observer : observers_)
observer.OnKeyboardHidden();
}
-void OnScreenKeyboardDisplayManagerInputPane::TryShow() {
- if (!EnsureInputPanePointers())
- return;
- boolean res;
- input_pane2_->TryShow(&res);
+OnScreenKeyboardDisplayManagerInputPane::
+ ~OnScreenKeyboardDisplayManagerInputPane() {
+ DCHECK(main_task_runner_->BelongsToCurrentThread());
+ if (virtual_keyboard_input_pane_.get()) {
+ background_task_runner_->ReleaseSoon(
+ FROM_HERE, std::move(virtual_keyboard_input_pane_));
+ }
}
-void OnScreenKeyboardDisplayManagerInputPane::TryHide() {
- if (!input_pane2_)
- return;
- boolean res;
- input_pane2_->TryHide(&res);
-}
-
-} // namespace ui
+} // namespace ui
\ No newline at end of file
diff --git a/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.h b/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.h
index 2a359fb..f97533f 100644
--- a/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.h
+++ b/ui/base/ime/win/on_screen_keyboard_display_manager_input_pane.h
@@ -29,7 +29,7 @@
class UI_BASE_IME_EXPORT OnScreenKeyboardDisplayManagerInputPane final
: public InputMethodKeyboardController {
public:
- OnScreenKeyboardDisplayManagerInputPane(HWND hwnd);
+ explicit OnScreenKeyboardDisplayManagerInputPane(HWND hwnd);
~OnScreenKeyboardDisplayManagerInputPane() override;
// InputMethodKeyboardController:
@@ -40,39 +40,24 @@
bool IsKeyboardVisible() override;
void SetInputPaneForTesting(
- ABI::Windows::UI::ViewManagement::IInputPane* pane);
+ Microsoft::WRL::ComPtr<ABI::Windows::UI::ViewManagement::IInputPane>
+ pane);
private:
+ class VirtualKeyboardInputPane;
friend class OnScreenKeyboardTest;
- bool EnsureInputPanePointers();
- HRESULT InputPaneShown(
- ABI::Windows::UI::ViewManagement::IInputPane* pane,
- ABI::Windows::UI::ViewManagement::IInputPaneVisibilityEventArgs* args);
- HRESULT InputPaneHidden(
- ABI::Windows::UI::ViewManagement::IInputPane* pane,
- ABI::Windows::UI::ViewManagement::IInputPaneVisibilityEventArgs* args);
-
void NotifyObserversOnKeyboardShown(gfx::Rect rect);
void NotifyObserversOnKeyboardHidden();
- void TryShow();
- void TryHide();
-
- using InputPaneEventHandler = ABI::Windows::Foundation::ITypedEventHandler<
- ABI::Windows::UI::ViewManagement::InputPane*,
- ABI::Windows::UI::ViewManagement::InputPaneVisibilityEventArgs*>;
// The main window which displays the on screen keyboard.
const HWND hwnd_;
- Microsoft::WRL::ComPtr<ABI::Windows::UI::ViewManagement::IInputPane>
- input_pane_;
- Microsoft::WRL::ComPtr<ABI::Windows::UI::ViewManagement::IInputPane2>
- input_pane2_;
base::ObserverList<InputMethodKeyboardControllerObserver, false>::Unchecked
observers_;
- EventRegistrationToken show_event_token_;
- EventRegistrationToken hide_event_token_;
const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+ const scoped_refptr<base::SingleThreadTaskRunner> background_task_runner_;
+ scoped_refptr<VirtualKeyboardInputPane> virtual_keyboard_input_pane_;
+ bool is_keyboard_visible_;
base::WeakPtrFactory<OnScreenKeyboardDisplayManagerInputPane> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(OnScreenKeyboardDisplayManagerInputPane);
diff --git a/ui/base/ime/win/on_screen_keyboard_display_manager_unittest.cc b/ui/base/ime/win/on_screen_keyboard_display_manager_unittest.cc
index 3a17be7a..6cc5319 100644
--- a/ui/base/ime/win/on_screen_keyboard_display_manager_unittest.cc
+++ b/ui/base/ime/win/on_screen_keyboard_display_manager_unittest.cc
@@ -10,6 +10,7 @@
#include "base/macros.h"
#include "base/message_loop/message_loop.h"
#include "base/strings/string16.h"
+#include "base/test/scoped_task_environment.h"
#include "base/win/windows_version.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -106,7 +107,9 @@
class OnScreenKeyboardTest : public ::testing::Test {
protected:
- OnScreenKeyboardTest() = default;
+ OnScreenKeyboardTest()
+ : scoped_task_environment_(
+ base::test::ScopedTaskEnvironment::MainThreadType::UI) {}
std::unique_ptr<OnScreenKeyboardDisplayManagerTabTip> CreateTabTip() {
return std::unique_ptr<OnScreenKeyboardDisplayManagerTabTip>(
@@ -118,8 +121,16 @@
new OnScreenKeyboardDisplayManagerInputPane(nullptr));
}
+ void WaitForEventsWithTimeDelay(int64_t time_delta_ms = 10) {
+ base::RunLoop run_loop;
+ scoped_task_environment_.GetMainThreadTaskRunner()->PostDelayedTask(
+ FROM_HERE, run_loop.QuitClosure(),
+ base::TimeDelta::FromMilliseconds(time_delta_ms));
+ run_loop.Run();
+ }
+
private:
- base::MessageLoop message_loop_;
+ base::test::ScopedTaskEnvironment scoped_task_environment_;
DISALLOW_COPY_AND_ASSIGN(OnScreenKeyboardTest);
};
@@ -163,17 +174,17 @@
Microsoft::WRL::ComPtr<MockInputPane> input_pane =
Microsoft::WRL::Make<MockInputPane>();
- keyboard_display_manager->SetInputPaneForTesting(input_pane.Get());
+ keyboard_display_manager->SetInputPaneForTesting(input_pane);
EXPECT_CALL(*observer, OnKeyboardVisible(testing::_)).Times(1);
keyboard_display_manager->AddObserver(observer.get());
keyboard_display_manager->DisplayVirtualKeyboard();
- base::RunLoop().RunUntilIdle();
+ WaitForEventsWithTimeDelay(100);
testing::Mock::VerifyAndClearExpectations(observer.get());
EXPECT_CALL(*observer, OnKeyboardHidden()).Times(1);
keyboard_display_manager->DismissVirtualKeyboard();
- base::RunLoop().RunUntilIdle();
+ WaitForEventsWithTimeDelay(100);
keyboard_display_manager->RemoveObserver(observer.get());
}
