From b454251f03f27ff422b2c197761493a9e3133909 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Mon, 13 Jan 2020 19:04:17 -0800 Subject: [PATCH] Implement HWND access for Windows plugins (#15378) plugin_registrar_windows.h was never fully updated for the Win32 switch, and didn't actually compile. This introduces FlutterView (parallel to the GLFW wrapper's FlutterWindow) as a ways of holding view-specific functionality to expose via the plugin registrar, and moves HWND access from the FlutterViewController to the FlutterView so that it's available to plugins. This allows the implementation of plugins that need access to the native HWND (e.g., moving or resizing the top-level window). Adds simple unit tests of the new wrapper functionality, ensuring that the files actually compile, and that the passthroughs work as expected. This is a breaking change for Windows runners due to moving GetNativeWindow() in the wrapper. It's not being done as a multi-stage change (addition + deprecation + later removal) since this API is explicitly unstable. --- ci/licenses_golden/licenses_flutter | 3 ++ .../platform/windows/client_wrapper/BUILD.gn | 4 ++- .../client_wrapper/flutter_view_controller.cc | 8 ++--- .../flutter_view_controller_unittests.cc | 26 ++++++++++++--- .../client_wrapper/flutter_view_unittests.cc | 31 +++++++++++++++++ .../include/flutter/flutter_view.h | 33 +++++++++++++++++++ .../include/flutter/flutter_view_controller.h | 7 ++-- .../flutter/plugin_registrar_windows.h | 14 ++++---- .../plugin_registrar_windows_unittests.cc | 30 +++++++++++++++++ .../testing/stub_flutter_windows_api.cc | 24 ++++++++++---- .../testing/stub_flutter_windows_api.h | 8 ++--- shell/platform/windows/flutter_windows.cc | 18 +++++++--- .../platform/windows/public/flutter_windows.h | 14 ++++++-- 13 files changed, 184 insertions(+), 36 deletions(-) create mode 100644 shell/platform/windows/client_wrapper/flutter_view_unittests.cc create mode 100644 shell/platform/windows/client_wrapper/include/flutter/flutter_view.h create mode 100644 shell/platform/windows/client_wrapper/plugin_registrar_windows_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 07534cf8a..671a5a51c 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1093,8 +1093,11 @@ FILE: ../../../flutter/shell/platform/windows/angle_surface_manager.cc FILE: ../../../flutter/shell/platform/windows/angle_surface_manager.h FILE: ../../../flutter/shell/platform/windows/client_wrapper/flutter_view_controller.cc FILE: ../../../flutter/shell/platform/windows/client_wrapper/flutter_view_controller_unittests.cc +FILE: ../../../flutter/shell/platform/windows/client_wrapper/flutter_view_unittests.cc +FILE: ../../../flutter/shell/platform/windows/client_wrapper/include/flutter/flutter_view.h FILE: ../../../flutter/shell/platform/windows/client_wrapper/include/flutter/flutter_view_controller.h FILE: ../../../flutter/shell/platform/windows/client_wrapper/include/flutter/plugin_registrar_windows.h +FILE: ../../../flutter/shell/platform/windows/client_wrapper/plugin_registrar_windows_unittests.cc FILE: ../../../flutter/shell/platform/windows/flutter_windows.cc FILE: ../../../flutter/shell/platform/windows/key_event_handler.cc FILE: ../../../flutter/shell/platform/windows/key_event_handler.h diff --git a/shell/platform/windows/client_wrapper/BUILD.gn b/shell/platform/windows/client_wrapper/BUILD.gn index 25f0f857f..e24b98e2a 100644 --- a/shell/platform/windows/client_wrapper/BUILD.gn +++ b/shell/platform/windows/client_wrapper/BUILD.gn @@ -7,6 +7,7 @@ import("$flutter_root/testing/testing.gni") _wrapper_includes = [ "include/flutter/flutter_view_controller.h", + "include/flutter/flutter_view.h", "include/flutter/plugin_registrar_windows.h", ] @@ -69,9 +70,10 @@ test_fixtures("client_wrapper_windows_fixtures") { executable("client_wrapper_windows_unittests") { testonly = true - # TODO: Add more unit tests. sources = [ "flutter_view_controller_unittests.cc", + "flutter_view_unittests.cc", + "plugin_registrar_windows_unittests.cc", ] deps = [ diff --git a/shell/platform/windows/client_wrapper/flutter_view_controller.cc b/shell/platform/windows/client_wrapper/flutter_view_controller.cc index 210524705..92a853862 100644 --- a/shell/platform/windows/client_wrapper/flutter_view_controller.cc +++ b/shell/platform/windows/client_wrapper/flutter_view_controller.cc @@ -30,8 +30,10 @@ FlutterViewController::FlutterViewController( width, height, assets_path.c_str(), icu_data_path_.c_str(), arg_count > 0 ? &engine_arguments[0] : nullptr, arg_count); if (!controller_) { - std::cerr << "Failed to create view." << std::endl; + std::cerr << "Failed to create view controller." << std::endl; + return; } + view_ = std::make_unique(FlutterDesktopGetView(controller_)); } FlutterViewController::~FlutterViewController() { @@ -40,10 +42,6 @@ FlutterViewController::~FlutterViewController() { } } -HWND FlutterViewController::GetNativeWindow() { - return FlutterDesktopGetHWND(controller_); -} - std::chrono::nanoseconds FlutterViewController::ProcessMessages() { return std::chrono::nanoseconds(FlutterDesktopProcessMessages(controller_)); } diff --git a/shell/platform/windows/client_wrapper/flutter_view_controller_unittests.cc b/shell/platform/windows/client_wrapper/flutter_view_controller_unittests.cc index e0c2d532b..1ebd9683c 100644 --- a/shell/platform/windows/client_wrapper/flutter_view_controller_unittests.cc +++ b/shell/platform/windows/client_wrapper/flutter_view_controller_unittests.cc @@ -14,20 +14,38 @@ namespace flutter { namespace { // Stub implementation to validate calls to the API. -class TestWindowsApi : public testing::StubFlutterWindowsApi {}; +class TestWindowsApi : public testing::StubFlutterWindowsApi { + FlutterDesktopViewControllerRef CreateViewController( + int initial_width, + int initial_height, + const char* assets_path, + const char* icu_data_path, + const char** arguments, + size_t argument_count) override { + return reinterpret_cast(1); + } +}; } // namespace TEST(FlutterViewControllerTest, CreateDestroy) { - const std::string icu_data_path = "fake/path/to/icu"; testing::ScopedStubFlutterWindowsApi scoped_api_stub( std::make_unique()); auto test_api = static_cast(scoped_api_stub.stub()); { - FlutterViewController controller(icu_data_path, 100, 100, - std::string("fake"), + FlutterViewController controller("", 100, 100, "", std::vector{}); } } +TEST(FlutterViewControllerTest, GetView) { + std::string icu_data_path = "fake_path_to_icu"; + testing::ScopedStubFlutterWindowsApi scoped_api_stub( + std::make_unique()); + auto test_api = static_cast(scoped_api_stub.stub()); + FlutterViewController controller("", 100, 100, "", + std::vector{}); + EXPECT_NE(controller.view(), nullptr); +} + } // namespace flutter diff --git a/shell/platform/windows/client_wrapper/flutter_view_unittests.cc b/shell/platform/windows/client_wrapper/flutter_view_unittests.cc new file mode 100644 index 000000000..6fbe2981a --- /dev/null +++ b/shell/platform/windows/client_wrapper/flutter_view_unittests.cc @@ -0,0 +1,31 @@ +// 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 +#include + +#include "flutter/shell/platform/windows/client_wrapper/include/flutter/flutter_view.h" +#include "flutter/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h" +#include "gtest/gtest.h" + +namespace flutter { + +namespace { + +// Stub implementation to validate calls to the API. +class TestWindowsApi : public testing::StubFlutterWindowsApi { + HWND ViewGetHWND() override { return reinterpret_cast(7); } +}; + +} // namespace + +TEST(FlutterViewTest, HwndAccessPassesThrough) { + testing::ScopedStubFlutterWindowsApi scoped_api_stub( + std::make_unique()); + auto test_api = static_cast(scoped_api_stub.stub()); + FlutterView view(reinterpret_cast(2)); + EXPECT_EQ(view.GetNativeWindow(), reinterpret_cast(7)); +} + +} // namespace flutter diff --git a/shell/platform/windows/client_wrapper/include/flutter/flutter_view.h b/shell/platform/windows/client_wrapper/include/flutter/flutter_view.h new file mode 100644 index 000000000..18c908e97 --- /dev/null +++ b/shell/platform/windows/client_wrapper/include/flutter/flutter_view.h @@ -0,0 +1,33 @@ +// 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_CLIENT_WRAPPER_INCLUDE_FLUTTER_FLUTTER_VIEW_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_CLIENT_WRAPPER_INCLUDE_FLUTTER_FLUTTER_VIEW_H_ + +#include + +namespace flutter { + +// A view displaying Flutter content. +class FlutterView { + public: + explicit FlutterView(FlutterDesktopViewRef view) : view_(view) {} + + virtual ~FlutterView() = default; + + // Prevent copying. + FlutterView(FlutterView const&) = delete; + FlutterView& operator=(FlutterView const&) = delete; + + // Returns the backing HWND for the view. + HWND GetNativeWindow() { return FlutterDesktopViewGetHWND(view_); } + + private: + // Handle for interacting with the C API's view. + FlutterDesktopViewRef view_ = nullptr; +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_CLIENT_WRAPPER_INCLUDE_FLUTTER_FLUTTER_VIEW_H_ diff --git a/shell/platform/windows/client_wrapper/include/flutter/flutter_view_controller.h b/shell/platform/windows/client_wrapper/include/flutter/flutter_view_controller.h index 80299bc69..c72b28522 100644 --- a/shell/platform/windows/client_wrapper/include/flutter/flutter_view_controller.h +++ b/shell/platform/windows/client_wrapper/include/flutter/flutter_view_controller.h @@ -11,6 +11,7 @@ #include #include +#include "flutter_view.h" #include "plugin_registrar.h" #include "plugin_registry.h" @@ -49,8 +50,7 @@ class FlutterViewController : public PluginRegistry { FlutterViewController(FlutterViewController const&) = delete; FlutterViewController& operator=(FlutterViewController const&) = delete; - // Return backing HWND for manipulation in host application. - HWND GetNativeWindow(); + FlutterView* view() { return view_.get(); } // Processes any pending events in the Flutter engine, and returns the // nanosecond delay until the next scheduled event (or max, if none). @@ -71,6 +71,9 @@ class FlutterViewController : public PluginRegistry { // Handle for interacting with the C API's view controller, if any. FlutterDesktopViewControllerRef controller_ = nullptr; + + // The owned FlutterView. + std::unique_ptr view_; }; } // namespace flutter diff --git a/shell/platform/windows/client_wrapper/include/flutter/plugin_registrar_windows.h b/shell/platform/windows/client_wrapper/include/flutter/plugin_registrar_windows.h index 0e7e60fc1..052f22bbc 100644 --- a/shell/platform/windows/client_wrapper/include/flutter/plugin_registrar_windows.h +++ b/shell/platform/windows/client_wrapper/include/flutter/plugin_registrar_windows.h @@ -9,12 +9,12 @@ #include -#include "flutter_window.h" +#include "flutter_view.h" #include "plugin_registrar.h" namespace flutter { -// An extension to PluginRegistrar providing access to windows-shell-specific +// An extension to PluginRegistrar providing access to Windows-specific // functionality. class PluginRegistrarWindows : public PluginRegistrar { public: @@ -23,8 +23,8 @@ class PluginRegistrarWindows : public PluginRegistrar { explicit PluginRegistrarWindows( FlutterDesktopPluginRegistrarRef core_registrar) : PluginRegistrar(core_registrar) { - window_ = std::make_unique( - FlutterDesktopRegistrarGetWindow(core_registrar)); + view_ = std::make_unique( + FlutterDesktopRegistrarGetView(core_registrar)); } virtual ~PluginRegistrarWindows() = default; @@ -33,11 +33,11 @@ class PluginRegistrarWindows : public PluginRegistrar { PluginRegistrarWindows(PluginRegistrarWindows const&) = delete; PluginRegistrarWindows& operator=(PluginRegistrarWindows const&) = delete; - FlutterWindow* window() { return window_.get(); } + FlutterView* GetView() { return view_.get(); } private: - // The owned FlutterWindow, if any. - std::unique_ptr window_; + // The associated FlutterView, if any. + std::unique_ptr view_; }; } // namespace flutter diff --git a/shell/platform/windows/client_wrapper/plugin_registrar_windows_unittests.cc b/shell/platform/windows/client_wrapper/plugin_registrar_windows_unittests.cc new file mode 100644 index 000000000..07eaff0c1 --- /dev/null +++ b/shell/platform/windows/client_wrapper/plugin_registrar_windows_unittests.cc @@ -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. + +#include +#include + +#include "flutter/shell/platform/windows/client_wrapper/include/flutter/plugin_registrar_windows.h" +#include "flutter/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h" +#include "gtest/gtest.h" + +namespace flutter { + +namespace { + +// Stub implementation to validate calls to the API. +class TestWindowsApi : public testing::StubFlutterWindowsApi {}; + +} // namespace + +TEST(PluginRegistrarWindowsTest, GetView) { + testing::ScopedStubFlutterWindowsApi scoped_api_stub( + std::make_unique()); + auto test_api = static_cast(scoped_api_stub.stub()); + PluginRegistrarWindows registrar( + reinterpret_cast(1)); + EXPECT_NE(registrar.GetView(), nullptr); +} + +} // namespace flutter diff --git a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc index d3367cc19..07dfb8bd7 100644 --- a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc +++ b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc @@ -57,21 +57,27 @@ void FlutterDesktopDestroyViewController( } } -HWND FlutterDesktopGetHWND(FlutterDesktopViewControllerRef controller) { - if (s_stub_implementation) { - return s_stub_implementation->FlutterDesktopGetHWND(); - } - return reinterpret_cast(-1); +FlutterDesktopViewRef FlutterDesktopGetView( + FlutterDesktopViewControllerRef controller) { + // The stub ignores this, so just return an arbitrary non-zero value. + return reinterpret_cast(1); } uint64_t FlutterDesktopProcessMessages( FlutterDesktopViewControllerRef controller) { if (s_stub_implementation) { - return s_stub_implementation->FlutterDesktopProcessMessages(); + return s_stub_implementation->ProcessMessages(); } return 0; } +HWND FlutterDesktopViewGetHWND(FlutterDesktopViewRef controller) { + if (s_stub_implementation) { + return s_stub_implementation->ViewGetHWND(); + } + return reinterpret_cast(-1); +} + FlutterDesktopEngineRef FlutterDesktopRunEngine(const char* assets_path, const char* icu_data_path, const char** arguments, @@ -96,3 +102,9 @@ FlutterDesktopPluginRegistrarRef FlutterDesktopGetPluginRegistrar( // The stub ignores this, so just return an arbitrary non-zero value. return reinterpret_cast(1); } + +FlutterDesktopViewRef FlutterDesktopRegistrarGetView( + FlutterDesktopPluginRegistrarRef controller) { + // The stub ignores this, so just return an arbitrary non-zero value. + return reinterpret_cast(1); +} diff --git a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h index 640dcabe9..0fd74c5b2 100644 --- a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h +++ b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h @@ -43,11 +43,11 @@ class StubFlutterWindowsApi { // Called for FlutterDesktopDestroyView. virtual void DestroyViewController() {} - // Called for FlutterDesktopProcessMessages - virtual uint64_t FlutterDesktopProcessMessages() { return 0; } + // Called for FlutterDesktopProcessMessages. + virtual uint64_t ProcessMessages() { return 0; } - // Called for FlutterDesktopProcessMessages - virtual HWND FlutterDesktopGetHWND() { return reinterpret_cast(1); } + // Called for FlutterDesktopViewGetHWND. + virtual HWND ViewGetHWND() { return reinterpret_cast(1); } // Called for FlutterDesktopRunEngine. virtual FlutterDesktopEngineRef RunEngine(const char* assets_path, diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index c210898e9..67ab22a5c 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -157,10 +157,6 @@ uint64_t FlutterDesktopProcessMessages( return controller->engine_state->task_runner->ProcessTasks().count(); } -HWND FlutterDesktopGetHWND(FlutterDesktopViewControllerRef controller) { - return controller->view->GetWindowHandle(); -} - void FlutterDesktopDestroyViewController( FlutterDesktopViewControllerRef controller) { FlutterEngineShutdown(controller->engine_state->engine); @@ -177,6 +173,15 @@ FlutterDesktopPluginRegistrarRef FlutterDesktopGetPluginRegistrar( return controller->view->GetRegistrar(); } +FlutterDesktopViewRef FlutterDesktopGetView( + FlutterDesktopViewControllerRef controller) { + return controller->view_wrapper.get(); +} + +HWND FlutterDesktopViewGetHWND(FlutterDesktopViewRef view) { + return view->window->GetWindowHandle(); +} + FlutterDesktopEngineRef FlutterDesktopRunEngine(const char* assets_path, const char* icu_data_path, const char** arguments, @@ -204,6 +209,11 @@ FlutterDesktopMessengerRef FlutterDesktopRegistrarGetMessenger( return registrar->messenger.get(); } +FlutterDesktopViewRef FlutterDesktopRegistrarGetView( + FlutterDesktopPluginRegistrarRef registrar) { + return registrar->window; +} + bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger, const char* channel, const uint8_t* message, diff --git a/shell/platform/windows/public/flutter_windows.h b/shell/platform/windows/public/flutter_windows.h index 7d3798d87..66c8287ab 100644 --- a/shell/platform/windows/public/flutter_windows.h +++ b/shell/platform/windows/public/flutter_windows.h @@ -61,9 +61,9 @@ FLUTTER_EXPORT FlutterDesktopPluginRegistrarRef FlutterDesktopGetPluginRegistrar(FlutterDesktopViewControllerRef controller, const char* plugin_name); -// Return backing HWND for manipulation in host application. -FLUTTER_EXPORT HWND -FlutterDesktopGetHWND(FlutterDesktopViewControllerRef controller); +// Returns the view managed by the given controller. +FLUTTER_EXPORT FlutterDesktopViewRef +FlutterDesktopGetView(FlutterDesktopViewControllerRef controller); // Processes any pending events in the Flutter engine, and returns the // number of nanoseconds until the next scheduled event (or max, if none). @@ -74,6 +74,9 @@ FlutterDesktopGetHWND(FlutterDesktopViewControllerRef controller); FLUTTER_EXPORT uint64_t FlutterDesktopProcessMessages(FlutterDesktopViewControllerRef controller); +// Return backing HWND for manipulation in host application. +FLUTTER_EXPORT HWND FlutterDesktopViewGetHWND(FlutterDesktopViewRef view); + // Runs an instance of a headless Flutter engine. // // The |assets_path| is the path to the flutter_assets folder for the Flutter @@ -96,6 +99,11 @@ FlutterDesktopRunEngine(const char* assets_path, FLUTTER_EXPORT bool FlutterDesktopShutDownEngine( FlutterDesktopEngineRef engine_ref); +// Returns the view associated with this registrar's engine instance +// This is a Windows-specific extension to flutter_plugin_registrar.h. +FLUTTER_EXPORT FlutterDesktopViewRef +FlutterDesktopRegistrarGetView(FlutterDesktopPluginRegistrarRef registrar); + #if defined(__cplusplus) } // extern "C" #endif -- GitLab