From 50f217f0836edece6908d1724541b464fd97dd9a Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Fri, 12 Aug 2022 11:24:23 -0700 Subject: [PATCH 01/10] [Windows] Don't crash if GetPointerType is unavailable --- shell/platform/windows/BUILD.gn | 4 + shell/platform/windows/direct_manipulation.h | 3 +- .../testing/mock_direct_manipulation.h | 30 ++++++ shell/platform/windows/testing/mock_window.cc | 10 +- shell/platform/windows/testing/mock_window.h | 7 +- .../windows/testing/mock_windows_proc_table.h | 30 ++++++ shell/platform/windows/window.cc | 19 ++-- shell/platform/windows/window.h | 8 +- shell/platform/windows/window_unittests.cc | 101 ++++++++++++++++-- shell/platform/windows/windows_proc_table.cc | 28 +++++ shell/platform/windows/windows_proc_table.h | 42 ++++++++ 11 files changed, 262 insertions(+), 20 deletions(-) create mode 100644 shell/platform/windows/testing/mock_direct_manipulation.h create mode 100644 shell/platform/windows/testing/mock_windows_proc_table.h create mode 100644 shell/platform/windows/windows_proc_table.cc create mode 100644 shell/platform/windows/windows_proc_table.h diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index 194e17f1f1e6d..fd570feb1abae 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -103,6 +103,8 @@ source_set("flutter_windows_source") { "window_proc_delegate_manager.cc", "window_proc_delegate_manager.h", "window_state.h", + "windows_proc_table.cc", + "windows_proc_table.h", ] libs = [ @@ -189,6 +191,7 @@ executable("flutter_windows_unittests") { "testing/engine_modifier.h", "testing/flutter_window_test.cc", "testing/flutter_window_test.h", + "testing/mock_direct_manipulation.h", "testing/mock_gl_functions.h", "testing/mock_text_input_manager.cc", "testing/mock_text_input_manager.h", @@ -196,6 +199,7 @@ executable("flutter_windows_unittests") { "testing/mock_window.h", "testing/mock_window_binding_handler.cc", "testing/mock_window_binding_handler.h", + "testing/mock_windows_proc_table.h", "testing/test_keyboard.cc", "testing/test_keyboard.h", "testing/test_keyboard_unittests.cc", diff --git a/shell/platform/windows/direct_manipulation.h b/shell/platform/windows/direct_manipulation.h index f25470ab4235d..5bac868dcbc25 100644 --- a/shell/platform/windows/direct_manipulation.h +++ b/shell/platform/windows/direct_manipulation.h @@ -22,6 +22,7 @@ class DirectManipulationEventHandler; class DirectManipulationOwner { public: explicit DirectManipulationOwner(Window* window); + virtual ~DirectManipulationOwner() = default; // Initialize a DirectManipulation viewport with specified width and height. // These should match the width and height of the application window. int Init(unsigned int width, unsigned int height); @@ -34,7 +35,7 @@ class DirectManipulationOwner { WindowBindingHandlerDelegate* binding_handler_delegate); // Called when DM_POINTERHITTEST occurs with an acceptable pointer type. Will // start DirectManipulation for that interaction. - void SetContact(UINT contactId); + virtual void SetContact(UINT contactId); // Called to get updates from DirectManipulation. Should be called frequently // to provide smooth updates. void Update(); diff --git a/shell/platform/windows/testing/mock_direct_manipulation.h b/shell/platform/windows/testing/mock_direct_manipulation.h new file mode 100644 index 0000000000000..d160f65e006e0 --- /dev/null +++ b/shell/platform/windows/testing/mock_direct_manipulation.h @@ -0,0 +1,30 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_DIRECT_MANIPULATION_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_DIRECT_MANIPULATION_H_ + +#include "flutter/shell/platform/windows/direct_manipulation.h" +#include "gmock/gmock.h" + +namespace flutter { +namespace testing { + +/// Mock for the |DirectManipulationOwner| base class. +class MockDirectManipulationOwner : public DirectManipulationOwner { + public: + MockDirectManipulationOwner(Window* window) + : DirectManipulationOwner(window){}; + virtual ~MockDirectManipulationOwner() = default; + + MOCK_METHOD1(SetContact, void(UINT contactId)); + + private: + FML_DISALLOW_COPY_AND_ASSIGN(MockDirectManipulationOwner); +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_DIRECT_MANIPULATION_H_ diff --git a/shell/platform/windows/testing/mock_window.cc b/shell/platform/windows/testing/mock_window.cc index a6d3dcda64b14..9021aecb3352a 100644 --- a/shell/platform/windows/testing/mock_window.cc +++ b/shell/platform/windows/testing/mock_window.cc @@ -7,8 +7,9 @@ namespace flutter { namespace testing { MockWindow::MockWindow() : Window(){}; -MockWindow::MockWindow(std::unique_ptr text_input_manager) - : Window(std::move(text_input_manager)){}; +MockWindow::MockWindow(std::unique_ptr window_proc_table, + std::unique_ptr text_input_manager) + : Window(std::move(window_proc_table), std::move(text_input_manager)){}; MockWindow::~MockWindow() = default; @@ -23,6 +24,11 @@ LRESULT MockWindow::Win32DefWindowProc(HWND hWnd, return kWmResultDefault; } +void MockWindow::SetDirectManipulationOwner( + std::unique_ptr owner) { + direct_manipulation_owner_ = std::move(owner); +} + LRESULT MockWindow::InjectWindowMessage(UINT const message, WPARAM const wparam, LPARAM const lparam) { diff --git a/shell/platform/windows/testing/mock_window.h b/shell/platform/windows/testing/mock_window.h index 107e016c390f8..8885096fa1d28 100644 --- a/shell/platform/windows/testing/mock_window.h +++ b/shell/platform/windows/testing/mock_window.h @@ -18,7 +18,8 @@ namespace testing { class MockWindow : public Window { public: MockWindow(); - MockWindow(std::unique_ptr text_input_manager); + MockWindow(std::unique_ptr windows_proc_table, + std::unique_ptr text_input_manager); virtual ~MockWindow(); // Prevent copying. @@ -28,6 +29,10 @@ class MockWindow : public Window { // Wrapper for GetCurrentDPI() which is a protected method. UINT GetDpi(); + // Set the Direct Manipulation owner for testing purposes. + void SetDirectManipulationOwner( + std::unique_ptr owner); + // Simulates a WindowProc message from the OS. LRESULT InjectWindowMessage(UINT const message, WPARAM const wparam, diff --git a/shell/platform/windows/testing/mock_windows_proc_table.h b/shell/platform/windows/testing/mock_windows_proc_table.h new file mode 100644 index 0000000000000..7c136def68654 --- /dev/null +++ b/shell/platform/windows/testing/mock_windows_proc_table.h @@ -0,0 +1,30 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WINDOWS_PROC_TABLE_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WINDOWS_PROC_TABLE_H_ + +#include "flutter/shell/platform/windows/windows_proc_table.h" +#include "gmock/gmock.h" + +namespace flutter { +namespace testing { + +/// Mock for the |WindowsProcTable| base class. +class MockWindowsProcTable : public WindowsProcTable { + public: + MockWindowsProcTable() = default; + virtual ~MockWindowsProcTable() = default; + + MOCK_METHOD2(GetPointerType, + BOOL(UINT32 pointer_id, POINTER_INPUT_TYPE* pointer_type)); + + private: + FML_DISALLOW_COPY_AND_ASSIGN(MockWindowsProcTable); +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_MOCK_WINDOWS_PROC_TABLE_H_ diff --git a/shell/platform/windows/window.cc b/shell/platform/windows/window.cc index a61ac35a4c909..08f5368a1472a 100644 --- a/shell/platform/windows/window.cc +++ b/shell/platform/windows/window.cc @@ -52,10 +52,12 @@ static const int kLinesPerScrollWindowsDefault = 3; } // namespace -Window::Window() : Window(nullptr) {} +Window::Window() : Window(nullptr, nullptr) {} -Window::Window(std::unique_ptr text_input_manager) +Window::Window(std::unique_ptr windows_proc_table, + std::unique_ptr text_input_manager) : touch_id_generator_(kMinTouchDeviceId, kMaxTouchDeviceId), + windows_proc_table_(std::move(windows_proc_table)), text_input_manager_(std::move(text_input_manager)) { // Get the DPI of the primary monitor as the initial DPI. If Per-Monitor V2 is // supported, |current_dpi_| should be updated in the @@ -67,6 +69,9 @@ Window::Window(std::unique_ptr text_input_manager) // https://github.com/flutter/flutter/issues/107248 UpdateScrollOffsetMultiplier(); + if (windows_proc_table_ == nullptr) { + windows_proc_table_ = std::make_unique(); + } if (text_input_manager_ == nullptr) { text_input_manager_ = std::make_unique(); } @@ -482,11 +487,11 @@ Window::HandleMessage(UINT const message, break; case DM_POINTERHITTEST: { if (direct_manipulation_owner_) { - UINT contactId = GET_POINTERID_WPARAM(wparam); - POINTER_INPUT_TYPE pointerType; - if (GetPointerType(contactId, &pointerType) && - pointerType == PT_TOUCHPAD) { - direct_manipulation_owner_->SetContact(contactId); + UINT contact_id = GET_POINTERID_WPARAM(wparam); + POINTER_INPUT_TYPE pointer_type; + if (windows_proc_table_->GetPointerType(contact_id, &pointer_type) && + pointer_type == PT_TOUCHPAD) { + direct_manipulation_owner_->SetContact(contact_id); } } break; diff --git a/shell/platform/windows/window.h b/shell/platform/windows/window.h index fe62fa5d2e965..7782803b10a52 100644 --- a/shell/platform/windows/window.h +++ b/shell/platform/windows/window.h @@ -18,6 +18,7 @@ #include "flutter/shell/platform/windows/keyboard_manager.h" #include "flutter/shell/platform/windows/sequential_id_generator.h" #include "flutter/shell/platform/windows/text_input_manager.h" +#include "flutter/shell/platform/windows/windows_proc_table.h" #include "flutter/third_party/accessibility/gfx/native_widget_types.h" namespace flutter { @@ -28,7 +29,8 @@ namespace flutter { class Window : public KeyboardManager::WindowDelegate { public: Window(); - Window(std::unique_ptr text_input_manager); + Window(std::unique_ptr windows_proc_table, + std::unique_ptr text_input_manager); virtual ~Window(); // Initializes as a child window with size using |width| and |height| and @@ -266,6 +268,10 @@ class Window : public KeyboardManager::WindowDelegate { double mouse_x_ = 0; double mouse_y_ = 0; + // Abstracts Windows APIs that may not be available on all supported versions + // of Windows. + std::unique_ptr windows_proc_table_; + // Manages IME state. std::unique_ptr text_input_manager_; diff --git a/shell/platform/windows/window_unittests.cc b/shell/platform/windows/window_unittests.cc index 22750e2755a90..4546bd54370af 100644 --- a/shell/platform/windows/window_unittests.cc +++ b/shell/platform/windows/window_unittests.cc @@ -4,11 +4,15 @@ #include +#include "flutter/shell/platform/windows/testing/mock_direct_manipulation.h" #include "flutter/shell/platform/windows/testing/mock_text_input_manager.h" #include "flutter/shell/platform/windows/testing/mock_window.h" +#include "flutter/shell/platform/windows/testing/mock_windows_proc_table.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::Return; @@ -39,9 +43,11 @@ TEST(MockWindow, VerticalScroll) { } TEST(MockWindow, OnImeCompositionCompose) { - MockTextInputManager* text_input_manager = new MockTextInputManager(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManager(); std::unique_ptr text_input_manager_ptr(text_input_manager); - MockWindow window(std::move(text_input_manager_ptr)); + MockWindow window(std::move(windows_proc_table), + std::move(text_input_manager_ptr)); EXPECT_CALL(*text_input_manager, GetComposingString()) .WillRepeatedly( Return(std::optional(std::u16string(u"nihao")))); @@ -63,9 +69,11 @@ TEST(MockWindow, OnImeCompositionCompose) { } TEST(MockWindow, OnImeCompositionResult) { - MockTextInputManager* text_input_manager = new MockTextInputManager(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManager(); std::unique_ptr text_input_manager_ptr(text_input_manager); - MockWindow window(std::move(text_input_manager_ptr)); + MockWindow window(std::move(windows_proc_table), + std::move(text_input_manager_ptr)); EXPECT_CALL(*text_input_manager, GetComposingString()) .WillRepeatedly( Return(std::optional(std::u16string(u"nihao")))); @@ -87,9 +95,11 @@ TEST(MockWindow, OnImeCompositionResult) { } TEST(MockWindow, OnImeCompositionResultAndCompose) { - MockTextInputManager* text_input_manager = new MockTextInputManager(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManager(); std::unique_ptr text_input_manager_ptr(text_input_manager); - MockWindow window(std::move(text_input_manager_ptr)); + MockWindow window(std::move(windows_proc_table), + std::move(text_input_manager_ptr)); // This situation is that Google Japanese Input finished composing "今日" in // "今日は" but is still composing "は". @@ -123,9 +133,11 @@ TEST(MockWindow, OnImeCompositionResultAndCompose) { } TEST(MockWindow, OnImeCompositionClearChange) { - MockTextInputManager* text_input_manager = new MockTextInputManager(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManager(); std::unique_ptr text_input_manager_ptr(text_input_manager); - MockWindow window(std::move(text_input_manager_ptr)); + MockWindow window(std::move(windows_proc_table), + std::move(text_input_manager_ptr)); EXPECT_CALL(window, OnComposeChange(std::u16string(u""), 0)).Times(1); EXPECT_CALL(window, OnComposeCommit()).Times(1); ON_CALL(window, OnImeComposition) @@ -272,5 +284,78 @@ TEST(MockWindow, Paint) { window.InjectWindowMessage(WM_PAINT, 0, 0); } +TEST(MockWindow, PointerHitTest) { + auto* windows_proc_table = new MockWindowsProcTable(); + auto text_input_manager = std::make_unique(); + + MockWindow window(std::unique_ptr(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = new MockDirectManipulationOwner(&window); + window.SetDirectManipulationOwner( + std::unique_ptr(direct_manipulation)); + + UINT32 pointer_id = 123; + + EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) + .Times(1) + .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { + *type = PT_POINTER; + return TRUE; + }); + + EXPECT_CALL(*direct_manipulation, SetContact).Times(0); + + window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); +} + +TEST(MockWindow, TouchPadHitTest) { + auto* windows_proc_table = new MockWindowsProcTable(); + auto text_input_manager = std::make_unique(); + + MockWindow window(std::unique_ptr(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = new MockDirectManipulationOwner(&window); + window.SetDirectManipulationOwner( + std::unique_ptr(direct_manipulation)); + + UINT32 pointer_id = 123; + + EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) + .Times(1) + .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { + *type = PT_TOUCHPAD; + return TRUE; + }); + + EXPECT_CALL(*direct_manipulation, SetContact(Eq(pointer_id))).Times(1); + + window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); +} + +TEST(MockWindow, UnknownPointerTypeSkipsDirectManipulation) { + auto* windows_proc_table = new MockWindowsProcTable(); + auto text_input_manager = std::make_unique(); + + MockWindow window(std::unique_ptr(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = new MockDirectManipulationOwner(&window); + window.SetDirectManipulationOwner( + std::unique_ptr(direct_manipulation)); + + UINT32 pointer_id = 123; + + EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) + .Times(1) + .WillOnce( + [](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { return FALSE; }); + + EXPECT_CALL(*direct_manipulation, SetContact).Times(0); + + window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/windows_proc_table.cc b/shell/platform/windows/windows_proc_table.cc new file mode 100644 index 0000000000000..d2b5bd99f4264 --- /dev/null +++ b/shell/platform/windows/windows_proc_table.cc @@ -0,0 +1,28 @@ +// Copyright 2013 The Flutter 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 "flutter/shell/platform/windows/windows_proc_table.h" + +namespace flutter { + +WindowsProcTable::WindowsProcTable() { + user32_ = fml::NativeLibrary::Create("user32.dll"); + get_pointer_type_ = + user32_->ResolveFunction("GetPointerType"); +} + +WindowsProcTable::~WindowsProcTable() { + user32_ = nullptr; +} + +BOOL WindowsProcTable::GetPointerType(UINT32 pointer_id, + POINTER_INPUT_TYPE* pointer_type) { + if (!get_pointer_type_.has_value()) { + return FALSE; + } + + return get_pointer_type_.value()(pointer_id, pointer_type); +} + +} // namespace flutter diff --git a/shell/platform/windows/windows_proc_table.h b/shell/platform/windows/windows_proc_table.h new file mode 100644 index 0000000000000..4a6edae11c9f2 --- /dev/null +++ b/shell/platform/windows/windows_proc_table.h @@ -0,0 +1,42 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_WINDOWS_PROC_TABLE_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_WINDOWS_PROC_TABLE_H_ + +#include "flutter/fml/native_library.h" + +#include + +namespace flutter { + +// Lookup table for Windows APIs that aren't available on all versions of +// Windows. +class WindowsProcTable { + public: + WindowsProcTable(); + virtual ~WindowsProcTable(); + + // Retrieves the pointer type for a specified pointer. + // + // Used to react differently to touch or pen inputs. Returns zero on failure. + // Available in Windows 8 and newer, otherwise returns zero. + virtual BOOL GetPointerType(UINT32 pointer_id, + POINTER_INPUT_TYPE* pointer_type); + + private: + using GetPointerType_ = BOOL __stdcall(UINT32 pointerId, + POINTER_INPUT_TYPE* pointerType); + + // The User32.dll library, used to resolve functions at runtime. + fml::RefPtr user32_; + + std::optional get_pointer_type_; + + FML_DISALLOW_COPY_AND_ASSIGN(WindowsProcTable); +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_WINDOWS_PROC_TABLE_H_ From 259a1e0d93ca3893d76fa2a185907108a512bb48 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Tue, 16 Aug 2022 16:59:04 -0700 Subject: [PATCH 02/10] License all the things --- ci/licenses_golden/licenses_flutter | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index a4657cb0b7e0f..8307c9f24e829 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2486,6 +2486,8 @@ FILE: ../../../flutter/shell/platform/windows/window_proc_delegate_manager.h FILE: ../../../flutter/shell/platform/windows/window_proc_delegate_manager_unittests.cc FILE: ../../../flutter/shell/platform/windows/window_state.h FILE: ../../../flutter/shell/platform/windows/window_unittests.cc +FILE: ../../../flutter/shell/platform/windows/windows_proc_table.cc +FILE: ../../../flutter/shell/platform/windows/windows_proc_table.h FILE: ../../../flutter/shell/profiling/sampling_profiler.cc FILE: ../../../flutter/shell/profiling/sampling_profiler.h FILE: ../../../flutter/shell/profiling/sampling_profiler_unittest.cc From bab8493510e086e93f86f2698215f42778636aa3 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Tue, 16 Aug 2022 17:04:52 -0700 Subject: [PATCH 03/10] Fix style --- shell/platform/windows/testing/mock_direct_manipulation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/windows/testing/mock_direct_manipulation.h b/shell/platform/windows/testing/mock_direct_manipulation.h index d160f65e006e0..ae007ee727bb2 100644 --- a/shell/platform/windows/testing/mock_direct_manipulation.h +++ b/shell/platform/windows/testing/mock_direct_manipulation.h @@ -18,7 +18,7 @@ class MockDirectManipulationOwner : public DirectManipulationOwner { : DirectManipulationOwner(window){}; virtual ~MockDirectManipulationOwner() = default; - MOCK_METHOD1(SetContact, void(UINT contactId)); + MOCK_METHOD1(SetContact, void(UINT contact_id)); private: FML_DISALLOW_COPY_AND_ASSIGN(MockDirectManipulationOwner); From 4fafb674071412fc3ab83a0f9d86bd681d71a4c3 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 17 Aug 2022 10:49:39 -0700 Subject: [PATCH 04/10] Disallow copy and assign --- shell/platform/windows/direct_manipulation.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/platform/windows/direct_manipulation.h b/shell/platform/windows/direct_manipulation.h index 5bac868dcbc25..40317333f6916 100644 --- a/shell/platform/windows/direct_manipulation.h +++ b/shell/platform/windows/direct_manipulation.h @@ -58,6 +58,8 @@ class DirectManipulationOwner { Microsoft::WRL::ComPtr viewport_; // Child needed for operation of the DirectManipulation API. fml::RefPtr handler_; + + FML_DISALLOW_COPY_AND_ASSIGN(DirectManipulationOwner); }; // Implements DirectManipulation event handling interfaces, receives calls from From 374bfdb96b5a3811a65b844a1d4f9b7451a136ec Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 17 Aug 2022 10:58:50 -0700 Subject: [PATCH 05/10] Address feedback --- shell/platform/windows/windows_proc_table.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/windows/windows_proc_table.h b/shell/platform/windows/windows_proc_table.h index 4a6edae11c9f2..db6397d2e4781 100644 --- a/shell/platform/windows/windows_proc_table.h +++ b/shell/platform/windows/windows_proc_table.h @@ -20,8 +20,8 @@ class WindowsProcTable { // Retrieves the pointer type for a specified pointer. // - // Used to react differently to touch or pen inputs. Returns zero on failure. - // Available in Windows 8 and newer, otherwise returns zero. + // Used to react differently to touch or pen inputs. Returns false on failure. + // Available in Windows 8 and newer, otherwise returns false. virtual BOOL GetPointerType(UINT32 pointer_id, POINTER_INPUT_TYPE* pointer_type); From fb43e52d7e6b62f0429c3bafc16e6ab96a5a0e4d Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 17 Aug 2022 13:44:54 -0700 Subject: [PATCH 06/10] Address feedback --- shell/platform/windows/testing/mock_direct_manipulation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/windows/testing/mock_direct_manipulation.h b/shell/platform/windows/testing/mock_direct_manipulation.h index ae007ee727bb2..be1fe35027f07 100644 --- a/shell/platform/windows/testing/mock_direct_manipulation.h +++ b/shell/platform/windows/testing/mock_direct_manipulation.h @@ -14,7 +14,7 @@ namespace testing { /// Mock for the |DirectManipulationOwner| base class. class MockDirectManipulationOwner : public DirectManipulationOwner { public: - MockDirectManipulationOwner(Window* window) + explicit MockDirectManipulationOwner(Window* window) : DirectManipulationOwner(window){}; virtual ~MockDirectManipulationOwner() = default; From 24b6f6a4dcb5b0685f55dacc40343dfa21edb39b Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 17 Aug 2022 13:52:25 -0700 Subject: [PATCH 07/10] Address feedback --- shell/platform/windows/window_unittests.cc | 69 +++++++++++----------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/shell/platform/windows/window_unittests.cc b/shell/platform/windows/window_unittests.cc index 4546bd54370af..11b41fff3d196 100644 --- a/shell/platform/windows/window_unittests.cc +++ b/shell/platform/windows/window_unittests.cc @@ -285,75 +285,72 @@ TEST(MockWindow, Paint) { } TEST(MockWindow, PointerHitTest) { - auto* windows_proc_table = new MockWindowsProcTable(); - auto text_input_manager = std::make_unique(); - - MockWindow window(std::unique_ptr(windows_proc_table), - std::move(text_input_manager)); - - auto direct_manipulation = new MockDirectManipulationOwner(&window); - window.SetDirectManipulationOwner( - std::unique_ptr(direct_manipulation)); - UINT32 pointer_id = 123; + auto windows_proc_table = std::make_unique(); + auto text_input_manager = std::make_unique(); - EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) + EXPECT_CALL(*windows_proc_table.get(), GetPointerType(Eq(pointer_id), _)) .Times(1) .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { *type = PT_POINTER; return TRUE; }); - EXPECT_CALL(*direct_manipulation, SetContact).Times(0); + MockWindow window(std::move(windows_proc_table), + std::move(text_input_manager)); + auto direct_manipulation = + std::make_unique(&window); + + EXPECT_CALL(*direct_manipulation.get(), SetContact).Times(0); + + window.SetDirectManipulationOwner(std::move(direct_manipulation)); window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); } TEST(MockWindow, TouchPadHitTest) { - auto* windows_proc_table = new MockWindowsProcTable(); - auto text_input_manager = std::make_unique(); - - MockWindow window(std::unique_ptr(windows_proc_table), - std::move(text_input_manager)); - - auto direct_manipulation = new MockDirectManipulationOwner(&window); - window.SetDirectManipulationOwner( - std::unique_ptr(direct_manipulation)); - UINT32 pointer_id = 123; + auto windows_proc_table = std::make_unique(); + auto text_input_manager = std::make_unique(); - EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) + EXPECT_CALL(*windows_proc_table.get(), GetPointerType(Eq(pointer_id), _)) .Times(1) .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { *type = PT_TOUCHPAD; return TRUE; }); - EXPECT_CALL(*direct_manipulation, SetContact(Eq(pointer_id))).Times(1); + MockWindow window(std::move(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = + std::make_unique(&window); + EXPECT_CALL(*direct_manipulation.get(), SetContact(Eq(pointer_id))).Times(1); + + window.SetDirectManipulationOwner(std::move(direct_manipulation)); window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); } TEST(MockWindow, UnknownPointerTypeSkipsDirectManipulation) { - auto* windows_proc_table = new MockWindowsProcTable(); - auto text_input_manager = std::make_unique(); - - MockWindow window(std::unique_ptr(windows_proc_table), - std::move(text_input_manager)); - - auto direct_manipulation = new MockDirectManipulationOwner(&window); - window.SetDirectManipulationOwner( - std::unique_ptr(direct_manipulation)); - UINT32 pointer_id = 123; + auto windows_proc_table = std::make_unique(); + auto text_input_manager = std::make_unique(); - EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) + EXPECT_CALL(*windows_proc_table.get(), GetPointerType(Eq(pointer_id), _)) .Times(1) .WillOnce( [](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { return FALSE; }); - EXPECT_CALL(*direct_manipulation, SetContact).Times(0); + MockWindow window(std::move(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = + std::make_unique(&window); + + EXPECT_CALL(*direct_manipulation.get(), SetContact).Times(0); + window.SetDirectManipulationOwner(std::move(direct_manipulation)); window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); } From 8af42f6f980014c49681497849d22e89fc22ddb0 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 17 Aug 2022 13:54:03 -0700 Subject: [PATCH 08/10] Address feedback --- shell/platform/windows/window_unittests.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/shell/platform/windows/window_unittests.cc b/shell/platform/windows/window_unittests.cc index 11b41fff3d196..520b64bdd526a 100644 --- a/shell/platform/windows/window_unittests.cc +++ b/shell/platform/windows/window_unittests.cc @@ -289,7 +289,7 @@ TEST(MockWindow, PointerHitTest) { auto windows_proc_table = std::make_unique(); auto text_input_manager = std::make_unique(); - EXPECT_CALL(*windows_proc_table.get(), GetPointerType(Eq(pointer_id), _)) + EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) .Times(1) .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { *type = PT_POINTER; @@ -302,7 +302,7 @@ TEST(MockWindow, PointerHitTest) { auto direct_manipulation = std::make_unique(&window); - EXPECT_CALL(*direct_manipulation.get(), SetContact).Times(0); + EXPECT_CALL(*direct_manipulation, SetContact).Times(0); window.SetDirectManipulationOwner(std::move(direct_manipulation)); window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); @@ -313,7 +313,7 @@ TEST(MockWindow, TouchPadHitTest) { auto windows_proc_table = std::make_unique(); auto text_input_manager = std::make_unique(); - EXPECT_CALL(*windows_proc_table.get(), GetPointerType(Eq(pointer_id), _)) + EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) .Times(1) .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { *type = PT_TOUCHPAD; @@ -326,7 +326,7 @@ TEST(MockWindow, TouchPadHitTest) { auto direct_manipulation = std::make_unique(&window); - EXPECT_CALL(*direct_manipulation.get(), SetContact(Eq(pointer_id))).Times(1); + EXPECT_CALL(*direct_manipulation, SetContact(Eq(pointer_id))).Times(1); window.SetDirectManipulationOwner(std::move(direct_manipulation)); window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); @@ -337,7 +337,7 @@ TEST(MockWindow, UnknownPointerTypeSkipsDirectManipulation) { auto windows_proc_table = std::make_unique(); auto text_input_manager = std::make_unique(); - EXPECT_CALL(*windows_proc_table.get(), GetPointerType(Eq(pointer_id), _)) + EXPECT_CALL(*windows_proc_table, GetPointerType(Eq(pointer_id), _)) .Times(1) .WillOnce( [](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { return FALSE; }); @@ -348,7 +348,7 @@ TEST(MockWindow, UnknownPointerTypeSkipsDirectManipulation) { auto direct_manipulation = std::make_unique(&window); - EXPECT_CALL(*direct_manipulation.get(), SetContact).Times(0); + EXPECT_CALL(*direct_manipulation, SetContact).Times(0); window.SetDirectManipulationOwner(std::move(direct_manipulation)); window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); From cd94fcfbe517fa40fbe750b126a1a1d3409d29d6 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 17 Aug 2022 13:58:19 -0700 Subject: [PATCH 09/10] Add test comments --- shell/platform/windows/window_unittests.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/shell/platform/windows/window_unittests.cc b/shell/platform/windows/window_unittests.cc index 520b64bdd526a..ebd8658952362 100644 --- a/shell/platform/windows/window_unittests.cc +++ b/shell/platform/windows/window_unittests.cc @@ -284,6 +284,7 @@ TEST(MockWindow, Paint) { window.InjectWindowMessage(WM_PAINT, 0, 0); } +// Verify direct manipulation isn't notified of pointer hit tests. TEST(MockWindow, PointerHitTest) { UINT32 pointer_id = 123; auto windows_proc_table = std::make_unique(); @@ -308,6 +309,7 @@ TEST(MockWindow, PointerHitTest) { window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); } +// Verify direct manipulation is notified of touchpad hit tests. TEST(MockWindow, TouchPadHitTest) { UINT32 pointer_id = 123; auto windows_proc_table = std::make_unique(); @@ -332,6 +334,9 @@ TEST(MockWindow, TouchPadHitTest) { window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); } +// Verify direct manipulation isn't notified of unknown hit tests. +// This can happen if determining the pointer type fails, for example, +// if GetPointerType is unsupported by the current Windows version. TEST(MockWindow, UnknownPointerTypeSkipsDirectManipulation) { UINT32 pointer_id = 123; auto windows_proc_table = std::make_unique(); From f94a856b0bcda83467cd1f9c6db7cc66643ac988 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 17 Aug 2022 13:59:04 -0700 Subject: [PATCH 10/10] Tag crash --- shell/platform/windows/window_unittests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/windows/window_unittests.cc b/shell/platform/windows/window_unittests.cc index ebd8658952362..1557d03b7a6a3 100644 --- a/shell/platform/windows/window_unittests.cc +++ b/shell/platform/windows/window_unittests.cc @@ -337,6 +337,7 @@ TEST(MockWindow, TouchPadHitTest) { // Verify direct manipulation isn't notified of unknown hit tests. // This can happen if determining the pointer type fails, for example, // if GetPointerType is unsupported by the current Windows version. +// See: https://github.com/flutter/flutter/issues/109412 TEST(MockWindow, UnknownPointerTypeSkipsDirectManipulation) { UINT32 pointer_id = 123; auto windows_proc_table = std::make_unique();