From e2fe6eb689fb720a8ed2c2fdc8f16474d7188bda Mon Sep 17 00:00:00 2001 From: David Worsham Date: Wed, 28 Apr 2021 23:57:30 -0700 Subject: [PATCH] fuchsia: Handle multiple views in platformViews path (#25343) --- flow/layers/fuchsia_layer_unittests.cc | 33 ++- flow/scene_update_context.cc | 12 +- flow/scene_update_context.h | 10 +- flow/view_holder.cc | 40 ++-- flow/view_holder.h | 26 ++- lib/ui/compositing/scene_host.cc | 72 +++--- lib/ui/compositing/scene_host.h | 6 + shell/platform/fuchsia/flutter/engine.cc | 50 +++-- shell/platform/fuchsia/flutter/engine.h | 8 +- .../flutter/fuchsia_external_view_embedder.cc | 15 +- .../flutter/fuchsia_external_view_embedder.h | 7 +- .../platform/fuchsia/flutter/platform_view.cc | 206 +++++++++++++++--- .../platform/fuchsia/flutter/platform_view.h | 22 +- .../fuchsia/flutter/platform_view_unittest.cc | 160 +++++++++++--- 14 files changed, 481 insertions(+), 186 deletions(-) diff --git a/flow/layers/fuchsia_layer_unittests.cc b/flow/layers/fuchsia_layer_unittests.cc index 85278b8c6..ee8b86bfa 100644 --- a/flow/layers/fuchsia_layer_unittests.cc +++ b/flow/layers/fuchsia_layer_unittests.cc @@ -20,8 +20,6 @@ #include "flutter/flow/layers/physical_shape_layer.h" #include "flutter/flow/layers/transform_layer.h" #include "flutter/flow/view_holder.h" -#include "flutter/fml/platform/fuchsia/message_loop_fuchsia.h" -#include "flutter/fml/task_runner.h" #include "gtest/gtest.h" namespace flutter { @@ -251,10 +249,6 @@ class MockSessionWrapper : public flutter::SessionWrapper { }; struct TestContext { - // Message loop. - fml::RefPtr loop; - fml::RefPtr task_runner; - // Session. fidl::InterfaceRequest listener_request; MockSession mock_session; @@ -273,10 +267,6 @@ struct TestContext { std::unique_ptr InitTest() { std::unique_ptr context = std::make_unique(); - // Init message loop. - context->loop = fml::MakeRefCounted(); - context->task_runner = fml::MakeRefCounted(context->loop); - // Init Session. fuchsia::ui::scenic::SessionPtr session_ptr; fuchsia::ui::scenic::SessionListenerPtr listener; @@ -318,7 +308,7 @@ zx_koid_t GetChildLayerId() { class AutoDestroyChildLayerId { public: AutoDestroyChildLayerId(zx_koid_t id) : id_(id) {} - ~AutoDestroyChildLayerId() { ViewHolder::Destroy(id_); } + ~AutoDestroyChildLayerId() { ViewHolder::Destroy(id_, nullptr); } private: zx_koid_t id_; @@ -360,9 +350,10 @@ TEST_F(FuchsiaLayerTest, DISABLED_PhysicalShapeLayersAndChildSceneLayers) { const zx_koid_t kChildLayerId1 = GetChildLayerId(); auto [unused_view_token1, unused_view_holder_token1] = scenic::ViewTokenPair::New(); - ViewHolder::Create(kChildLayerId1, test_context->task_runner, - std::move(unused_view_holder_token1), - /*bind_callback=*/[](scenic::ResourceId id) {}); + ViewHolder::Create( + kChildLayerId1, + /*bind_callback=*/[](scenic::ResourceId id) {}, + std::move(unused_view_holder_token1)); // Will destroy only when we go out of scope (i.e. end of the test). AutoDestroyChildLayerId auto_destroy1(kChildLayerId1); auto child_view1 = std::make_shared( @@ -388,9 +379,10 @@ TEST_F(FuchsiaLayerTest, DISABLED_PhysicalShapeLayersAndChildSceneLayers) { const zx_koid_t kChildLayerId2 = GetChildLayerId(); auto [unused_view_token2, unused_view_holder_token2] = scenic::ViewTokenPair::New(); - ViewHolder::Create(kChildLayerId2, test_context->task_runner, - std::move(unused_view_holder_token2), - /*bind_callback=*/[](scenic::ResourceId id) {}); + ViewHolder::Create( + kChildLayerId2, + /*bind_callback=*/[](scenic::ResourceId id) {}, + std::move(unused_view_holder_token2)); // Will destroy only when we go out of scope (i.e. end of the test). AutoDestroyChildLayerId auto_destroy2(kChildLayerId2); auto child_view2 = std::make_shared( @@ -604,9 +596,10 @@ TEST_F(FuchsiaLayerTest, DISABLED_OpacityAndTransformLayer) { auto [unused_view_token1, unused_view_holder_token1] = scenic::ViewTokenPair::New(); - ViewHolder::Create(kChildLayerId1, test_context->task_runner, - std::move(unused_view_holder_token1), - /*bind_callback=*/[](scenic::ResourceId id) {}); + ViewHolder::Create( + kChildLayerId1, + /*bind_callback=*/[](scenic::ResourceId id) {}, + std::move(unused_view_holder_token1)); // Will destroy only when we go out of scope (i.e. end of the test). AutoDestroyChildLayerId auto_destroy1(kChildLayerId1); auto child_view1 = std::make_shared( diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index 6f0e4fa8f..aaf1f75a1 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -223,13 +223,13 @@ void SceneUpdateContext::UpdateView(int64_t view_id, } void SceneUpdateContext::CreateView(int64_t view_id, + ViewHolder::ViewIdCallback on_view_created, bool hit_testable, bool focusable) { FML_LOG(INFO) << "CreateView for view holder: " << view_id; zx_handle_t handle = (zx_handle_t)view_id; - flutter::ViewHolder::Create(handle, nullptr, - scenic::ToViewHolderToken(zx::eventpair(handle)), - nullptr); + flutter::ViewHolder::Create(handle, std::move(on_view_created), + scenic::ToViewHolderToken(zx::eventpair(handle))); auto* view_holder = ViewHolder::FromId(view_id); FML_DCHECK(view_holder); @@ -250,8 +250,10 @@ void SceneUpdateContext::UpdateView(int64_t view_id, view_holder->set_focusable(focusable); } -void SceneUpdateContext::DestroyView(int64_t view_id) { - ViewHolder::Destroy(view_id); +void SceneUpdateContext::DestroyView( + int64_t view_id, + ViewHolder::ViewIdCallback on_view_destroyed) { + ViewHolder::Destroy(view_id, std::move(on_view_destroyed)); } SceneUpdateContext::Entity::Entity(std::shared_ptr context) diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index dac827718..442251159 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -16,6 +16,7 @@ #include #include "flutter/flow/embedded_views.h" +#include "flutter/flow/view_holder.h" #include "flutter/fml/logging.h" #include "flutter/fml/macros.h" #include "third_party/skia/include/core/SkRect.h" @@ -167,9 +168,14 @@ class SceneUpdateContext : public flutter::ExternalViewEmbedder { return nullptr; } - void CreateView(int64_t view_id, bool hit_testable, bool focusable); + // View manipulation. + void CreateView(int64_t view_id, + ViewHolder::ViewIdCallback on_view_created, + bool hit_testable, + bool focusable); + void DestroyView(int64_t view_id, + ViewHolder::ViewIdCallback on_view_destroyed); void UpdateView(int64_t view_id, bool hit_testable, bool focusable); - void DestroyView(int64_t view_id); void UpdateView(int64_t view_id, const SkPoint& offset, const SkSize& size, diff --git a/flow/view_holder.cc b/flow/view_holder.cc index c2011825c..a3f17b699 100644 --- a/flow/view_holder.cc +++ b/flow/view_holder.cc @@ -51,9 +51,8 @@ fuchsia::ui::gfx::ViewProperties ToViewProperties(float width, namespace flutter { void ViewHolder::Create(zx_koid_t id, - fml::RefPtr ui_task_runner, - fuchsia::ui::views::ViewHolderToken view_holder_token, - const BindCallback& on_bind_callback) { + ViewIdCallback on_view_created, + fuchsia::ui::views::ViewHolderToken view_holder_token) { // This raster thread contains at least 1 ViewHolder. Initialize the // per-thread bindings. if (tls_view_holder_bindings.get() == nullptr) { @@ -64,16 +63,20 @@ void ViewHolder::Create(zx_koid_t id, FML_DCHECK(bindings); FML_DCHECK(bindings->find(id) == bindings->end()); - auto view_holder = std::make_unique(std::move(ui_task_runner), - std::move(view_holder_token), - on_bind_callback); + auto view_holder = std::unique_ptr( + new ViewHolder(std::move(view_holder_token), std::move(on_view_created))); bindings->emplace(id, std::move(view_holder)); } -void ViewHolder::Destroy(zx_koid_t id) { +void ViewHolder::Destroy(zx_koid_t id, ViewIdCallback on_view_destroyed) { auto* bindings = tls_view_holder_bindings.get(); FML_DCHECK(bindings); + auto binding = bindings->find(id); + FML_DCHECK(binding != bindings->end()); + if (binding->second->view_holder_) { + on_view_destroyed(binding->second->view_holder_->id()); + } bindings->erase(id); } @@ -91,12 +94,10 @@ ViewHolder* ViewHolder::FromId(zx_koid_t id) { return binding->second.get(); } -ViewHolder::ViewHolder(fml::RefPtr ui_task_runner, - fuchsia::ui::views::ViewHolderToken view_holder_token, - const BindCallback& on_bind_callback) - : ui_task_runner_(std::move(ui_task_runner)), - pending_view_holder_token_(std::move(view_holder_token)), - pending_bind_callback_(on_bind_callback) { +ViewHolder::ViewHolder(fuchsia::ui::views::ViewHolderToken view_holder_token, + ViewIdCallback on_view_created) + : pending_view_holder_token_(std::move(view_holder_token)), + on_view_created_(std::move(on_view_created)) { FML_DCHECK(pending_view_holder_token_.value); } @@ -114,12 +115,13 @@ void ViewHolder::UpdateScene(scenic::Session* session, opacity_node_->AddChild(*entity_node_); opacity_node_->SetLabel("flutter::ViewHolder"); entity_node_->Attach(*view_holder_); - if (ui_task_runner_ && pending_view_holder_token_.value) { - ui_task_runner_->PostTask( - [bind_callback = std::move(pending_bind_callback_), - view_holder_id = view_holder_->id()]() { - bind_callback(view_holder_id); - }); + + // Inform the rest of Flutter about the view being created. + // As long as we do this before calling `Present` on the session, + // View-related messages sent to the UI thread will never be processed + // before this internal message is delivered to the UI thread. + if (on_view_created_) { + on_view_created_(view_holder_->id()); } } FML_DCHECK(entity_node_); diff --git a/flow/view_holder.h b/flow/view_holder.h index f25b205c7..2f11c1f08 100644 --- a/flow/view_holder.h +++ b/flow/view_holder.h @@ -29,18 +29,20 @@ namespace flutter { // This object is created and destroyed on the |Rasterizer|'s thread. class ViewHolder { public: - using BindCallback = std::function; - + using ViewIdCallback = std::function; + + // `Create`, `Destroy`, and `FromId` must be executed on the raster thread or + // errors will occur. + // + // `on_bind_callback` and `on_unbind_callback` will likewise execute on the + // raster thread; clients are responsible for re-threading the callback if + // needed. static void Create(zx_koid_t id, - fml::RefPtr ui_task_runner, - fuchsia::ui::views::ViewHolderToken view_holder_token, - const BindCallback& on_bind_callback); - static void Destroy(zx_koid_t id); + ViewIdCallback on_view_created, + fuchsia::ui::views::ViewHolderToken view_holder_token); + static void Destroy(zx_koid_t id, ViewIdCallback on_view_destroyed); static ViewHolder* FromId(zx_koid_t id); - ViewHolder(fml::RefPtr ui_task_runner, - fuchsia::ui::views::ViewHolderToken view_holder_token, - const BindCallback& on_bind_callback); ~ViewHolder() = default; // Sets the properties/opacity of the child view by issuing a Scenic command. @@ -68,18 +70,20 @@ class ViewHolder { void set_focusable(bool value) { focusable_ = value; } private: - fml::RefPtr ui_task_runner_; + ViewHolder(fuchsia::ui::views::ViewHolderToken view_holder_token, + ViewIdCallback on_view_created); std::unique_ptr entity_node_; std::unique_ptr opacity_node_; std::unique_ptr view_holder_; fuchsia::ui::views::ViewHolderToken pending_view_holder_token_; - BindCallback pending_bind_callback_; bool hit_testable_ = true; bool focusable_ = true; + ViewIdCallback on_view_created_; + fuchsia::ui::gfx::ViewProperties pending_properties_; bool has_pending_properties_ = false; diff --git a/lib/ui/compositing/scene_host.cc b/lib/ui/compositing/scene_host.cc index 25cd5525e..81d4e0594 100644 --- a/lib/ui/compositing/scene_host.cc +++ b/lib/ui/compositing/scene_host.cc @@ -19,24 +19,26 @@ namespace { struct SceneHostBindingKey { std::string isolate_service_id; - zx_koid_t koid; + scenic::ResourceId resource_id; - SceneHostBindingKey(const zx_koid_t koid, - const std::string isolate_service_id) { - this->koid = koid; + SceneHostBindingKey(const std::string isolate_service_id, + scenic::ResourceId resource_id) { this->isolate_service_id = isolate_service_id; + this->resource_id = resource_id; } bool operator==(const SceneHostBindingKey& other) const { - return isolate_service_id == other.isolate_service_id && koid == other.koid; + return isolate_service_id == other.isolate_service_id && + resource_id == other.resource_id; } }; struct SceneHostBindingKeyHasher { std::size_t operator()(const SceneHostBindingKey& key) const { - std::size_t koid_hash = std::hash()(key.koid); std::size_t isolate_hash = std::hash()(key.isolate_service_id); - return koid_hash ^ isolate_hash; + std::size_t resource_id_hash = + std::hash()(key.resource_id); + return isolate_hash ^ resource_id_hash; } }; @@ -54,7 +56,7 @@ void SceneHost_constructor(Dart_NativeArguments args) { flutter::SceneHost* GetSceneHost(scenic::ResourceId id, std::string isolate_service_id) { auto binding = - scene_host_bindings.find(SceneHostBindingKey(id, isolate_service_id)); + scene_host_bindings.find(SceneHostBindingKey(isolate_service_id, id)); if (binding == scene_host_bindings.end()) { return nullptr; } else { @@ -165,7 +167,8 @@ SceneHost::SceneHost(fml::RefPtr viewHolderToken, Dart_Handle viewStateChangedCallback) : raster_task_runner_( UIDartState::Current()->GetTaskRunners().GetRasterTaskRunner()), - koid_(GetKoid(viewHolderToken->handle())) { + koid_(GetKoid(viewHolderToken->handle())), + weak_factory_(this) { auto dart_state = UIDartState::Current(); isolate_service_id_ = Dart_IsolateServiceId(Dart_CurrentIsolate()); @@ -180,34 +183,47 @@ SceneHost::SceneHost(fml::RefPtr viewHolderToken, view_state_changed_callback_.Set(dart_state, viewStateChangedCallback); } - // This callback will be posted as a task when the |scenic::ViewHolder| - // resource is created and given an id by the raster thread. - auto bind_callback = [scene_host = this, - isolate_service_id = - isolate_service_id_](scenic::ResourceId id) { - const auto key = SceneHostBindingKey(id, isolate_service_id); - scene_host_bindings.emplace(std::make_pair(key, scene_host)); + // This callback is invoked on the raster thread when the |scenic::ViewHolder| + // resource is created and given an id. + auto bind_callback = [weak = weak_factory_.GetWeakPtr(), + ui_task_runner = + dart_state->GetTaskRunners().GetUITaskRunner()]( + scenic::ResourceId id) { + ui_task_runner->PostTask([weak, id]() { + if (!weak) { + FML_LOG(WARNING) << "ViewHolder bound to SceneHost after SceneHost was " + "destroyed; ignoring."; + return; + } + + FML_DCHECK(weak->resource_id_ == 0); + weak->resource_id_ = id; + + const auto key = + SceneHostBindingKey(weak->isolate_service_id_, weak->resource_id_); + scene_host_bindings.emplace(std::make_pair(key, weak.get())); + }); }; // Pass the raw handle to the raster thread; destroying a // |zircon::dart::Handle| on that thread can cause a race condition. - raster_task_runner_->PostTask( - [id = koid_, - ui_task_runner = - UIDartState::Current()->GetTaskRunners().GetUITaskRunner(), - raw_handle = viewHolderToken->ReleaseHandle(), bind_callback]() { - flutter::ViewHolder::Create( - id, std::move(ui_task_runner), - scenic::ToViewHolderToken(zx::eventpair(raw_handle)), - std::move(bind_callback)); - }); + raster_task_runner_->PostTask([koid = koid_, + raw_handle = viewHolderToken->ReleaseHandle(), + bind_callback = std::move(bind_callback)]() { + flutter::ViewHolder::Create( + koid, std::move(bind_callback), + scenic::ToViewHolderToken(zx::eventpair(raw_handle))); + }); } SceneHost::~SceneHost() { - scene_host_bindings.erase(SceneHostBindingKey(koid_, isolate_service_id_)); + if (resource_id_ != 0) { + scene_host_bindings.erase( + SceneHostBindingKey(isolate_service_id_, resource_id_)); + } raster_task_runner_->PostTask( - [id = koid_]() { flutter::ViewHolder::Destroy(id); }); + [koid = koid_]() { flutter::ViewHolder::Destroy(koid, nullptr); }); } void SceneHost::dispose() { diff --git a/lib/ui/compositing/scene_host.h b/lib/ui/compositing/scene_host.h index 9283ad40d..5e76a0a18 100644 --- a/lib/ui/compositing/scene_host.h +++ b/lib/ui/compositing/scene_host.h @@ -12,6 +12,7 @@ #include "dart-pkg/zircon/sdk_ext/handle.h" #include "flutter/fml/memory/ref_counted.h" +#include "flutter/fml/memory/weak_ptr.h" #include "flutter/fml/task_runner.h" #include "flutter/lib/ui/dart_wrapper.h" #include "third_party/tonic/dart_library_natives.h" @@ -59,7 +60,12 @@ class SceneHost : public RefCountedDartWrappable { tonic::DartPersistentValue view_disconnected_callback_; tonic::DartPersistentValue view_state_changed_callback_; std::string isolate_service_id_; + scenic::ResourceId resource_id_ = 0; zx_koid_t koid_ = ZX_KOID_INVALID; + + fml::WeakPtrFactory weak_factory_; + + FML_DISALLOW_COPY_AND_ASSIGN(SceneHost); }; } // namespace flutter diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index cddd5cde4..13018e750 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -176,16 +176,16 @@ Engine::Engine(Delegate& delegate, OnEnableWireframe on_enable_wireframe_callback = std::bind( &Engine::DebugWireframeSettingsChanged, this, std::placeholders::_1); - OnCreateView on_create_view_callback = - std::bind(&Engine::CreateView, this, std::placeholders::_1, - std::placeholders::_2, std::placeholders::_3); + OnCreateView on_create_view_callback = std::bind( + &Engine::CreateView, this, std::placeholders::_1, std::placeholders::_2, + std::placeholders::_3, std::placeholders::_4); OnUpdateView on_update_view_callback = std::bind(&Engine::UpdateView, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3); - OnDestroyView on_destroy_view_callback = - std::bind(&Engine::DestroyView, this, std::placeholders::_1); + OnDestroyView on_destroy_view_callback = std::bind( + &Engine::DestroyView, this, std::placeholders::_1, std::placeholders::_2); OnCreateSurface on_create_surface_callback = std::bind(&Engine::CreateSurface, this); @@ -580,21 +580,26 @@ void Engine::DebugWireframeSettingsChanged(bool enabled) { }); } -void Engine::CreateView(int64_t view_id, bool hit_testable, bool focusable) { +void Engine::CreateView(int64_t view_id, + ViewIdCallback on_view_bound, + bool hit_testable, + bool focusable) { FML_CHECK(shell_); shell_->GetTaskRunners().GetRasterTaskRunner()->PostTask( - [this, view_id, hit_testable, focusable]() { + [this, view_id, hit_testable, focusable, + on_view_bound = std::move(on_view_bound)]() { #if defined(LEGACY_FUCHSIA_EMBEDDER) if (use_legacy_renderer_) { FML_CHECK(legacy_external_view_embedder_); - legacy_external_view_embedder_->CreateView(view_id, hit_testable, - focusable); + legacy_external_view_embedder_->CreateView( + view_id, std::move(on_view_bound), hit_testable, focusable); } else #endif { FML_CHECK(external_view_embedder_); - external_view_embedder_->CreateView(view_id); + external_view_embedder_->CreateView(view_id, + std::move(on_view_bound)); external_view_embedder_->SetViewProperties(view_id, hit_testable, focusable); } @@ -621,21 +626,24 @@ void Engine::UpdateView(int64_t view_id, bool hit_testable, bool focusable) { }); } -void Engine::DestroyView(int64_t view_id) { +void Engine::DestroyView(int64_t view_id, ViewIdCallback on_view_unbound) { FML_CHECK(shell_); - shell_->GetTaskRunners().GetRasterTaskRunner()->PostTask([this, view_id]() { + shell_->GetTaskRunners().GetRasterTaskRunner()->PostTask( + [this, view_id, on_view_unbound = std::move(on_view_unbound)]() { #if defined(LEGACY_FUCHSIA_EMBEDDER) - if (use_legacy_renderer_) { - FML_CHECK(legacy_external_view_embedder_); - legacy_external_view_embedder_->DestroyView(view_id); - } else + if (use_legacy_renderer_) { + FML_CHECK(legacy_external_view_embedder_); + legacy_external_view_embedder_->DestroyView( + view_id, std::move(on_view_unbound)); + } else #endif - { - FML_CHECK(external_view_embedder_); - external_view_embedder_->DestroyView(view_id); - } - }); + { + FML_CHECK(external_view_embedder_); + external_view_embedder_->DestroyView(view_id, + std::move(on_view_unbound)); + } + }); } std::unique_ptr Engine::CreateSurface() { diff --git a/shell/platform/fuchsia/flutter/engine.h b/shell/platform/fuchsia/flutter/engine.h index fd77f3ff4..c66e32695 100644 --- a/shell/platform/fuchsia/flutter/engine.h +++ b/shell/platform/fuchsia/flutter/engine.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -105,9 +106,12 @@ class Engine final { void Terminate(); void DebugWireframeSettingsChanged(bool enabled); - void CreateView(int64_t view_id, bool hit_testable, bool focusable); + void CreateView(int64_t view_id, + ViewIdCallback on_view_bound, + bool hit_testable, + bool focusable); void UpdateView(int64_t view_id, bool hit_testable, bool focusable); - void DestroyView(int64_t view_id); + void DestroyView(int64_t view_id, ViewIdCallback on_view_unbound); std::shared_ptr GetExternalViewEmbedder(); std::unique_ptr CreateSurface(); diff --git a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc index 8144da0c2..76a8425cf 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc +++ b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.cc @@ -475,7 +475,8 @@ void FuchsiaExternalViewEmbedder::EnableWireframe(bool enable) { session_.Present(); } -void FuchsiaExternalViewEmbedder::CreateView(int64_t view_id) { +void FuchsiaExternalViewEmbedder::CreateView(int64_t view_id, + ViewIdCallback on_view_bound) { FML_DCHECK(scenic_views_.find(view_id) == scenic_views_.end()); ScenicView new_view = { @@ -486,6 +487,7 @@ void FuchsiaExternalViewEmbedder::CreateView(int64_t view_id) { scenic::ToViewHolderToken(zx::eventpair((zx_handle_t)view_id)), "Flutter::PlatformView"), }; + on_view_bound(new_view.view_holder.id()); new_view.opacity_node.SetLabel("flutter::PlatformView::OpacityMutator"); new_view.entity_node.SetLabel("flutter::PlatformView::TransformMutator"); @@ -497,9 +499,14 @@ void FuchsiaExternalViewEmbedder::CreateView(int64_t view_id) { scenic_views_.emplace(std::make_pair(view_id, std::move(new_view))); } -void FuchsiaExternalViewEmbedder::DestroyView(int64_t view_id) { - size_t erased = scenic_views_.erase(view_id); - FML_DCHECK(erased == 1); +void FuchsiaExternalViewEmbedder::DestroyView(int64_t view_id, + ViewIdCallback on_view_unbound) { + auto scenic_view = scenic_views_.find(view_id); + FML_DCHECK(scenic_view != scenic_views_.end()); + scenic::ResourceId resource_id = scenic_view->second.view_holder.id(); + + scenic_views_.erase(scenic_view); + on_view_unbound(resource_id); } void FuchsiaExternalViewEmbedder::SetViewProperties(int64_t view_id, diff --git a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h index 531d936de..6ed35c595 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h @@ -6,6 +6,7 @@ #define FLUTTER_SHELL_PLATFORM_FUCHSIA_FLUTTER_FUCHSIA_EXTERNAL_VIEW_EMBEDDER_H_ #include +#include #include #include @@ -30,6 +31,8 @@ namespace flutter_runner { +using ViewIdCallback = std::function; + // This class orchestrates interaction with the Scenic compositor on Fuchsia. It // ensures that flutter content and platform view content are both rendered // correctly in a unified scene. @@ -89,8 +92,8 @@ class FuchsiaExternalViewEmbedder final : public flutter::ExternalViewEmbedder { // |SetViewProperties| doesn't manipulate the view directly -- it sets pending // properties for the next |UpdateView| call. void EnableWireframe(bool enable); - void CreateView(int64_t view_id); - void DestroyView(int64_t view_id); + void CreateView(int64_t view_id, ViewIdCallback on_view_bound); + void DestroyView(int64_t view_id, ViewIdCallback on_view_unbound); void SetViewProperties(int64_t view_id, bool hit_testable, bool focusable); private: diff --git a/shell/platform/fuchsia/flutter/platform_view.cc b/shell/platform/fuchsia/flutter/platform_view.cc index 7f45dee9c..66a276144 100644 --- a/shell/platform/fuchsia/flutter/platform_view.cc +++ b/shell/platform/fuchsia/flutter/platform_view.cc @@ -13,6 +13,7 @@ #include #include "flutter/fml/logging.h" +#include "flutter/fml/make_copyable.h" #include "flutter/lib/ui/window/pointer_data.h" #include "flutter/lib/ui/window/window.h" #include "flutter/shell/platform/common/client_wrapper/include/flutter/encodable_value.h" @@ -90,7 +91,8 @@ PlatformView::PlatformView( ime_client_(this), vsync_offset_(std::move(vsync_offset)), vsync_event_handle_(vsync_event_handle), - keyboard_listener_binding_(this, std::move(keyboard_listener_request)) { + keyboard_listener_binding_(this, std::move(keyboard_listener_request)), + weak_factory_(this) { // Register all error handlers. SetInterfaceErrorHandler(session_listener_binding_, "SessionListener"); SetInterfaceErrorHandler(ime_, "Input Method Editor"); @@ -220,8 +222,9 @@ void PlatformView::OnScenicEvent( std::vector events) { TRACE_EVENT0("flutter", "PlatformView::OnScenicEvent"); + std::vector deferred_view_events; bool metrics_changed = false; - for (const auto& event : events) { + for (auto& event : events) { switch (event.Which()) { case fuchsia::ui::scenic::Event::Tag::kGfx: switch (event.gfx().Which()) { @@ -278,16 +281,46 @@ void PlatformView::OnScenicEvent( break; } case fuchsia::ui::gfx::Event::Tag::kViewConnected: - OnChildViewConnected(event.gfx().view_connected().view_holder_id); +#if defined(LEGACY_FUCHSIA_EMBEDDER) + task_runners_.GetUITaskRunner()->PostTask( + [view_holder_id = + event.gfx().view_connected().view_holder_id]() { + flutter::SceneHost::OnViewConnected(view_holder_id); + }); +#endif // LEGACY_FUCHSIA_EMBEDDER + if (!OnChildViewConnected( + event.gfx().view_connected().view_holder_id)) { + deferred_view_events.push_back(std::move(event.gfx())); + } break; case fuchsia::ui::gfx::Event::Tag::kViewDisconnected: - OnChildViewDisconnected( - event.gfx().view_disconnected().view_holder_id); +#if defined(LEGACY_FUCHSIA_EMBEDDER) + task_runners_.GetUITaskRunner()->PostTask( + [view_holder_id = + event.gfx().view_disconnected().view_holder_id]() { + flutter::SceneHost::OnViewDisconnected(view_holder_id); + }); +#endif // LEGACY_FUCHSIA_EMBEDDER + if (!OnChildViewDisconnected( + event.gfx().view_disconnected().view_holder_id)) { + deferred_view_events.push_back(std::move(event.gfx())); + } break; case fuchsia::ui::gfx::Event::Tag::kViewStateChanged: - OnChildViewStateChanged( - event.gfx().view_state_changed().view_holder_id, - event.gfx().view_state_changed().state.is_rendering); +#if defined(LEGACY_FUCHSIA_EMBEDDER) + task_runners_.GetUITaskRunner()->PostTask( + [view_holder_id = + event.gfx().view_state_changed().view_holder_id, + state = + event.gfx().view_state_changed().state.is_rendering]() { + flutter::SceneHost::OnViewStateChanged(view_holder_id, state); + }); +#endif // LEGACY_FUCHSIA_EMBEDDER + if (!OnChildViewStateChanged( + event.gfx().view_state_changed().view_holder_id, + event.gfx().view_state_changed().state.is_rendering)) { + deferred_view_events.push_back(std::move(event.gfx())); + } break; case fuchsia::ui::gfx::Event::Tag::Invalid: FML_DCHECK(false) << "Flutter PlatformView::OnScenicEvent: Got " @@ -327,6 +360,50 @@ void PlatformView::OnScenicEvent( } } + // If some View events went unmatched, try processing them again one more time + // in case they arrived out-of-order with the View creation callback. + if (!deferred_view_events.empty()) { + task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable( + [weak = weak_factory_.GetWeakPtr(), + deferred_view_events = std::move(deferred_view_events)]() { + if (!weak) { + FML_LOG(WARNING) + << "PlatformView already destroyed when " + "processing deferred view events; dropping events."; + return; + } + + for (const auto& event : deferred_view_events) { + switch (event.Which()) { + case fuchsia::ui::gfx::Event::Tag::kViewConnected: { + bool view_found = weak->OnChildViewConnected( + event.view_connected().view_holder_id); + FML_DCHECK(view_found); + break; + } + case fuchsia::ui::gfx::Event::Tag::kViewDisconnected: { + bool view_found = weak->OnChildViewDisconnected( + event.view_disconnected().view_holder_id); + FML_DCHECK(view_found); + break; + } + case fuchsia::ui::gfx::Event::Tag::kViewStateChanged: { + bool view_found = weak->OnChildViewStateChanged( + event.view_state_changed().view_holder_id, + event.view_state_changed().state.is_rendering); + FML_DCHECK(view_found); + break; + } + default: + FML_DCHECK(false) << "Flutter PlatformView::OnScenicEvent: Got " + "an invalid deferred GFX event."; + break; + } + } + })); + } + + // If any of the viewport metrics changed, inform the engine now. if (view_pixel_ratio_.has_value() && view_logical_size_.has_value() && metrics_changed) { const float pixel_ratio = *view_pixel_ratio_; @@ -351,47 +428,70 @@ void PlatformView::OnScenicEvent( } } -void PlatformView::OnChildViewConnected(scenic::ResourceId view_holder_id) { -#if defined(LEGACY_FUCHSIA_EMBEDDER) - task_runners_.GetUITaskRunner()->PostTask([view_holder_id]() { - flutter::SceneHost::OnViewConnected(view_holder_id); - }); -#endif // LEGACY_FUCHSIA_EMBEDDER - std::string call = "{\"method\":\"View.viewConnected\",\"args\":null}"; +bool PlatformView::OnChildViewConnected(scenic::ResourceId view_holder_id) { + auto view_id_mapping = child_view_ids_.find(view_holder_id); + if (view_id_mapping == child_view_ids_.end()) { + return false; + } + + std::ostringstream out; + out << "{" + << "\"method\":\"View.viewConnected\"," + << "\"args\":{" + << " \"viewId\":" << view_id_mapping->second // ViewHolderToken handle + << " }" + << "}"; + auto call = out.str(); fml::RefPtr message = fml::MakeRefCounted( "flutter/platform_views", std::vector(call.begin(), call.end()), nullptr); DispatchPlatformMessage(message); + + return true; } -void PlatformView::OnChildViewDisconnected(scenic::ResourceId view_holder_id) { -#if defined(LEGACY_FUCHSIA_EMBEDDER) - task_runners_.GetUITaskRunner()->PostTask([view_holder_id]() { - flutter::SceneHost::OnViewDisconnected(view_holder_id); - }); -#endif // LEGACY_FUCHSIA_EMBEDDER - std::string call = "{\"method\":\"View.viewDisconnected\",\"args\":null}"; +bool PlatformView::OnChildViewDisconnected(scenic::ResourceId view_holder_id) { + auto view_id_mapping = child_view_ids_.find(view_holder_id); + if (view_id_mapping == child_view_ids_.end()) { + return false; + } + + std::ostringstream out; + out << "{" + << "\"method\":\"View.viewDisconnected\"," + << "\"args\":{" + << " \"viewId\":" << view_id_mapping->second // ViewHolderToken handle + << " }" + << "}"; + auto call = out.str(); fml::RefPtr message = fml::MakeRefCounted( "flutter/platform_views", std::vector(call.begin(), call.end()), nullptr); DispatchPlatformMessage(message); + + return true; } -void PlatformView::OnChildViewStateChanged(scenic::ResourceId view_holder_id, - bool state) { -#if defined(LEGACY_FUCHSIA_EMBEDDER) - task_runners_.GetUITaskRunner()->PostTask([view_holder_id, state]() { - flutter::SceneHost::OnViewStateChanged(view_holder_id, state); - }); -#endif // LEGACY_FUCHSIA_EMBEDDER +bool PlatformView::OnChildViewStateChanged(scenic::ResourceId view_holder_id, + bool is_rendering) { + auto view_id_mapping = child_view_ids_.find(view_holder_id); + if (view_id_mapping == child_view_ids_.end()) { + return false; + } + std::ostringstream out; - std::string str_state = state ? "true" : "false"; - out << "{\"method\":\"View.viewStateChanged\",\"args\":{\"state\":" - << str_state << "}}"; + out << "{" + << "\"method\":\"View.viewStateChanged\"," + << "\"args\":{" + << " \"viewId\":" << view_id_mapping->second // ViewHolderToken handle + << " \"is_rendering\":" << is_rendering // IsViewRendering + << " \"state\":" << is_rendering // IsViewRendering + << " }" + << "}"; auto call = out.str(); fml::RefPtr message = @@ -399,6 +499,8 @@ void PlatformView::OnChildViewStateChanged(scenic::ResourceId view_holder_id, "flutter/platform_views", std::vector(call.begin(), call.end()), nullptr); DispatchPlatformMessage(message); + + return true; } static flutter::PointerData::Change GetChangeFromPointerEventPhase( @@ -850,9 +952,27 @@ void PlatformView::HandleFlutterPlatformViewsChannelPlatformMessage( return; } - on_create_view_callback_(view_id->value.GetUint64(), + const int64_t view_id_raw = view_id->value.GetUint64(); + auto on_view_bound = + [weak = weak_factory_.GetWeakPtr(), + platform_task_runner = task_runners_.GetPlatformTaskRunner(), + view_id = view_id_raw](scenic::ResourceId resource_id) { + platform_task_runner->PostTask([weak, view_id, resource_id]() { + if (!weak) { + FML_LOG(WARNING) + << "ViewHolder bound to PlatformView after PlatformView was " + "destroyed; ignoring."; + return; + } + + FML_DCHECK(weak->child_view_ids_.count(resource_id) == 0); + weak->child_view_ids_[resource_id] = view_id; + }); + }; + on_create_view_callback_(view_id_raw, std::move(on_view_bound), hit_testable->value.GetBool(), focusable->value.GetBool()); + // The client is waiting for view creation. Send an empty response back // to signal the view was created. if (message->response().get()) { @@ -901,7 +1021,25 @@ void PlatformView::HandleFlutterPlatformViewsChannelPlatformMessage( FML_LOG(ERROR) << "Argument 'viewId' is not a int64"; return; } - on_destroy_view_callback_(view_id->value.GetUint64()); + + const int64_t view_id_raw = view_id->value.GetUint64(); + auto on_view_unbound = + [weak = weak_factory_.GetWeakPtr(), + platform_task_runner = task_runners_.GetPlatformTaskRunner()]( + scenic::ResourceId resource_id) { + platform_task_runner->PostTask([weak, resource_id]() { + if (!weak) { + FML_LOG(WARNING) + << "ViewHolder unbound from PlatformView after PlatformView" + "was destroyed; ignoring."; + return; + } + + FML_DCHECK(weak->child_view_ids_.count(resource_id) == 1); + weak->child_view_ids_.erase(resource_id); + }); + }; + on_destroy_view_callback_(view_id_raw, std::move(on_view_unbound)); } else if (method->value == "View.requestFocus") { auto args_it = root.FindMember("args"); if (args_it == root.MemberEnd() || !args_it->value.IsObject()) { diff --git a/shell/platform/fuchsia/flutter/platform_view.h b/shell/platform/fuchsia/flutter/platform_view.h index 4d7bf6d76..6cf1c3456 100644 --- a/shell/platform/fuchsia/flutter/platform_view.h +++ b/shell/platform/fuchsia/flutter/platform_view.h @@ -14,9 +14,11 @@ #include #include +#include #include "flow/embedded_views.h" #include "flutter/fml/macros.h" +#include "flutter/fml/memory/weak_ptr.h" #include "flutter/fml/time/time_delta.h" #include "flutter/shell/common/platform_view.h" #include "flutter/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h" @@ -27,9 +29,9 @@ namespace flutter_runner { using OnEnableWireframe = fit::function; -using OnCreateView = fit::function; +using OnCreateView = fit::function; using OnUpdateView = fit::function; -using OnDestroyView = fit::function; +using OnDestroyView = fit::function; using OnCreateSurface = fit::function()>; // PlatformView is the per-engine component residing on the platform thread that @@ -107,9 +109,13 @@ class PlatformView final : public flutter::PlatformView, void OnScenicError(std::string error) override; void OnScenicEvent(std::vector events) override; - void OnChildViewConnected(scenic::ResourceId view_holder_id); - void OnChildViewDisconnected(scenic::ResourceId view_holder_id); - void OnChildViewStateChanged(scenic::ResourceId view_holder_id, bool state); + // ViewHolder event handlers. These return false if the ViewHolder + // corresponding to `view_holder_id` could not be found and the evnt was + // unhandled. + bool OnChildViewConnected(scenic::ResourceId view_holder_id); + bool OnChildViewDisconnected(scenic::ResourceId view_holder_id); + bool OnChildViewStateChanged(scenic::ResourceId view_holder_id, + bool is_rendering); bool OnHandlePointerEvent(const fuchsia::ui::input::PointerEvent& pointer); @@ -191,6 +197,10 @@ class PlatformView final : public flutter::PlatformView, fuchsia::sys::ServiceProviderPtr parent_environment_service_provider_; + // child_view_ids_ maintains a persistent mapping from Scenic ResourceId's to + // flutter view ids, which are really zx_handle_t of ViewHolderToken. + std::unordered_map child_view_ids_; + // last_text_state_ is the last state of the text input as reported by the IME // or initialized by Flutter. We set it to null if Flutter doesn't want any // input, since then there is no text input state at all. @@ -217,6 +227,8 @@ class PlatformView final : public flutter::PlatformView, // The keyboard translation for fuchsia.ui.input3.KeyEvent. Keyboard keyboard_; + fml::WeakPtrFactory weak_factory_; // Must be the last member. + FML_DISALLOW_COPY_AND_ASSIGN(PlatformView); }; diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index b4bc3464b..3873081dd 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -59,8 +59,17 @@ class MockExternalViewEmbedder : public flutter::ExternalViewEmbedder { class MockPlatformViewDelegate : public flutter::PlatformView::Delegate { public: + void Reset() { + message_ = nullptr; + metrics_ = flutter::ViewportMetrics{}; + semantics_features_ = 0; + semantics_enabled_ = false; + } + // |flutter::PlatformView::Delegate| void OnPlatformViewCreated(std::unique_ptr surface) { + ASSERT_EQ(surface_.get(), nullptr); + surface_ = std::move(surface); } // |flutter::PlatformView::Delegate| @@ -547,7 +556,7 @@ TEST_F(PlatformViewTests, EnableWireframeTest) { // Cast platform_view to its base view so we can have access to the public // "HandlePlatformMessage" function. - auto base_view = dynamic_cast(&platform_view); + auto base_view = static_cast(&platform_view); EXPECT_TRUE(base_view); // JSON for the message to be passed into the PlatformView. @@ -577,15 +586,25 @@ TEST_F(PlatformViewTests, CreateViewTest) { sys::testing::ServiceDirectoryProvider services_provider(dispatcher()); MockPlatformViewDelegate delegate; flutter::TaskRunners task_runners = - flutter::TaskRunners("test_runners", nullptr, nullptr, nullptr, nullptr); + flutter::TaskRunners("test_runners", // label + flutter_runner::CreateFMLTaskRunner( + async_get_default_dispatcher()), // platform + nullptr, // raster + nullptr, // ui + nullptr // io + ); // Test wireframe callback function. If the message sent to the platform // view was properly handled and parsed, this function should be called, // setting |wireframe_enabled| to true. int64_t create_view_called = false; auto CreateViewCallback = [&create_view_called]( - int64_t view_id, bool hit_testable, - bool focusable) { create_view_called = true; }; + int64_t view_id, + flutter_runner::ViewIdCallback on_view_bound, + bool hit_testable, bool focusable) { + create_view_called = true; + on_view_bound(0); + }; flutter_runner::PlatformView platform_view = PlatformViewBuilder(delegate, std::move(task_runners), @@ -595,7 +614,7 @@ TEST_F(PlatformViewTests, CreateViewTest) { // Cast platform_view to its base view so we can have access to the public // "HandlePlatformMessage" function. - auto base_view = dynamic_cast(&platform_view); + auto base_view = static_cast(&platform_view); EXPECT_TRUE(base_view); // JSON for the message to be passed into the PlatformView. @@ -645,7 +664,7 @@ TEST_F(PlatformViewTests, UpdateViewTest) { // Cast platform_view to its base view so we can have access to the public // "HandlePlatformMessage" function. - auto base_view = dynamic_cast(&platform_view); + auto base_view = static_cast(&platform_view); EXPECT_TRUE(base_view); // JSON for the message to be passed into the PlatformView. @@ -677,15 +696,24 @@ TEST_F(PlatformViewTests, DestroyViewTest) { sys::testing::ServiceDirectoryProvider services_provider(dispatcher()); MockPlatformViewDelegate delegate; flutter::TaskRunners task_runners = - flutter::TaskRunners("test_runners", nullptr, nullptr, nullptr, nullptr); + flutter::TaskRunners("test_runners", // label + flutter_runner::CreateFMLTaskRunner( + async_get_default_dispatcher()), // platform + nullptr, // raster + nullptr, // ui + nullptr // io + ); // Test wireframe callback function. If the message sent to the platform // view was properly handled and parsed, this function should be called, // setting |wireframe_enabled| to true. int64_t destroy_view_called = false; - auto DestroyViewCallback = [&destroy_view_called](int64_t view_id) { - destroy_view_called = true; - }; + auto DestroyViewCallback = + [&destroy_view_called](int64_t view_id, + flutter_runner::ViewIdCallback on_view_unbound) { + destroy_view_called = true; + on_view_unbound(0); + }; flutter_runner::PlatformView platform_view = PlatformViewBuilder(delegate, std::move(task_runners), @@ -695,7 +723,7 @@ TEST_F(PlatformViewTests, DestroyViewTest) { // Cast platform_view to its base view so we can have access to the public // "HandlePlatformMessage" function. - auto base_view = dynamic_cast(&platform_view); + auto base_view = static_cast(&platform_view); EXPECT_TRUE(base_view); // JSON for the message to be passed into the PlatformView. @@ -723,61 +751,116 @@ TEST_F(PlatformViewTests, DestroyViewTest) { // "flutter/platform_views" channel for ViewConnected, ViewDisconnected, and // ViewStateChanged events. TEST_F(PlatformViewTests, ViewEventsTest) { + constexpr int64_t kViewId = 33; + constexpr scenic::ResourceId kViewHolderId = 42; MockPlatformViewDelegate delegate; fuchsia::ui::scenic::SessionListenerPtr session_listener; std::vector events; sys::testing::ServiceDirectoryProvider services_provider(dispatcher()); + flutter::TaskRunners task_runners = + flutter::TaskRunners("test_runners", // label + flutter_runner::CreateFMLTaskRunner( + async_get_default_dispatcher()), // platform + flutter_runner::CreateFMLTaskRunner( + async_get_default_dispatcher()), // raster + flutter_runner::CreateFMLTaskRunner( + async_get_default_dispatcher()), // ui + nullptr // io + ); - flutter::TaskRunners task_runners = flutter::TaskRunners( - "test_runners", nullptr, nullptr, - flutter_runner::CreateFMLTaskRunner(async_get_default_dispatcher()), - nullptr); + auto on_create_view = [kViewId](int64_t view_id, + flutter_runner::ViewIdCallback on_view_bound, + bool hit_testable, bool focusable) { + ASSERT_EQ(view_id, kViewId); + on_view_bound(kViewHolderId); + }; flutter_runner::PlatformView platform_view = PlatformViewBuilder(delegate, std::move(task_runners), services_provider.service_directory()) .SetSessionListenerRequest(session_listener.NewRequest()) + .SetCreateViewCallback(on_create_view) .Build(); - + RunLoopUntilIdle(); + ASSERT_EQ(delegate.message(), nullptr); + + // Create initial view for testing. + std::ostringstream create_view_message; + create_view_message << "{" + << " \"method\":\"View.create\"," + << " \"args\":{" + << " \"viewId\":" << kViewId << "," + << " \"hitTestable\":true," + << " \"focusable\":true" + << " }" + << "}"; + std::string create_view_call = create_view_message.str(); + static_cast(&platform_view) + ->HandlePlatformMessage(fml::MakeRefCounted( + "flutter/platform_views", + std::vector(create_view_call.begin(), + create_view_call.end()), + fml::RefPtr())); RunLoopUntilIdle(); // ViewConnected event. + delegate.Reset(); events.clear(); events.emplace_back(fuchsia::ui::scenic::Event::WithGfx( fuchsia::ui::gfx::Event::WithViewConnected( fuchsia::ui::gfx::ViewConnectedEvent{ - .view_holder_id = 0, + .view_holder_id = kViewHolderId, }))); session_listener->OnScenicEvent(std::move(events)); RunLoopUntilIdle(); - auto data = delegate.message()->data(); - auto call = std::string(data.begin(), data.end()); - std::string expected = "{\"method\":\"View.viewConnected\",\"args\":null}"; - EXPECT_EQ(expected, call); + flutter::PlatformMessage* view_connected_msg = delegate.message(); + ASSERT_NE(view_connected_msg, nullptr); + std::ostringstream view_connected_expected_out; + view_connected_expected_out + << "{" + << "\"method\":\"View.viewConnected\"," + << "\"args\":{" + << " \"viewId\":" << kViewId // ViewHolderToken handle + << " }" + << "}"; + EXPECT_EQ(view_connected_expected_out.str(), + std::string(view_connected_msg->data().begin(), + view_connected_msg->data().end())); // ViewDisconnected event. + delegate.Reset(); events.clear(); events.emplace_back(fuchsia::ui::scenic::Event::WithGfx( fuchsia::ui::gfx::Event::WithViewDisconnected( fuchsia::ui::gfx::ViewDisconnectedEvent{ - .view_holder_id = 0, + .view_holder_id = kViewHolderId, }))); session_listener->OnScenicEvent(std::move(events)); RunLoopUntilIdle(); - data = delegate.message()->data(); - call = std::string(data.begin(), data.end()); - expected = "{\"method\":\"View.viewDisconnected\",\"args\":null}"; - EXPECT_EQ(expected, call); + flutter::PlatformMessage* view_disconnected_msg = delegate.message(); + ASSERT_NE(view_disconnected_msg, nullptr); + std::ostringstream view_disconnected_expected_out; + view_disconnected_expected_out + << "{" + << "\"method\":\"View.viewDisconnected\"," + << "\"args\":{" + << " \"viewId\":" << kViewId // ViewHolderToken handle + << " }" + << "}"; + EXPECT_EQ(view_disconnected_expected_out.str(), + std::string(view_disconnected_msg->data().begin(), + view_disconnected_msg->data().end())); // ViewStateChanged event. + delegate.Reset(); events.clear(); events.emplace_back(fuchsia::ui::scenic::Event::WithGfx( fuchsia::ui::gfx::Event::WithViewStateChanged( fuchsia::ui::gfx::ViewStateChangedEvent{ - .view_holder_id = 0, + .view_holder_id = kViewHolderId, .state = fuchsia::ui::gfx::ViewState{ .is_rendering = true, @@ -786,10 +869,21 @@ TEST_F(PlatformViewTests, ViewEventsTest) { session_listener->OnScenicEvent(std::move(events)); RunLoopUntilIdle(); - data = delegate.message()->data(); - call = std::string(data.begin(), data.end()); - expected = "{\"method\":\"View.viewStateChanged\",\"args\":{\"state\":true}}"; - EXPECT_EQ(expected, call); + flutter::PlatformMessage* view_state_changed_msg = delegate.message(); + ASSERT_NE(view_state_changed_msg, nullptr); + std::ostringstream view_state_changed_expected_out; + view_state_changed_expected_out + << "{" + << "\"method\":\"View.viewStateChanged\"," + << "\"args\":{" + << " \"viewId\":" << kViewId // ViewHolderToken handle + << " \"is_rendering\":" << true // IsViewRendering + << " \"state\":" << true // IsViewRendering + << " }" + << "}"; + EXPECT_EQ(view_state_changed_expected_out.str(), + std::string(view_state_changed_msg->data().begin(), + view_state_changed_msg->data().end())); } // This test makes sure that the PlatformView forwards messages on the @@ -812,7 +906,7 @@ TEST_F(PlatformViewTests, RequestFocusTest) { // Cast platform_view to its base view so we can have access to the public // "HandlePlatformMessage" function. - auto base_view = dynamic_cast(&platform_view); + auto base_view = static_cast(&platform_view); EXPECT_TRUE(base_view); // This "Mock" ViewRef serves as the target for the RequestFocus operation. @@ -875,7 +969,7 @@ TEST_F(PlatformViewTests, RequestFocusFailTest) { // Cast platform_view to its base view so we can have access to the public // "HandlePlatformMessage" function. - auto base_view = dynamic_cast(&platform_view); + auto base_view = static_cast(&platform_view); EXPECT_TRUE(base_view); // This "Mock" ViewRef serves as the target for the RequestFocus operation. -- GitLab