Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 55420ec

Browse files
ColdPaleLightColdPaleLight
authored
Revert "Teardown external view embedder prior to unmerging threads (#30893)" (#31108)
This reverts commit d1d8119.
1 parent 4252c5a commit 55420ec

File tree

4 files changed

+31
-68
lines changed

4 files changed

+31
-68
lines changed

‎shell/platform/android/external_view_embedder/external_view_embedder.cc‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44

55
#include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h"
66

7-
#include "flutter/fml/synchronization/waitable_event.h"
8-
#include "flutter/fml/task_runner.h"
97
#include "flutter/fml/trace_event.h"
108
#include "flutter/shell/platform/android/surface/android_surface.h"
119

@@ -14,14 +12,12 @@ namespace flutter {
1412
AndroidExternalViewEmbedder::AndroidExternalViewEmbedder(
1513
const AndroidContext& android_context,
1614
std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
17-
std::shared_ptr<AndroidSurfaceFactory> surface_factory,
18-
TaskRunners task_runners)
15+
std::shared_ptr<AndroidSurfaceFactory> surface_factory)
1916
: ExternalViewEmbedder(),
2017
android_context_(android_context),
2118
jni_facade_(jni_facade),
2219
surface_factory_(surface_factory),
23-
surface_pool_(std::make_unique<SurfacePool>()),
24-
task_runners_(task_runners) {}
20+
surface_pool_(std::make_unique<SurfacePool>()) {}
2521

2622
// |ExternalViewEmbedder|
2723
void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView(
@@ -268,8 +264,8 @@ void AndroidExternalViewEmbedder::BeginFrame(
268264

269265
// The surface size changed. Therefore, destroy existing surfaces as
270266
// the existing surfaces in the pool can't be recycled.
271-
if (frame_size_ != frame_size) {
272-
DestroySurfaces();
267+
if (frame_size_ != frame_size && raster_thread_merger->IsOnPlatformThread()) {
268+
surface_pool_->DestroyLayers(jni_facade_);
273269
}
274270
surface_pool_->SetFrameSize(frame_size);
275271
// JNI method must be called on the platform thread.
@@ -304,18 +300,7 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() {
304300

305301
// |ExternalViewEmbedder|
306302
void AndroidExternalViewEmbedder::Teardown() {
307-
DestroySurfaces();
308-
}
309-
310-
// |ExternalViewEmbedder|
311-
void AndroidExternalViewEmbedder::DestroySurfaces() {
312-
fml::AutoResetWaitableEvent latch;
313-
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetPlatformTaskRunner(),
314-
[&]() {
315-
surface_pool_->DestroyLayers(jni_facade_);
316-
latch.Signal();
317-
});
318-
latch.Wait();
303+
surface_pool_->DestroyLayers(jni_facade_);
319304
}
320305

321306
} // namespace flutter

‎shell/platform/android/external_view_embedder/external_view_embedder.h‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
#include <unordered_map>
99

10-
#include "flutter/common/task_runners.h"
1110
#include "flutter/flow/embedded_views.h"
1211
#include "flutter/flow/rtree.h"
1312
#include "flutter/shell/platform/android/context/android_context.h"
@@ -33,8 +32,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder {
3332
AndroidExternalViewEmbedder(
3433
const AndroidContext& android_context,
3534
std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
36-
std::shared_ptr<AndroidSurfaceFactory> surface_factory,
37-
TaskRunners task_runners);
35+
std::shared_ptr<AndroidSurfaceFactory> surface_factory);
3836

3937
// |ExternalViewEmbedder|
4038
void PrerollCompositeEmbeddedView(
@@ -101,9 +99,6 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder {
10199
// Holds surfaces. Allows to recycle surfaces or allocate new ones.
102100
const std::unique_ptr<SurfacePool> surface_pool_;
103101

104-
// The task runners.
105-
const TaskRunners task_runners_;
106-
107102
// The size of the root canvas.
108103
SkISize frame_size_;
109104

@@ -131,11 +126,6 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder {
131126
// The number of platform views in the previous frame.
132127
int64_t previous_frame_view_count_;
133128

134-
// Destroys the surfaces created from the surface factory.
135-
// This method schedules a task on the platform thread, and waits for
136-
// the task until it completes.
137-
void DestroySurfaces();
138-
139129
// Resets the state.
140130
void Reset();
141131

‎shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc‎

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#define FML_USED_ON_EMBEDDER
6-
75
#include <memory>
86
#include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h"
97

@@ -89,24 +87,12 @@ fml::RefPtr<fml::RasterThreadMerger> GetThreadMergerFromRasterThread(
8987
rasterizer_queue_id);
9088
}
9189

92-
TaskRunners GetTaskRunnersForFixture() {
93-
fml::MessageLoop::EnsureInitializedForCurrentThread();
94-
auto& loop = fml::MessageLoop::GetCurrent();
95-
return {
96-
"test",
97-
loop.GetTaskRunner(), // platform
98-
loop.GetTaskRunner(), // raster
99-
loop.GetTaskRunner(), // ui
100-
loop.GetTaskRunner() // io
101-
};
102-
}
103-
10490
TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) {
10591
auto jni_mock = std::make_shared<JNIMock>();
10692

10793
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
10894
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
109-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
95+
android_context, jni_mock, nullptr);
11096
fml::Thread rasterizer_thread("rasterizer");
11197
auto raster_thread_merger =
11298
GetThreadMergerFromPlatformThread(&rasterizer_thread);
@@ -131,7 +117,7 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvasesCompositeOrder) {
131117

132118
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
133119
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
134-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
120+
android_context, jni_mock, nullptr);
135121
fml::Thread rasterizer_thread("rasterizer");
136122
auto raster_thread_merger =
137123
GetThreadMergerFromPlatformThread(&rasterizer_thread);
@@ -154,7 +140,7 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvasesCompositeOrder) {
154140
TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) {
155141
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
156142
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
157-
android_context, nullptr, nullptr, GetTaskRunnersForFixture());
143+
android_context, nullptr, nullptr);
158144

159145
ASSERT_EQ(nullptr, embedder->CompositeEmbeddedView(0));
160146
embedder->PrerollCompositeEmbeddedView(
@@ -170,7 +156,7 @@ TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) {
170156
TEST(AndroidExternalViewEmbedder, CancelFrame) {
171157
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
172158
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
173-
android_context, nullptr, nullptr, GetTaskRunnersForFixture());
159+
android_context, nullptr, nullptr);
174160

175161
embedder->PrerollCompositeEmbeddedView(
176162
0, std::make_unique<EmbeddedViewParams>());
@@ -184,7 +170,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) {
184170
auto jni_mock = std::make_shared<JNIMock>();
185171
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
186172
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
187-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
173+
android_context, jni_mock, nullptr);
188174

189175
fml::Thread rasterizer_thread("rasterizer");
190176
auto raster_thread_merger =
@@ -218,7 +204,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) {
218204
auto jni_mock = std::make_shared<JNIMock>();
219205
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
220206
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
221-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
207+
android_context, jni_mock, nullptr);
222208

223209
fml::Thread rasterizer_thread("rasterizer");
224210
auto raster_thread_merger =
@@ -239,7 +225,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) {
239225

240226
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
241227
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
242-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
228+
android_context, jni_mock, nullptr);
243229
fml::Thread rasterizer_thread("rasterizer");
244230
auto raster_thread_merger =
245231
GetThreadMergerFromPlatformThread(&rasterizer_thread);
@@ -267,7 +253,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRectChangedParams) {
267253

268254
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
269255
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
270-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
256+
android_context, jni_mock, nullptr);
271257
fml::Thread rasterizer_thread("rasterizer");
272258
auto raster_thread_merger =
273259
GetThreadMergerFromPlatformThread(&rasterizer_thread);
@@ -342,7 +328,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) {
342328
return android_surface_mock;
343329
});
344330
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
345-
*android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
331+
*android_context, jni_mock, surface_factory);
346332

347333
auto raster_thread_merger = GetThreadMergerFromPlatformThread();
348334

@@ -535,7 +521,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrameOverlayComposition) {
535521
return android_surface_mock;
536522
});
537523
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
538-
*android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
524+
*android_context, jni_mock, surface_factory);
539525

540526
auto raster_thread_merger = GetThreadMergerFromPlatformThread();
541527

@@ -637,7 +623,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFramePlatformViewWithoutAnyOverlay) {
637623
return android_surface_mock;
638624
});
639625
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
640-
*android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
626+
*android_context, jni_mock, surface_factory);
641627

642628
auto raster_thread_merger = GetThreadMergerFromPlatformThread();
643629

@@ -676,7 +662,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotCallJNIPlatformThreadOnlyMethods) {
676662

677663
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
678664
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
679-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
665+
android_context, jni_mock, nullptr);
680666

681667
// While on the raster thread, don't make JNI calls as these methods can only
682668
// run on the platform thread.
@@ -725,7 +711,7 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) {
725711
});
726712

727713
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
728-
*android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
714+
*android_context, jni_mock, surface_factory);
729715
fml::Thread rasterizer_thread("rasterizer");
730716
auto raster_thread_merger =
731717
GetThreadMergerFromPlatformThread(&rasterizer_thread);
@@ -812,7 +798,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) {
812798
});
813799

814800
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
815-
*android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
801+
*android_context, jni_mock, surface_factory);
816802

817803
// ------------------ First frame ------------------ //
818804
{
@@ -857,7 +843,9 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) {
857843
embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger);
858844
}
859845

860-
EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1);
846+
// Changing the frame size from the raster thread does not make JNI calls.
847+
848+
EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0);
861849
EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0);
862850

863851
fml::Thread platform_thread("platform");
@@ -869,15 +857,15 @@ TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) {
869857
auto jni_mock = std::make_shared<JNIMock>();
870858
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
871859
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
872-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
860+
android_context, jni_mock, nullptr);
873861
ASSERT_TRUE(embedder->SupportsDynamicThreadMerging());
874862
}
875863

876864
TEST(AndroidExternalViewEmbedder, DisableThreadMerger) {
877865
auto jni_mock = std::make_shared<JNIMock>();
878866
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
879867
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
880-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
868+
android_context, jni_mock, nullptr);
881869

882870
fml::Thread platform_thread("platform");
883871
auto raster_thread_merger = GetThreadMergerFromRasterThread(&platform_thread);
@@ -933,7 +921,7 @@ TEST(AndroidExternalViewEmbedder, Teardown) {
933921
});
934922

935923
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
936-
*android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
924+
*android_context, jni_mock, surface_factory);
937925
fml::Thread rasterizer_thread("rasterizer");
938926
auto raster_thread_merger =
939927
GetThreadMergerFromPlatformThread(&rasterizer_thread);

‎shell/platform/android/platform_view_android.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@
1414
#include "flutter/shell/platform/android/android_external_texture_gl.h"
1515
#include "flutter/shell/platform/android/android_surface_gl.h"
1616
#include "flutter/shell/platform/android/android_surface_software.h"
17-
#include "flutter/shell/platform/android/context/android_context.h"
1817
#include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h"
19-
#include "flutter/shell/platform/android/jni/platform_view_android_jni.h"
20-
#include "flutter/shell/platform/android/platform_message_response_android.h"
2118
#include "flutter/shell/platform/android/surface/android_surface.h"
2219
#include "flutter/shell/platform/android/surface/snapshot_surface_producer.h"
20+
21+
#include "flutter/shell/platform/android/context/android_context.h"
22+
#include "flutter/shell/platform/android/jni/platform_view_android_jni.h"
23+
#include "flutter/shell/platform/android/platform_message_response_android.h"
2324
#include "flutter/shell/platform/android/vsync_waiter_android.h"
2425

2526
namespace flutter {
@@ -254,8 +255,7 @@ std::unique_ptr<Surface> PlatformViewAndroid::CreateRenderingSurface() {
254255
std::shared_ptr<ExternalViewEmbedder>
255256
PlatformViewAndroid::CreateExternalViewEmbedder() {
256257
return std::make_shared<AndroidExternalViewEmbedder>(
257-
*android_context_, jni_facade_, surface_factory_,
258-
std::move(task_runners_));
258+
*android_context_, jni_facade_, surface_factory_);
259259
}
260260

261261
// |PlatformView|

0 commit comments

Comments
 (0)