diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 45290d3a28cc7..574b9e27f33ca 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -121,26 +121,28 @@ void PlatformConfiguration::AddView(int64_t view_id, })); } -void PlatformConfiguration::RemoveView(int64_t view_id) { +bool PlatformConfiguration::RemoveView(int64_t view_id) { if (view_id == kFlutterImplicitViewId) { - FML_LOG(ERROR) << "The implicit view #" << view_id << " cannot be removed."; - FML_DCHECK(false); - return; + FML_LOG(FATAL) << "The implicit view #" << view_id << " cannot be removed."; + return false; } size_t erased_elements = metrics_.erase(view_id); - FML_DCHECK(erased_elements != 0) << "View #" << view_id << " doesn't exist."; - (void)erased_elements; // Suppress unused variable warning + if (erased_elements == 0) { + FML_LOG(ERROR) << "View #" << view_id << " doesn't exist."; + return false; + } std::shared_ptr dart_state = remove_view_.dart_state().lock(); if (!dart_state) { - return; + return false; } tonic::DartState::Scope scope(dart_state); tonic::CheckAndHandleError( tonic::DartInvoke(remove_view_.Get(), { tonic::ToDart(view_id), })); + return true; } bool PlatformConfiguration::UpdateViewMetrics( diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index a8b54a80b8cee..6a583765d962f 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -327,7 +327,9 @@ class PlatformConfiguration final { /// /// @param[in] view_id The ID of the view. /// - void RemoveView(int64_t view_id); + /// @return Whether the view was removed. + /// + bool RemoveView(int64_t view_id); //---------------------------------------------------------------------------- /// @brief Update the view metrics for the specified view. diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index f6b9b5422ce0d..b967b9cde9912 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -151,8 +151,7 @@ bool RuntimeController::AddView(int64_t view_id, bool RuntimeController::RemoveView(int64_t view_id) { platform_data_.viewport_metrics_for_views.erase(view_id); if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { - platform_configuration->RemoveView(view_id); - return true; + return platform_configuration->RemoveView(view_id); } return false; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 062f9e6b9be9c..7dd03ed19fb78 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -301,8 +301,8 @@ void Engine::AddView(int64_t view_id, const ViewportMetrics& view_metrics) { runtime_controller_->AddView(view_id, view_metrics); } -void Engine::RemoveView(int64_t view_id) { - runtime_controller_->RemoveView(view_id); +bool Engine::RemoveView(int64_t view_id) { + return runtime_controller_->RemoveView(view_id); } void Engine::SetViewportMetrics(int64_t view_id, diff --git a/shell/common/engine.h b/shell/common/engine.h index 501e47178eafe..65660f6124c3f 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -735,7 +735,9 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// /// @param[in] view_id The ID of the view. /// - void RemoveView(int64_t view_id); + /// @return Whether the view was removed. + /// + bool RemoveView(int64_t view_id); //---------------------------------------------------------------------------- /// @brief Updates the viewport metrics for a view. The viewport metrics diff --git a/shell/common/shell.cc b/shell/common/shell.cc index f0cfe5bf520cf..0228f2744f72d 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -2120,7 +2120,7 @@ void Shell::AddView(int64_t view_id, const ViewportMetrics& viewport_metrics) { }); } -void Shell::RemoveView(int64_t view_id) { +void Shell::RemoveView(int64_t view_id, RemoveViewCallback callback) { TRACE_EVENT0("flutter", "Shell::RemoveView"); FML_DCHECK(is_set_up_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); @@ -2133,10 +2133,12 @@ void Shell::RemoveView(int64_t view_id) { [&task_runners = task_runners_, // engine = engine_->GetWeakPtr(), // rasterizer = rasterizer_->GetWeakPtr(), // - view_id // + view_id, // + callback = std::move(callback) // ] { if (engine) { - engine->RemoveView(view_id); + bool removed = engine->RemoveView(view_id); + callback(removed); } // Don't wait for the raster task here, which only cleans up memory and // does not affect functionality. Make sure it is done after Dart diff --git a/shell/common/shell.h b/shell/common/shell.h index 14d3dae22a5b1..3e2da290ed130 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -133,6 +133,7 @@ class Shell final : public PlatformView::Delegate, const std::shared_ptr& gpu_disabled_switch, impeller::RuntimeStageBackend runtime_stage_type)> EngineCreateCallback; + using RemoveViewCallback = std::function; //---------------------------------------------------------------------------- /// @brief Creates a shell instance using the provided settings. The @@ -330,8 +331,10 @@ class Shell final : public PlatformView::Delegate, /// `kFlutterImplicitViewId` triggers an assertion. /// /// @param[in] view_id The view ID of the view to be removed. + /// @param[in] callback The callback that's invoked once the engine has + /// attempted to remove the view. /// - void RemoveView(int64_t view_id); + void RemoveView(int64_t view_id, RemoveViewCallback callback); //---------------------------------------------------------------------------- /// @brief Captures a screenshot and optionally Base64 encodes the data diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 162f13d626016..c145f2b8b6da7 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4511,8 +4511,9 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) { ASSERT_EQ(viewIds.size(), 2u); ASSERT_EQ(viewIds[1], 2ll); - PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), - [&shell] { shell->RemoveView(2); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { + shell->RemoveView(2, [](bool removed) { ASSERT_TRUE(removed); }); + }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 1u); diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index f35dd805ecb77..8c311cf90e575 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -2111,6 +2111,44 @@ FlutterEngineResult FlutterEngineRunInitialized( return kSuccess; } +FLUTTER_EXPORT +FlutterEngineResult FlutterEngineRemoveView(FLUTTER_API_SYMBOL(FlutterEngine) + engine, + const FlutterRemoveViewInfo* info) { + if (!engine) { + return LOG_EMBEDDER_ERROR(kInvalidArguments, "Engine handle was invalid."); + } + if (!info || !info->remove_view_callback) { + return LOG_EMBEDDER_ERROR(kInvalidArguments, + "Remove view info handle was invalid."); + } + + if (info->view_id == kFlutterImplicitViewId) { + return LOG_EMBEDDER_ERROR( + kInvalidArguments, + "Remove view info was invalid. The implicit view cannot be removed."); + } + + // The engine must be running to remove a view. + auto embedder_engine = reinterpret_cast(engine); + if (!embedder_engine->IsValid()) { + return LOG_EMBEDDER_ERROR(kInvalidArguments, "Engine handle was invalid."); + } + + flutter::Shell::RemoveViewCallback callback = + [c_callback = info->remove_view_callback, + user_data = info->user_data](bool removed) { + FlutterRemoveViewResult result = {}; + result.struct_size = sizeof(FlutterRemoveViewResult); + result.removed = removed; + result.user_data = user_data; + c_callback(&result); + }; + + embedder_engine->GetShell().RemoveView(info->view_id, callback); + return kSuccess; +} + FLUTTER_EXPORT FlutterEngineResult FlutterEngineDeinitialize(FLUTTER_API_SYMBOL(FlutterEngine) engine) { diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index dce500fe66897..536a5412626a3 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -831,6 +831,53 @@ typedef struct { }; } FlutterRendererConfig; +typedef struct { + /// The size of this struct. + /// Must be sizeof(FlutterRemoveViewResult). + size_t struct_size; + + /// True if the remove view operation succeeded. + bool removed; + + /// The |FlutterRemoveViewInfo.user_data|. + void* user_data; +} FlutterRemoveViewResult; + +/// The callback invoked by the engine when the engine has attempted to remove +/// a view. +/// +/// The |FlutterRemoveViewResult| will be deallocated once the callback returns. +typedef void (*FlutterRemoveViewCallback)( + const FlutterRemoveViewResult* /* result */); + +typedef struct { + /// The size of this struct. + /// Must be sizeof(FlutterRemoveViewInfo). + size_t struct_size; + + /// The identifier for the view to remove. + /// + /// The implicit view cannot be removed if it is enabled. + FlutterViewId view_id; + + /// A baton that is not interpreted by the engine in any way. + /// It will be given back to the embedder in |remove_view_callback|. + /// Embedder resources may be associated with this baton. + void* user_data; + + /// Called once the engine has attempted to remove the view. + /// This callback is required. + /// + /// The embedder must not destroy the underlying surface until the callback is + /// invoked with a `removed` value of `true`. + /// + /// This callback is invoked on an internal engine managed thread. + /// Embedders must re-thread if necessary. + /// + /// The |result| argument will be deallocated when the callback returns. + FlutterRemoveViewCallback remove_view_callback; +} FlutterRemoveViewInfo; + /// Display refers to a graphics hardware system consisting of a framebuffer, /// typically a monitor or a screen. This ID is unique per display and is /// stable until the Flutter application restarts. @@ -2418,6 +2465,25 @@ FLUTTER_EXPORT FlutterEngineResult FlutterEngineRunInitialized( FLUTTER_API_SYMBOL(FlutterEngine) engine); +//------------------------------------------------------------------------------ +/// @brief Removes a view. +/// +/// This is an asynchronous operation. The view's resources must not +/// be cleaned up until the |remove_view_callback| is invoked with +/// a |removed| value of `true`. +/// +/// @param[in] engine A running engine instance. +/// @param[in] info The remove view arguments. This can be deallocated +/// once |FlutterEngineRemoveView| returns, before +/// |remove_view_callback| is invoked. +/// +/// @return The result of *starting* the asynchronous operation. If +/// `kSuccess`, the |remove_view_callback| will be invoked. +FLUTTER_EXPORT +FlutterEngineResult FlutterEngineRemoveView(FLUTTER_API_SYMBOL(FlutterEngine) + engine, + const FlutterRemoveViewInfo* info); + FLUTTER_EXPORT FlutterEngineResult FlutterEngineSendWindowMetricsEvent( FLUTTER_API_SYMBOL(FlutterEngine) engine, diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 44c7426fd70fd..2b3422c5e7c99 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -10,6 +10,7 @@ #include "embedder.h" #include "embedder_engine.h" +#include "flutter/common/constants.h" #include "flutter/flow/raster_cache.h" #include "flutter/fml/file.h" #include "flutter/fml/make_copyable.h" @@ -1196,6 +1197,49 @@ TEST_F(EmbedderTest, CanDeinitializeAnEngine) { engine.reset(); } +TEST_F(EmbedderTest, CanRemoveView) { + // TODO(loicsharma): We can't test this until views can be added! + // https://github.com/flutter/flutter/issues/144806 +} + +TEST_F(EmbedderTest, CannotRemoveImplicitView) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); + EmbedderConfigBuilder builder(context); + builder.SetSoftwareRendererConfig(); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + FlutterRemoveViewInfo info = {}; + info.struct_size = sizeof(FlutterRemoveViewInfo); + info.view_id = kFlutterImplicitViewId; + info.remove_view_callback = [](const FlutterRemoveViewResult* result) { + FAIL(); + }; + ASSERT_EQ(FlutterEngineRemoveView(engine.get(), &info), kInvalidArguments); +} + +TEST_F(EmbedderTest, CannotRemoveUnknownView) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); + EmbedderConfigBuilder builder(context); + builder.SetSoftwareRendererConfig(); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + fml::AutoResetWaitableEvent latch; + FlutterRemoveViewInfo info = {}; + info.struct_size = sizeof(FlutterRemoveViewInfo); + info.view_id = 123; + info.user_data = &latch; + info.remove_view_callback = [](const FlutterRemoveViewResult* result) { + ASSERT_FALSE(result->removed); + reinterpret_cast(result->user_data)->Signal(); + }; + ASSERT_EQ(FlutterEngineRemoveView(engine.get(), &info), kSuccess); + latch.Wait(); +} + TEST_F(EmbedderTest, CanUpdateLocales) { auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); EmbedderConfigBuilder builder(context);