From 804aa6d2e38cd5ac8660c28b8be8c3aaf3fbbbab Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 27 Sep 2021 17:04:06 -0700 Subject: [PATCH 1/3] Destroy overlay surfaces when the rasterizer is torn down --- flow/embedded_views.h | 5 ++ shell/common/rasterizer.cc | 4 ++ .../shell_test_external_view_embedder.cc | 2 + .../shell_test_external_view_embedder.h | 3 ++ .../external_view_embedder.cc | 5 ++ .../external_view_embedder.h | 2 + .../external_view_embedder_unittests.cc | 52 +++++++++++++++++++ .../darwin/ios/ios_external_view_embedder.h | 3 ++ .../darwin/ios/ios_external_view_embedder.mm | 3 ++ .../flutter/flatland_external_view_embedder.h | 3 ++ .../flutter/fuchsia_external_view_embedder.h | 3 ++ shell/testing/tester_main.cc | 3 ++ 12 files changed, 88 insertions(+) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index c94d671752e0c..b5ff328eb0956 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -337,6 +337,11 @@ class ExternalViewEmbedder { // |RasterThreadMerger| instance. virtual bool SupportsDynamicThreadMerging(); + // Called when the rasterizer is being torn down. + // This method provides a way to release resources associated with the current + // embedder. + virtual void Teardown(); + FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); }; // ExternalViewEmbedder diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index cc17ce762339c..9462fd6d78163 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -97,6 +97,10 @@ void Rasterizer::Teardown() { raster_thread_merger_->UnMergeNowIfLastOne(); raster_thread_merger_->SetMergeUnmergeCallback(nullptr); } + + if (external_view_embedder_) { + external_view_embedder_->Teardown(); + } } void Rasterizer::EnableThreadMergerIfNeeded() { diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index 5a8edc19fb45a..26446df7f1850 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -90,4 +90,6 @@ bool ShellTestExternalViewEmbedder::SupportsDynamicThreadMerging() { return support_thread_merging_; } +void ShellTestExternalViewEmbedder::Teardown() {} + } // namespace flutter diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 72c101ed1f4bc..6a711cad93f6b 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -76,6 +76,9 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override; + // |ExternalViewEmbedder| + void Teardown() override; + const EndFrameCallBack end_frame_call_back_; PostPrerollResult post_preroll_result_; diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 08596ba8ad925..0c326eb950b03 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -298,4 +298,9 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { return true; } +// |ExternalViewEmbedder| +void AndroidExternalViewEmbedder::Teardown() { + surface_pool_->DestroyLayers(jni_facade_); +} + } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index a0386167418d1..278d7693e7661 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -73,6 +73,8 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { bool SupportsDynamicThreadMerging() override; + void Teardown() override; + // Gets the rect based on the device pixel ratio of a platform view displayed // on the screen. SkRect GetViewRect(int view_id) const; diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index 4e1014ac9ad3f..efe4ce375c3ac 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -883,5 +883,57 @@ TEST(AndroidExternalViewEmbedder, DisableThreadMerger) { ASSERT_FALSE(raster_thread_merger->IsMerged()); } +TEST(AndroidExternalViewEmbedder, Teardown) { + auto jni_mock = std::make_shared(); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + auto window = fml::MakeRefCounted(nullptr); + auto gr_context = GrDirectContext::MakeMock(nullptr); + auto surface_factory = std::make_shared( + [&android_context, gr_context, window]() { + auto surface_frame_1 = std::make_unique( + SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { + return true; + }); + + auto surface_mock = std::make_unique(); + auto android_surface_mock = + std::make_unique(android_context); + return android_surface_mock; + }); + + auto embedder = std::make_unique( + *android_context, jni_mock, surface_factory); + fml::Thread rasterizer_thread("rasterizer"); + auto raster_thread_merger = + GetThreadMergerFromPlatformThread(&rasterizer_thread); + + auto frame_size = SkISize::Make(1000, 1000); + embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger); + + // Add an Android view. + MutatorsStack stack; + auto view_params = std::make_unique( + SkMatrix(), SkSize::Make(200, 200), stack); + + embedder->PrerollCompositeEmbeddedView(0, std::move(view_params)); + + // This simulates Flutter UI that intersects with the Android view. + embedder->CompositeEmbeddedView(0)->drawRect( + SkRect::MakeXYWH(50, 50, 200, 200), SkPaint()); + + auto surface_frame = std::make_unique( + SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); + + embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); + + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()); + // Teardown. + embedder->Teardown(); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.h b/shell/platform/darwin/ios/ios_external_view_embedder.h index 6c023d1b793e1..be295bb75755d 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.h +++ b/shell/platform/darwin/ios/ios_external_view_embedder.h @@ -64,6 +64,9 @@ class IOSExternalViewEmbedder : public ExternalViewEmbedder { // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override; + // |ExternalViewEmbedder| + void Teardown() override; + FML_DISALLOW_COPY_AND_ASSIGN(IOSExternalViewEmbedder); }; diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.mm b/shell/platform/darwin/ios/ios_external_view_embedder.mm index a4921af254af9..d767b97f7fae1 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.mm +++ b/shell/platform/darwin/ios/ios_external_view_embedder.mm @@ -92,4 +92,7 @@ return true; } +// |ExternalViewEmbedder| +void IOSExternalViewEmbedder::Teardown() {} + } // namespace flutter diff --git a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h index 72e794022ec60..6c2061c65e5ec 100644 --- a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h @@ -91,6 +91,9 @@ class FlatlandExternalViewEmbedder final // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override { return false; } + // |ExternalViewEmbedder| + void Teardown() override {} + // View manipulation. // |SetViewProperties| doesn't manipulate the view directly -- it sets pending // properties for the next |UpdateView| call. diff --git a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h index 7021a3df3332e..9dea5c9129be5 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h @@ -120,6 +120,9 @@ class FuchsiaExternalViewEmbedder final : public flutter::ExternalViewEmbedder { // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override { return false; } + // |ExternalViewEmbedder| + void Teardown() override {} + // View manipulation. // |SetViewProperties| doesn't manipulate the view directly -- it sets pending // properties for the next |UpdateView| call. diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index b96166c5faee6..cb6136e491054 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -58,6 +58,9 @@ class TesterExternalViewEmbedder : public ExternalViewEmbedder { // |ExternalViewEmbedder| SkCanvas* CompositeEmbeddedView(int view_id) override { return &canvas_; } + // |ExternalViewEmbedder| + void Teardown() override {} + private: SkCanvas canvas_; }; From a2249cd56419ef215ae3052656f1c6dfd44b7be5 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 27 Sep 2021 17:31:19 -0700 Subject: [PATCH 2/3] simplify --- flow/embedded_views.cc | 2 ++ shell/common/shell_test_external_view_embedder.cc | 2 -- shell/common/shell_test_external_view_embedder.h | 3 --- shell/platform/darwin/ios/ios_external_view_embedder.h | 3 --- shell/platform/darwin/ios/ios_external_view_embedder.mm | 3 --- .../platform/fuchsia/flutter/flatland_external_view_embedder.h | 3 --- .../platform/fuchsia/flutter/fuchsia_external_view_embedder.h | 3 --- shell/testing/tester_main.cc | 3 --- 8 files changed, 2 insertions(+), 20 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index 9441c8dc9470c..c85ad66f8dc75 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -64,4 +64,6 @@ bool ExternalViewEmbedder::SupportsDynamicThreadMerging() { return false; } +void ExternalViewEmbedder::Teardown() {} + } // namespace flutter diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index 26446df7f1850..5a8edc19fb45a 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -90,6 +90,4 @@ bool ShellTestExternalViewEmbedder::SupportsDynamicThreadMerging() { return support_thread_merging_; } -void ShellTestExternalViewEmbedder::Teardown() {} - } // namespace flutter diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 6a711cad93f6b..72c101ed1f4bc 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -76,9 +76,6 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override; - // |ExternalViewEmbedder| - void Teardown() override; - const EndFrameCallBack end_frame_call_back_; PostPrerollResult post_preroll_result_; diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.h b/shell/platform/darwin/ios/ios_external_view_embedder.h index be295bb75755d..6c023d1b793e1 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.h +++ b/shell/platform/darwin/ios/ios_external_view_embedder.h @@ -64,9 +64,6 @@ class IOSExternalViewEmbedder : public ExternalViewEmbedder { // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override; - // |ExternalViewEmbedder| - void Teardown() override; - FML_DISALLOW_COPY_AND_ASSIGN(IOSExternalViewEmbedder); }; diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.mm b/shell/platform/darwin/ios/ios_external_view_embedder.mm index d767b97f7fae1..a4921af254af9 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.mm +++ b/shell/platform/darwin/ios/ios_external_view_embedder.mm @@ -92,7 +92,4 @@ return true; } -// |ExternalViewEmbedder| -void IOSExternalViewEmbedder::Teardown() {} - } // namespace flutter diff --git a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h index 6c2061c65e5ec..72e794022ec60 100644 --- a/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h @@ -91,9 +91,6 @@ class FlatlandExternalViewEmbedder final // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override { return false; } - // |ExternalViewEmbedder| - void Teardown() override {} - // View manipulation. // |SetViewProperties| doesn't manipulate the view directly -- it sets pending // properties for the next |UpdateView| call. diff --git a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h index 9dea5c9129be5..7021a3df3332e 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/fuchsia_external_view_embedder.h @@ -120,9 +120,6 @@ class FuchsiaExternalViewEmbedder final : public flutter::ExternalViewEmbedder { // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override { return false; } - // |ExternalViewEmbedder| - void Teardown() override {} - // View manipulation. // |SetViewProperties| doesn't manipulate the view directly -- it sets pending // properties for the next |UpdateView| call. diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index cb6136e491054..b96166c5faee6 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -58,9 +58,6 @@ class TesterExternalViewEmbedder : public ExternalViewEmbedder { // |ExternalViewEmbedder| SkCanvas* CompositeEmbeddedView(int view_id) override { return &canvas_; } - // |ExternalViewEmbedder| - void Teardown() override {} - private: SkCanvas canvas_; }; From 128b0d5cba788c4e849cf139bdafd9f9f4180698 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Tue, 28 Sep 2021 11:50:10 -0700 Subject: [PATCH 3/3] Fix test --- .../external_view_embedder_unittests.cc | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index efe4ce375c3ac..aa72b6b13f73f 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -889,8 +889,9 @@ TEST(AndroidExternalViewEmbedder, Teardown) { std::make_shared(AndroidRenderingAPI::kSoftware); auto window = fml::MakeRefCounted(nullptr); auto gr_context = GrDirectContext::MakeMock(nullptr); + auto frame_size = SkISize::Make(1000, 1000); auto surface_factory = std::make_shared( - [&android_context, gr_context, window]() { + [&android_context, gr_context, window, frame_size]() { auto surface_frame_1 = std::make_unique( SkSurface::MakeNull(1000, 1000), false, [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { @@ -898,8 +899,15 @@ TEST(AndroidExternalViewEmbedder, Teardown) { }); auto surface_mock = std::make_unique(); + EXPECT_CALL(*surface_mock, AcquireFrame(frame_size)) + .WillOnce(Return(ByMove(std::move(surface_frame_1)))); + auto android_surface_mock = std::make_unique(android_context); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())) + .WillOnce(Return(ByMove(std::move(surface_mock)))); + return android_surface_mock; }); @@ -909,7 +917,6 @@ TEST(AndroidExternalViewEmbedder, Teardown) { auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); - auto frame_size = SkISize::Make(1000, 1000); embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger); // Add an Android view. @@ -923,6 +930,12 @@ TEST(AndroidExternalViewEmbedder, Teardown) { embedder->CompositeEmbeddedView(0)->drawRect( SkRect::MakeXYWH(50, 50, 200, 200), SkPaint()); + // Create a new overlay surface. + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))); + auto surface_frame = std::make_unique( SkSurface::MakeNull(1000, 1000), false, [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; });