From 5850d41dd04976b7eb40309c2a5a37d9fcf7cd2c Mon Sep 17 00:00:00 2001 From: ColdPaleLight <31977171+ColdPaleLight@users.noreply.github.com> Date: Fri, 25 Mar 2022 05:25:03 +0800 Subject: [PATCH] Make sure that 'BeginFrame' and 'EndFrame' of the 'ExternalViewEmbedder' are paired (flutter/engine#32058) --- engine/src/flutter/flow/embedded_views.h | 13 +++++ engine/src/flutter/shell/common/rasterizer.cc | 9 +++- .../shell/common/rasterizer_unittests.cc | 54 +++++++++++++++++++ .../flutter/shell/common/shell_unittests.cc | 4 ++ 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/flow/embedded_views.h b/engine/src/flutter/flow/embedded_views.h index f62e4e86fe4..7a5a34f3306 100644 --- a/engine/src/flutter/flow/embedded_views.h +++ b/engine/src/flutter/flow/embedded_views.h @@ -335,6 +335,19 @@ class ExternalViewEmbedder { // embedder. virtual void Teardown(); + // Change the flag about whether it is used in this frame, it will be set to + // true when 'BeginFrame' and false when 'EndFrame'. + void SetUsedThisFrame(bool used_this_frame) { + used_this_frame_ = used_this_frame; + } + + // Whether it is used in this frame, returns true between 'BeginFrame' and + // 'EndFrame', otherwise returns false. + bool GetUsedThisFrame() const { return used_this_frame_; } + + private: + bool used_this_frame_ = false; + FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); }; // ExternalViewEmbedder diff --git a/engine/src/flutter/shell/common/rasterizer.cc b/engine/src/flutter/shell/common/rasterizer.cc index d19646c810a..c01b2978cd7 100644 --- a/engine/src/flutter/shell/common/rasterizer.cc +++ b/engine/src/flutter/shell/common/rasterizer.cc @@ -151,10 +151,11 @@ void Rasterizer::DrawLastLayerTree( DrawToSurface(*frame_timings_recorder, *last_layer_tree_); // EndFrame should perform cleanups for the external_view_embedder. - if (external_view_embedder_) { + if (external_view_embedder_ && external_view_embedder_->GetUsedThisFrame()) { bool should_resubmit_frame = ShouldResubmitFrame(raster_status); external_view_embedder_->EndFrame(should_resubmit_frame, raster_thread_merger_); + external_view_embedder_->SetUsedThisFrame(false); } } @@ -208,9 +209,11 @@ RasterStatus Rasterizer::Draw( } // EndFrame should perform cleanups for the external_view_embedder. - if (surface_ && external_view_embedder_) { + if (surface_ && external_view_embedder_ && + external_view_embedder_->GetUsedThisFrame()) { external_view_embedder_->EndFrame(should_resubmit_frame, raster_thread_merger_); + external_view_embedder_->SetUsedThisFrame(false); } // Consume as many pipeline items as possible. But yield the event loop @@ -521,6 +524,8 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe( SkCanvas* embedder_root_canvas = nullptr; if (external_view_embedder_) { + FML_DCHECK(!external_view_embedder_->GetUsedThisFrame()); + external_view_embedder_->SetUsedThisFrame(true); external_view_embedder_->BeginFrame( layer_tree.frame_size(), surface_->GetContext(), layer_tree.device_pixel_ratio(), raster_thread_merger_); diff --git a/engine/src/flutter/shell/common/rasterizer_unittests.cc b/engine/src/flutter/shell/common/rasterizer_unittests.cc index 06418cf9685..b626bd386bf 100644 --- a/engine/src/flutter/shell/common/rasterizer_unittests.cc +++ b/engine/src/flutter/shell/common/rasterizer_unittests.cc @@ -419,6 +419,60 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNoSurfaceIsSet) { latch.Wait(); } +TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNotUsedThisFrame) { + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + ThreadHost thread_host("io.flutter.test." + test_name + ".", + ThreadHost::Type::Platform | ThreadHost::Type::RASTER | + ThreadHost::Type::IO | ThreadHost::Type::UI); + TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), + thread_host.ui_thread->GetTaskRunner(), + thread_host.io_thread->GetTaskRunner()); + MockDelegate delegate; + EXPECT_CALL(delegate, GetTaskRunners()) + .WillRepeatedly(ReturnRef(task_runners)); + + auto rasterizer = std::make_unique(delegate); + auto surface = std::make_unique(); + EXPECT_CALL(*surface, MakeRenderContextCurrent()) + .WillOnce(Return(ByMove(std::make_unique(true)))); + + std::shared_ptr external_view_embedder = + std::make_shared(); + rasterizer->SetExternalViewEmbedder(external_view_embedder); + rasterizer->Setup(std::move(surface)); + + EXPECT_CALL(*external_view_embedder, + BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr, + /*device_pixel_ratio=*/2.0, + /*raster_thread_merger=*/_)) + .Times(0); + EXPECT_CALL( + *external_view_embedder, + EndFrame(/*should_resubmit_frame=*/false, + /*raster_thread_merger=*/fml::RefPtr( + nullptr))) + .Times(0); + + fml::AutoResetWaitableEvent latch; + thread_host.raster_thread->GetTaskRunner()->PostTask([&] { + auto pipeline = std::make_shared>(/*depth=*/10); + auto layer_tree = std::make_unique(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0f); + PipelineProduceResult result = + pipeline->Produce().Complete(std::move(layer_tree)); + EXPECT_TRUE(result.success); + // Always discard the layer tree. + auto discard_callback = [](LayerTree&) { return true; }; + RasterStatus status = rasterizer->Draw(CreateFinishedBuildRecorder(), + pipeline, discard_callback); + EXPECT_EQ(status, RasterStatus::kDiscarded); + latch.Signal(); + }); + latch.Wait(); +} + TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenPipelineIsEmpty) { std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); diff --git a/engine/src/flutter/shell/common/shell_unittests.cc b/engine/src/flutter/shell/common/shell_unittests.cc index 96dae426387..20b2aea6c35 100644 --- a/engine/src/flutter/shell/common/shell_unittests.cc +++ b/engine/src/flutter/shell/common/shell_unittests.cc @@ -2396,6 +2396,8 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { // TODO(https://github.com/flutter/flutter/issues/100273): Disabled due to // flakiness. +// TODO(https://github.com/flutter/flutter/issues/100299): Fix it when +// re-enabling. TEST_F(ShellTest, DISABLED_DiscardLayerTreeOnResize) { auto settings = CreateSettingsForFixture(); @@ -2448,6 +2450,8 @@ TEST_F(ShellTest, DISABLED_DiscardLayerTreeOnResize) { // TODO(https://github.com/flutter/flutter/issues/100273): Disabled due to // flakiness. +// TODO(https://github.com/flutter/flutter/issues/100299): Fix it when +// re-enabling. TEST_F(ShellTest, DISABLED_DiscardResubmittedLayerTreeOnResize) { auto settings = CreateSettingsForFixture();