From cdcad7f93e15022fbf57c2c1dbc258efdea53858 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 20 Mar 2024 16:49:16 -0700 Subject: [PATCH 1/2] [Windows] Allow view controllers to not own the engine --- shell/platform/windows/flutter_windows.cc | 28 ++++++++++++++++--- .../windows/flutter_windows_internal.h | 26 +++++++++++++++++ .../windows/flutter_windows_unittests.cc | 21 ++++++++++++++ .../windows/flutter_windows_view_controller.h | 9 +++++- .../testing/windows_test_config_builder.cc | 2 ++ .../testing/windows_test_config_builder.h | 2 +- 6 files changed, 82 insertions(+), 6 deletions(-) diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index 2629b42f150c0..af8afbea2785b 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -72,18 +72,24 @@ static FlutterDesktopTextureRegistrarRef HandleForTextureRegistrar( return reinterpret_cast(registrar); } -FlutterDesktopViewControllerRef FlutterDesktopViewControllerCreate( +// Creates a view controller that might own the engine. +static FlutterDesktopViewControllerRef CreateViewController( + FlutterDesktopEngineRef engine_ref, int width, int height, - FlutterDesktopEngineRef engine_ref) { + bool owns_engine) { flutter::FlutterWindowsEngine* engine_ptr = EngineFromHandle(engine_ref); std::unique_ptr window_wrapper = std::make_unique( width, height, engine_ptr->windows_proc_table()); - auto engine = std::unique_ptr(engine_ptr); + std::unique_ptr engine; + if (owns_engine) { + engine = std::unique_ptr(engine_ptr); + } + std::unique_ptr view = - engine->CreateView(std::move(window_wrapper)); + engine_ptr->CreateView(std::move(window_wrapper)); auto controller = std::make_unique( std::move(engine), std::move(view)); @@ -105,6 +111,20 @@ FlutterDesktopViewControllerRef FlutterDesktopViewControllerCreate( return HandleForViewController(controller.release()); } +FlutterDesktopViewControllerRef FlutterDesktopViewControllerCreate( + int width, + int height, + FlutterDesktopEngineRef engine) { + return CreateViewController(engine, width, height, /*owns_engine=*/true); +} + +FlutterDesktopViewControllerRef FlutterDesktopEngineCreateViewController( + FlutterDesktopEngineRef engine, + const FlutterDesktopViewControllerProperties* properties) { + return CreateViewController(engine, properties->width, properties->height, + /*owns_engine=*/false); +} + void FlutterDesktopViewControllerDestroy(FlutterDesktopViewControllerRef ref) { auto controller = ViewControllerFromHandle(ref); delete controller; diff --git a/shell/platform/windows/flutter_windows_internal.h b/shell/platform/windows/flutter_windows_internal.h index 47b98e983a48a..d06c2155c011e 100644 --- a/shell/platform/windows/flutter_windows_internal.h +++ b/shell/platform/windows/flutter_windows_internal.h @@ -14,6 +14,32 @@ extern "C" { // Declare functions that are currently in-progress and shall be exposed to the // public facing API upon completion. +// Properties for configuring a Flutter view controller. +typedef struct { + // The view's initial width. + int width; + + // The view's initial height. + int height; + +} FlutterDesktopViewControllerProperties; + +// Creates a view for the given engine. +// +// The |engine| will be started if it is not already running. +// +// The caller owns the returned reference, and is responsible for calling +// |FlutterDesktopViewControllerDestroy|. Returns a null pointer in the event of +// an error. +// +// Unlike |FlutterDesktopViewControllerCreate|, this does *not* take ownership +// of |engine| and |FlutterDesktopEngineDestroy| must be called to destroy +// the engine. +FLUTTER_EXPORT FlutterDesktopViewControllerRef +FlutterDesktopEngineCreateViewController( + FlutterDesktopEngineRef engine, + const FlutterDesktopViewControllerProperties* properties); + typedef int64_t PlatformViewId; typedef struct { diff --git a/shell/platform/windows/flutter_windows_unittests.cc b/shell/platform/windows/flutter_windows_unittests.cc index 58b5b50b1db6e..37fbbcabd5c24 100644 --- a/shell/platform/windows/flutter_windows_unittests.cc +++ b/shell/platform/windows/flutter_windows_unittests.cc @@ -120,6 +120,27 @@ TEST_F(WindowsTest, LaunchHeadlessEngine) { ASSERT_NE(engine, nullptr); } +// Verify that the engine can return to headless mode. +TEST_F(WindowsTest, EngineCanTransitionToHeadless) { + auto& context = GetContext(); + WindowsConfigBuilder builder(context); + EnginePtr engine{builder.RunHeadless()}; + ASSERT_NE(engine, nullptr); + + // Create and then destroy a view controller that does not own its engine. + // This causes the engine to transition back to headless mode. + { + FlutterDesktopViewControllerProperties properties = {}; + ViewControllerPtr controller{ + FlutterDesktopEngineCreateViewController(engine.get(), &properties)}; + + ASSERT_NE(controller, nullptr); + } + + // The engine is back in headless mode now. + ASSERT_NE(engine, nullptr); +} + // Verify that accessibility features are initialized when a view is created. TEST_F(WindowsTest, LaunchRefreshesAccessibility) { auto& context = GetContext(); diff --git a/shell/platform/windows/flutter_windows_view_controller.h b/shell/platform/windows/flutter_windows_view_controller.h index c8b697dcb6a99..7d567d8641751 100644 --- a/shell/platform/windows/flutter_windows_view_controller.h +++ b/shell/platform/windows/flutter_windows_view_controller.h @@ -19,11 +19,18 @@ class FlutterWindowsViewController { std::unique_ptr view) : engine_(std::move(engine)), view_(std::move(view)) {} - FlutterWindowsEngine* engine() { return engine_.get(); } + FlutterWindowsEngine* engine() { return view_->GetEngine(); } FlutterWindowsView* view() { return view_.get(); } private: + // The engine owned by this view controller, if any. + // This is only used if the view controller was created + // using |FlutterDesktopViewControllerCreate| as that takes + // ownership of the engine. View controllers created using + // |FlutterDesktopEngineCreateViewController| do not take + // ownership of the engine and do not set this. std::unique_ptr engine_; + std::unique_ptr view_; }; diff --git a/shell/platform/windows/testing/windows_test_config_builder.cc b/shell/platform/windows/testing/windows_test_config_builder.cc index f39b4db64377d..2395b5ed524a0 100644 --- a/shell/platform/windows/testing/windows_test_config_builder.cc +++ b/shell/platform/windows/testing/windows_test_config_builder.cc @@ -108,6 +108,8 @@ ViewControllerPtr WindowsConfigBuilder::Run() const { int width = 600; int height = 400; + + // Create a view controller that owns the engine. ViewControllerPtr controller{ FlutterDesktopViewControllerCreate(width, height, engine.release())}; if (!controller) { diff --git a/shell/platform/windows/testing/windows_test_config_builder.h b/shell/platform/windows/testing/windows_test_config_builder.h index f634c86f53948..1b54b5e009a40 100644 --- a/shell/platform/windows/testing/windows_test_config_builder.h +++ b/shell/platform/windows/testing/windows_test_config_builder.h @@ -71,7 +71,7 @@ class WindowsConfigBuilder { EnginePtr RunHeadless() const; // Returns a configured and initialized view controller that runs the - // configured Dart entrypoint. + // configured Dart entrypoint and owns its engine. // // Returns null on failure. ViewControllerPtr Run() const; From f03127cf582d980b4515ac6681ebec002ddeddeb Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Thu, 21 Mar 2024 14:23:21 -0700 Subject: [PATCH 2/2] Feedback --- shell/platform/windows/flutter_windows.cc | 3 +++ shell/platform/windows/flutter_windows_internal.h | 1 - .../windows/flutter_windows_view_controller.h | 12 ++++++++---- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index af8afbea2785b..874b97d537c22 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -73,6 +73,9 @@ static FlutterDesktopTextureRegistrarRef HandleForTextureRegistrar( } // Creates a view controller that might own the engine. +// +// If `owns_engine` is true, then the returned `FlutterDesktopViewControllerRef` +// owns `engine_ref` and will deallocate `engine_ref` upon its own destruction. static FlutterDesktopViewControllerRef CreateViewController( FlutterDesktopEngineRef engine_ref, int width, diff --git a/shell/platform/windows/flutter_windows_internal.h b/shell/platform/windows/flutter_windows_internal.h index d06c2155c011e..bb1e6f767905f 100644 --- a/shell/platform/windows/flutter_windows_internal.h +++ b/shell/platform/windows/flutter_windows_internal.h @@ -21,7 +21,6 @@ typedef struct { // The view's initial height. int height; - } FlutterDesktopViewControllerProperties; // Creates a view for the given engine. diff --git a/shell/platform/windows/flutter_windows_view_controller.h b/shell/platform/windows/flutter_windows_view_controller.h index 7d567d8641751..10175fc9546ba 100644 --- a/shell/platform/windows/flutter_windows_view_controller.h +++ b/shell/platform/windows/flutter_windows_view_controller.h @@ -24,11 +24,15 @@ class FlutterWindowsViewController { private: // The engine owned by this view controller, if any. - // This is only used if the view controller was created + // + // This is used only if the view controller was created // using |FlutterDesktopViewControllerCreate| as that takes - // ownership of the engine. View controllers created using - // |FlutterDesktopEngineCreateViewController| do not take - // ownership of the engine and do not set this. + // ownership of the engine. Destroying this view controller + // also destroys the engine. + // + // View controllers created using |FlutterDesktopEngineCreateViewController| + // do not take ownership of the engine and this will be null. Destroying + // this view controller does not destroy the engine. std::unique_ptr engine_; std::unique_ptr view_;