diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 0a793dc3977eb..4e7da0768650b 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -3375,6 +3375,8 @@ FlutterEngineResult FlutterEngineGetProcAddresses( SET_PROC(NotifyDisplayUpdate, FlutterEngineNotifyDisplayUpdate); SET_PROC(ScheduleFrame, FlutterEngineScheduleFrame); SET_PROC(SetNextFrameCallback, FlutterEngineSetNextFrameCallback); + SET_PROC(AddView, FlutterEngineAddView); + SET_PROC(RemoveView, FlutterEngineRemoveView); #undef SET_PROC return kSuccess; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 702b1d4923043..fd6d3db247ac4 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -3287,6 +3287,12 @@ typedef FlutterEngineResult (*FlutterEngineSetNextFrameCallbackFnPtr)( FLUTTER_API_SYMBOL(FlutterEngine) engine, VoidCallback callback, void* user_data); +typedef FlutterEngineResult (*FlutterEngineAddViewFnPtr)( + FLUTTER_API_SYMBOL(FlutterEngine) engine, + const FlutterAddViewInfo* info); +typedef FlutterEngineResult (*FlutterEngineRemoveViewFnPtr)( + FLUTTER_API_SYMBOL(FlutterEngine) engine, + const FlutterRemoveViewInfo* info); /// Function-pointer-based versions of the APIs above. typedef struct { @@ -3333,6 +3339,8 @@ typedef struct { FlutterEngineNotifyDisplayUpdateFnPtr NotifyDisplayUpdate; FlutterEngineScheduleFrameFnPtr ScheduleFrame; FlutterEngineSetNextFrameCallbackFnPtr SetNextFrameCallback; + FlutterEngineAddViewFnPtr AddView; + FlutterEngineRemoveViewFnPtr RemoveView; } FlutterEngineProcTable; //------------------------------------------------------------------------------ diff --git a/shell/platform/windows/fixtures/main.dart b/shell/platform/windows/fixtures/main.dart index 3df3e670f9cf0..e24c9d872b87a 100644 --- a/shell/platform/windows/fixtures/main.dart +++ b/shell/platform/windows/fixtures/main.dart @@ -361,3 +361,12 @@ void signalViewIds() { signalStringValue('View IDs: [${viewIds.join(', ')}]'); } + +@pragma('vm:entry-point') +void onMetricsChangedSignalViewIds() { + ui.PlatformDispatcher.instance.onMetricsChanged = () { + signalViewIds(); + }; + + signal(); +} diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index 39aac3fbcef20..c79c6aca60fd8 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -93,6 +93,10 @@ static FlutterDesktopViewControllerRef CreateViewController( std::unique_ptr view = engine_ptr->CreateView(std::move(window_wrapper)); + if (!view) { + return nullptr; + } + auto controller = std::make_unique( std::move(engine), std::move(view)); diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index a92be617af724..bd1705440040d 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -12,6 +12,7 @@ #include "flutter/fml/logging.h" #include "flutter/fml/paths.h" #include "flutter/fml/platform/win/wstring_conversion.h" +#include "flutter/fml/synchronization/waitable_event.h" #include "flutter/shell/platform/common/client_wrapper/binary_messenger_impl.h" #include "flutter/shell/platform/common/client_wrapper/include/flutter/standard_message_codec.h" #include "flutter/shell/platform/common/path_utils.h" @@ -149,6 +150,7 @@ FlutterWindowsEngine::FlutterWindowsEngine( : project_(std::make_unique(project)), windows_proc_table_(std::move(windows_proc_table)), aot_data_(nullptr, nullptr), + views_mutex_(fml::SharedMutex::Create()), lifecycle_manager_(std::make_unique(this)) { if (windows_proc_table_ == nullptr) { windows_proc_table_ = std::make_shared(); @@ -492,34 +494,118 @@ bool FlutterWindowsEngine::Stop() { std::unique_ptr FlutterWindowsEngine::CreateView( std::unique_ptr window) { - // TODO(loicsharma): Remove implicit view assumption. - // https://github.com/flutter/flutter/issues/142845 + auto view_id = next_view_id_; auto view = std::make_unique( - kImplicitViewId, this, std::move(window), windows_proc_table_); + view_id, this, std::move(window), windows_proc_table_); view->CreateRenderSurface(); - views_[kImplicitViewId] = view.get(); + next_view_id_++; + + { + // Add the view to the embedder. This must happen before the engine + // is notified the view exists and starts presenting to it. + fml::UniqueLock write_lock{*views_mutex_}; + FML_DCHECK(views_.find(view_id) == views_.end()); + views_[view_id] = view.get(); + } + + if (!view->IsImplicitView()) { + FML_DCHECK(running()); + + struct Captures { + fml::AutoResetWaitableEvent latch; + bool added; + }; + Captures captures = {}; + + FlutterWindowMetricsEvent metrics = view->CreateWindowMetricsEvent(); + + FlutterAddViewInfo info = {}; + info.struct_size = sizeof(FlutterAddViewInfo); + info.view_id = view_id; + info.view_metrics = &metrics; + info.user_data = &captures; + info.add_view_callback = [](const FlutterAddViewResult* result) { + Captures* captures = reinterpret_cast(result->user_data); + captures->added = result->added; + captures->latch.Signal(); + }; + + embedder_api_.AddView(engine_, &info); + + // Block the platform thread until the engine has added the view. + // TODO(loicsharma): This blocks the platform thread eagerly and can + // cause unnecessary delay in input processing. Instead, this should block + // lazily only when the app does an operation which needs the view. + // https://github.com/flutter/flutter/issues/146248 + captures.latch.Wait(); + + if (!captures.added) { + // Adding the view failed. Update the embedder's state to match the + // engine's state. This is unexpected and indicates a bug in the Windows + // embedder. + FML_LOG(ERROR) << "FlutterEngineAddView failed to add view"; + fml::UniqueLock write_lock{*views_mutex_}; + views_.erase(view_id); + return nullptr; + } + } return std::move(view); } void FlutterWindowsEngine::RemoveView(FlutterViewId view_id) { FML_DCHECK(running()); - FML_DCHECK(views_.find(view_id) != views_.end()); - if (view_id == kImplicitViewId) { - // The engine and framework assume the implicit view always exists. - // Attempts to render to the implicit view will be ignored. - views_.erase(view_id); - return; + // Notify the engine to stop rendering to the view if it isn't the implicit + // view. The engine and framework assume the implicit view always exists and + // can continue presenting. + if (view_id != kImplicitViewId) { + struct Captures { + fml::AutoResetWaitableEvent latch; + bool removed; + }; + Captures captures = {}; + + FlutterRemoveViewInfo info = {}; + info.struct_size = sizeof(FlutterRemoveViewInfo); + info.view_id = view_id; + info.user_data = &captures; + info.remove_view_callback = [](const FlutterRemoveViewResult* result) { + // This is invoked on the raster thread, the same thread that the present + // callback is invoked. If |FlutterRemoveViewResult.removed| is `true`, + // the engine guarantees the view won't be presented. + Captures* captures = reinterpret_cast(result->user_data); + captures->removed = result->removed; + captures->latch.Signal(); + }; + + embedder_api_.RemoveView(engine_, &info); + + // Block the platform thread until the engine has removed the view. + // TODO(loicsharma): This blocks the platform thread eagerly and can + // cause unnecessary delay in input processing. Instead, this should block + // lazily only when an operation needs the view. + // https://github.com/flutter/flutter/issues/146248 + captures.latch.Wait(); + + if (!captures.removed) { + // Removing the view failed. This is unexpected and indicates a bug in the + // Windows embedder. + FML_LOG(ERROR) << "FlutterEngineRemoveView failed to remove view"; + return; + } } - // TODO(loicsharma): Remove the view from the engine using the - // `FlutterEngineRemoveView` embedder API. Windows does not - // support views other than the implicit view yet. - // https://github.com/flutter/flutter/issues/144810 - FML_UNREACHABLE(); + { + // The engine no longer presents to the view. Remove the view from the + // embedder. + fml::UniqueLock write_lock{*views_mutex_}; + + FML_DCHECK(views_.find(view_id) != views_.end()); + views_.erase(view_id); + } } void FlutterWindowsEngine::OnVsync(intptr_t baton) { @@ -551,6 +637,8 @@ std::chrono::nanoseconds FlutterWindowsEngine::FrameInterval() { } FlutterWindowsView* FlutterWindowsEngine::view(FlutterViewId view_id) const { + fml::SharedLock read_lock{*views_mutex_}; + auto iterator = views_.find(view_id); if (iterator == views_.end()) { return nullptr; @@ -779,6 +867,8 @@ bool FlutterWindowsEngine::DispatchSemanticsAction( void FlutterWindowsEngine::UpdateSemanticsEnabled(bool enabled) { if (engine_ && semantics_enabled_ != enabled) { + fml::SharedLock read_lock{*views_mutex_}; + semantics_enabled_ = enabled; embedder_api_.UpdateSemanticsEnabled(engine_, enabled); for (auto iterator = views_.begin(); iterator != views_.end(); iterator++) { @@ -844,6 +934,8 @@ void FlutterWindowsEngine::OnQuit(std::optional hwnd, } void FlutterWindowsEngine::OnDwmCompositionChanged() { + fml::SharedLock read_lock{*views_mutex_}; + for (auto iterator = views_.begin(); iterator != views_.end(); iterator++) { iterator->second->OnDwmCompositionChanged(); } @@ -875,6 +967,11 @@ void FlutterWindowsEngine::OnChannelUpdate(std::string name, bool listening) { } bool FlutterWindowsEngine::Present(const FlutterPresentViewInfo* info) { + // This runs on the raster thread. Lock the views map for the entirety of the + // present operation to block the platform thread from destroying the + // view during the present. + fml::SharedLock read_lock{*views_mutex_}; + auto iterator = views_.find(info->view_id); if (iterator == views_.end()) { return false; diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index 0f620c282c485..c0f4185be0f8b 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -16,6 +16,7 @@ #include "flutter/fml/closure.h" #include "flutter/fml/macros.h" +#include "flutter/fml/synchronization/shared_mutex.h" #include "flutter/shell/platform/common/accessibility_bridge.h" #include "flutter/shell/platform/common/app_lifecycle_state.h" #include "flutter/shell/platform/common/client_wrapper/binary_messenger_impl.h" @@ -121,6 +122,8 @@ class FlutterWindowsEngine { virtual bool Stop(); // Create a view that can display this engine's content. + // + // Returns null on failure. std::unique_ptr CreateView( std::unique_ptr window); @@ -359,9 +362,30 @@ class FlutterWindowsEngine { // AOT data, if any. UniqueAotDataPtr aot_data_; + // The ID that the next view will have. + FlutterViewId next_view_id_ = kImplicitViewId; + // The views displaying the content running in this engine, if any. + // + // This is read and mutated by the platform thread. This is read by the raster + // thread to present content to a view. + // + // Reads to this object on non-platform threads must be protected + // by acquiring a shared lock on |views_mutex_|. + // + // Writes to this object must only happen on the platform thread + // and must be protected by acquiring an exclusive lock on |views_mutex_|. std::unordered_map views_; + // The mutex that protects the |views_| map. + // + // The raster thread acquires a shared lock to present to a view. + // + // The platform thread acquires a shared lock to access the view. + // The platform thread acquires an exclusive lock before adding + // a view to the engine or after removing a view from the engine. + std::unique_ptr views_mutex_; + // Task runner for tasks posted from the engine. std::unique_ptr task_runner_; diff --git a/shell/platform/windows/flutter_windows_unittests.cc b/shell/platform/windows/flutter_windows_unittests.cc index 650edad904211..6d0befe60c628 100644 --- a/shell/platform/windows/flutter_windows_unittests.cc +++ b/shell/platform/windows/flutter_windows_unittests.cc @@ -570,5 +570,65 @@ TEST_F(WindowsTest, GetKeyboardStateHeadless) { } } +// Verify the embedder can add and remove views. +TEST_F(WindowsTest, AddRemoveView) { + std::mutex mutex; + std::string view_ids; + + auto& context = GetContext(); + WindowsConfigBuilder builder(context); + builder.SetDartEntrypoint("onMetricsChangedSignalViewIds"); + + fml::AutoResetWaitableEvent ready_latch; + context.AddNativeFunction( + "Signal", CREATE_NATIVE_ENTRY( + [&](Dart_NativeArguments args) { ready_latch.Signal(); })); + + context.AddNativeFunction( + "SignalStringValue", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + ASSERT_FALSE(Dart_IsError(handle)); + + std::scoped_lock lock{mutex}; + view_ids = tonic::DartConverter::FromDart(handle); + })); + + // Create the implicit view. + ViewControllerPtr first_controller{builder.Run()}; + ASSERT_NE(first_controller, nullptr); + + ready_latch.Wait(); + + // Create a second view. + FlutterDesktopEngineRef engine = + FlutterDesktopViewControllerGetEngine(first_controller.get()); + FlutterDesktopViewControllerProperties properties = {}; + properties.width = 100; + properties.height = 100; + ViewControllerPtr second_controller{ + FlutterDesktopEngineCreateViewController(engine, &properties)}; + ASSERT_NE(second_controller, nullptr); + + // Pump messages for the Windows platform task runner until the view is added. + while (true) { + PumpMessage(); + std::scoped_lock lock{mutex}; + if (view_ids == "View IDs: [0, 1]") { + break; + } + } + + // Delete the second view and pump messages for the Windows platform task + // runner until the view is removed. + second_controller.reset(); + while (true) { + PumpMessage(); + std::scoped_lock lock{mutex}; + if (view_ids == "View IDs: [0]") { + break; + } + } +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 93f523194714a..72c9dd2216087 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -362,21 +362,38 @@ void FlutterWindowsView::OnResetImeComposing() { // Sends new size information to FlutterEngine. void FlutterWindowsView::SendWindowMetrics(size_t width, size_t height, - double dpiScale) const { + double pixel_ratio) const { FlutterWindowMetricsEvent event = {}; event.struct_size = sizeof(event); event.width = width; event.height = height; - event.pixel_ratio = dpiScale; + event.pixel_ratio = pixel_ratio; event.view_id = view_id_; engine_->SendWindowMetricsEvent(event); } -void FlutterWindowsView::SendInitialBounds() { +FlutterWindowMetricsEvent FlutterWindowsView::CreateWindowMetricsEvent() const { PhysicalWindowBounds bounds = binding_handler_->GetPhysicalWindowBounds(); + double pixel_ratio = binding_handler_->GetDpiScale(); - SendWindowMetrics(bounds.width, bounds.height, - binding_handler_->GetDpiScale()); + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = bounds.width; + event.height = bounds.height; + event.pixel_ratio = pixel_ratio; + event.view_id = view_id_; + + return event; +} + +void FlutterWindowsView::SendInitialBounds() { + // Non-implicit views' initial window metrics are sent when the view is added + // to the engine. + if (!IsImplicitView()) { + return; + } + + engine_->SendWindowMetricsEvent(CreateWindowMetricsEvent()); } FlutterWindowsView::PointerState* FlutterWindowsView::GetOrCreatePointerState( @@ -673,6 +690,10 @@ FlutterViewId FlutterWindowsView::view_id() const { return view_id_; } +bool FlutterWindowsView::IsImplicitView() const { + return view_id_ == kImplicitViewId; +} + void FlutterWindowsView::CreateRenderSurface() { FML_DCHECK(surface_ == nullptr); diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index 782e63ad53136..c269b2c32d6db 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -46,8 +46,25 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { // Get the view's unique identifier. FlutterViewId view_id() const; - // Creates rendering surface for Flutter engine to draw into. - // Should be called before calling FlutterEngineRun using this view. + // Whether this view is the implicit view. + // + // The implicit view is a special view for backwards compatibility. + // The engine assumes it can always render to this view, even if the app has + // destroyed the window for this view. + // + // Today, the implicit view is the first view that is created. It is the only + // view that can be created before the engine is launched. + // + // The embedder must ignore presents to this view before it is created and + // after it is destroyed. + // + // See: + // https://api.flutter.dev/flutter/dart-ui/PlatformDispatcher/implicitView.html + bool IsImplicitView() const; + + // Create a rendering surface for Flutter engine to draw into. + // + // This is a no-op if using software rasterization. void CreateRenderSurface(); // Get the EGL surface that backs the Flutter view. @@ -72,7 +89,15 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { size_t row_bytes, size_t height); - // Send initial bounds to embedder. Must occur after engine has initialized. + // Creates a window metric for this view. + // + // Used to notify the engine of a view's current size and device pixel ratio. + FlutterWindowMetricsEvent CreateWindowMetricsEvent() const; + + // Send initial bounds to embedder. Must occur after engine has initialized. + // + // This is a no-op if this is not the implicit view. Non-implicit views' + // initial window metrics are sent when the view is added to the engine. void SendInitialBounds(); // Set the text of the alert, and create it if it does not yet exist. @@ -286,8 +311,8 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { bool ResizeRenderSurface(size_t width, size_t height); // Sends a window metrics update to the Flutter engine using current window - // dimensions in physical - void SendWindowMetrics(size_t width, size_t height, double dpiscale) const; + // dimensions in physical pixels. + void SendWindowMetrics(size_t width, size_t height, double pixel_ratio) const; // Reports a mouse movement to Flutter engine. void SendPointerMove(double x, double y, PointerState* state);