From 4141570d5409dab5f3d79f02536f428526d1075f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= <737941+loic-sharma@users.noreply.github.com> Date: Wed, 17 Aug 2022 14:50:46 -0700 Subject: [PATCH] [Windows] Don't crash if GetPointerType is unsupported (#35442) --- ci/licenses_golden/licenses_flutter | 2 + shell/platform/windows/BUILD.gn | 4 + shell/platform/windows/direct_manipulation.h | 5 +- .../testing/mock_direct_manipulation.h | 30 +++++ .../windows/testing/mock_window_win32.cc | 9 +- .../windows/testing/mock_window_win32.h | 7 +- .../windows/testing/mock_windows_proc_table.h | 30 +++++ shell/platform/windows/window_win32.cc | 18 ++- shell/platform/windows/window_win32.h | 8 +- .../windows/window_win32_unittests.cc | 108 ++++++++++++++++-- shell/platform/windows/windows_proc_table.cc | 28 +++++ shell/platform/windows/windows_proc_table.h | 42 +++++++ 12 files changed, 269 insertions(+), 22 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/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 5ce695d846cc3..677e78b464ba5 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2397,6 +2397,8 @@ FILE: ../../../flutter/shell/platform/windows/window_state.h FILE: ../../../flutter/shell/platform/windows/window_win32.cc FILE: ../../../flutter/shell/platform/windows/window_win32.h FILE: ../../../flutter/shell/platform/windows/window_win32_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 diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index 8402f1471d4c5..13077b7c12481 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -110,6 +110,8 @@ source_set("flutter_windows_source") { "window_state.h", "window_win32.cc", "window_win32.h", + "windows_proc_table.cc", + "windows_proc_table.h", ] libs = [ @@ -195,6 +197,7 @@ executable("flutter_windows_unittests") { "testing/engine_modifier.h", "testing/flutter_window_win32_test.cc", "testing/flutter_window_win32_test.h", + "testing/mock_direct_manipulation.h", "testing/mock_gl_functions.h", "testing/mock_text_input_manager_win32.cc", "testing/mock_text_input_manager_win32.h", @@ -202,6 +205,7 @@ executable("flutter_windows_unittests") { "testing/mock_window_binding_handler.h", "testing/mock_window_win32.cc", "testing/mock_window_win32.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 bca747173e4f8..26d546d898347 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(WindowWin32* 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(); @@ -57,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 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..1631a0875ae65 --- /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: + explicit MockDirectManipulationOwner(WindowWin32* window) + : DirectManipulationOwner(window){}; + virtual ~MockDirectManipulationOwner() = default; + + MOCK_METHOD1(SetContact, void(UINT contact_id)); + + 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_win32.cc b/shell/platform/windows/testing/mock_window_win32.cc index a44bc0972a4c2..12e64e7f48a2e 100644 --- a/shell/platform/windows/testing/mock_window_win32.cc +++ b/shell/platform/windows/testing/mock_window_win32.cc @@ -8,8 +8,10 @@ namespace flutter { namespace testing { MockWin32Window::MockWin32Window() : WindowWin32(){}; MockWin32Window::MockWin32Window( + std::unique_ptr window_proc_table, std::unique_ptr text_input_manager) - : WindowWin32(std::move(text_input_manager)){}; + : WindowWin32(std::move(window_proc_table), + std::move(text_input_manager)){}; MockWin32Window::~MockWin32Window() = default; @@ -24,6 +26,11 @@ LRESULT MockWin32Window::Win32DefWindowProc(HWND hWnd, return kWmResultDefault; } +void MockWin32Window::SetDirectManipulationOwner( + std::unique_ptr owner) { + direct_manipulation_owner_ = std::move(owner); +} + LRESULT MockWin32Window::InjectWindowMessage(UINT const message, WPARAM const wparam, LPARAM const lparam) { diff --git a/shell/platform/windows/testing/mock_window_win32.h b/shell/platform/windows/testing/mock_window_win32.h index cfd7bac65c3ff..0278e6d3e15ba 100644 --- a/shell/platform/windows/testing/mock_window_win32.h +++ b/shell/platform/windows/testing/mock_window_win32.h @@ -18,7 +18,8 @@ namespace testing { class MockWin32Window : public WindowWin32 { public: MockWin32Window(); - MockWin32Window(std::unique_ptr text_input_manager); + MockWin32Window(std::unique_ptr windows_proc_table, + std::unique_ptr text_input_manager); virtual ~MockWin32Window(); // Prevent copying. @@ -28,6 +29,10 @@ class MockWin32Window : public WindowWin32 { // 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_win32.cc b/shell/platform/windows/window_win32.cc index e05063346135d..f45457ca72c10 100644 --- a/shell/platform/windows/window_win32.cc +++ b/shell/platform/windows/window_win32.cc @@ -51,16 +51,22 @@ static const int kMaxTouchDeviceId = 128; } // namespace -WindowWin32::WindowWin32() : WindowWin32(nullptr) {} +WindowWin32::WindowWin32() : WindowWin32(nullptr, nullptr) {} WindowWin32::WindowWin32( + 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 // kWmDpiChangedBeforeParent message. current_dpi_ = GetDpiForHWND(nullptr); + + if (windows_proc_table_ == nullptr) { + windows_proc_table_ = std::make_unique(); + } if (text_input_manager_ == nullptr) { text_input_manager_ = std::make_unique(); } @@ -473,11 +479,11 @@ WindowWin32::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_win32.h b/shell/platform/windows/window_win32.h index cc142e699468e..f86320722b212 100644 --- a/shell/platform/windows/window_win32.h +++ b/shell/platform/windows/window_win32.h @@ -18,6 +18,7 @@ #include "flutter/shell/platform/windows/keyboard_manager_win32.h" #include "flutter/shell/platform/windows/sequential_id_generator.h" #include "flutter/shell/platform/windows/text_input_manager_win32.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 WindowWin32 : public KeyboardManagerWin32::WindowDelegate { public: WindowWin32(); - WindowWin32(std::unique_ptr text_input_manager); + WindowWin32(std::unique_ptr windows_proc_table, + std::unique_ptr text_input_manager); virtual ~WindowWin32(); // Initializes as a child window with size using |width| and |height| and @@ -254,6 +256,10 @@ class WindowWin32 : public KeyboardManagerWin32::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_win32_unittests.cc b/shell/platform/windows/window_win32_unittests.cc index 9f8f010a33217..3bb59bc670837 100644 --- a/shell/platform/windows/window_win32_unittests.cc +++ b/shell/platform/windows/window_win32_unittests.cc @@ -2,11 +2,15 @@ // 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/testing/mock_direct_manipulation.h" #include "flutter/shell/platform/windows/testing/mock_text_input_manager_win32.h" #include "flutter/shell/platform/windows/testing/mock_window_win32.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; @@ -37,11 +41,12 @@ TEST(MockWin32Window, VerticalScroll) { } TEST(MockWin32Window, OnImeCompositionCompose) { - MockTextInputManagerWin32* text_input_manager = - new MockTextInputManagerWin32(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManagerWin32(); std::unique_ptr text_input_manager_ptr( text_input_manager); - MockWin32Window window(std::move(text_input_manager_ptr)); + MockWin32Window 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,11 +68,12 @@ TEST(MockWin32Window, OnImeCompositionCompose) { } TEST(MockWin32Window, OnImeCompositionResult) { - MockTextInputManagerWin32* text_input_manager = - new MockTextInputManagerWin32(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManagerWin32(); std::unique_ptr text_input_manager_ptr( text_input_manager); - MockWin32Window window(std::move(text_input_manager_ptr)); + MockWin32Window 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")))); @@ -89,11 +95,12 @@ TEST(MockWin32Window, OnImeCompositionResult) { } TEST(MockWin32Window, OnImeCompositionResultAndCompose) { - MockTextInputManagerWin32* text_input_manager = - new MockTextInputManagerWin32(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManagerWin32(); std::unique_ptr text_input_manager_ptr( text_input_manager); - MockWin32Window window(std::move(text_input_manager_ptr)); + MockWin32Window 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 "は". @@ -127,11 +134,12 @@ TEST(MockWin32Window, OnImeCompositionResultAndCompose) { } TEST(MockWin32Window, OnImeCompositionClearChange) { - MockTextInputManagerWin32* text_input_manager = - new MockTextInputManagerWin32(); + auto windows_proc_table = std::make_unique(); + auto* text_input_manager = new MockTextInputManagerWin32(); std::unique_ptr text_input_manager_ptr( text_input_manager); - MockWin32Window window(std::move(text_input_manager_ptr)); + MockWin32Window 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) @@ -273,5 +281,81 @@ TEST(MockWin32Window, KeyDownWithCtrlToggled) { SetKeyboardState(keyboard_state); } +// Verify direct manipulation isn't notified of pointer hit tests. +TEST(MockWin32Window, PointerHitTest) { + 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), _)) + .Times(1) + .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { + *type = PT_POINTER; + return TRUE; + }); + + MockWin32Window window(std::move(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = + std::make_unique(&window); + + EXPECT_CALL(*direct_manipulation, SetContact).Times(0); + + window.SetDirectManipulationOwner(std::move(direct_manipulation)); + window.InjectWindowMessage(DM_POINTERHITTEST, MAKEWPARAM(pointer_id, 0), 0); +} + +// Verify direct manipulation is notified of touchpad hit tests. +TEST(MockWin32Window, TouchPadHitTest) { + 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), _)) + .Times(1) + .WillOnce([](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { + *type = PT_TOUCHPAD; + return TRUE; + }); + + MockWin32Window window(std::move(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = + std::make_unique(&window); + + 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); +} + +// 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(MockWin32Window, UnknownPointerTypeSkipsDirectManipulation) { + 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), _)) + .Times(1) + .WillOnce( + [](UINT32 pointer_id, POINTER_INPUT_TYPE* type) { return FALSE; }); + + MockWin32Window window(std::move(windows_proc_table), + std::move(text_input_manager)); + + auto direct_manipulation = + std::make_unique(&window); + + EXPECT_CALL(*direct_manipulation, SetContact).Times(0); + + window.SetDirectManipulationOwner(std::move(direct_manipulation)); + 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..db6397d2e4781 --- /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 false on failure. + // Available in Windows 8 and newer, otherwise returns false. + 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_