Skip to content

Commit c8afdba

Browse files
mboetgerflutter-zl
authored andcommitted
Ensure content-sizing resize is run on UI thread (flutter#181686)
While testing with a different add-to-app flutter/samples#2787, an exception is sometimes thrown when the resize is attempted because the callback comes from a different thread. It is expected that the raster thread calls this but with previous [testing](https://github.com/mboetger/test-add-to-app/tree/content-sizing) never caused this exception. This PR ensures the resize happens on the UI thread. It also a flag to ensure content sizing is disabled. Fixes: flutter#181573 ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing.
1 parent 8a76da9 commit c8afdba

11 files changed

Lines changed: 106 additions & 10 deletions

File tree

engine/src/flutter/shell/platform/android/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ android_java_sources = [
221221
"io/flutter/Log.java",
222222
"io/flutter/app/FlutterApplication.java",
223223
"io/flutter/embedding/android/AndroidTouchProcessor.java",
224+
"io/flutter/embedding/android/ContentSizingFlag.java",
224225
"io/flutter/embedding/android/ExclusiveAppComponent.java",
225226
"io/flutter/embedding/android/FlutterActivity.java",
226227
"io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java",

engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "flutter/common/constants.h"
88
#include "flutter/fml/synchronization/waitable_event.h"
99
#include "flutter/fml/trace_event.h"
10+
#include "fml/make_copyable.h"
1011

1112
namespace flutter {
1213

@@ -225,7 +226,11 @@ void AndroidExternalViewEmbedder::PrepareFlutterView(
225226
DestroySurfaces();
226227
}
227228
surface_pool_->SetFrameSize(frame_size);
228-
jni_facade_->MaybeResizeSurfaceView(frame_size.width, frame_size.height);
229+
230+
task_runners_.GetPlatformTaskRunner()->PostTask(
231+
fml::MakeCopyable([jni_facade = jni_facade_, frame_size = frame_size]() {
232+
jni_facade->MaybeResizeSurfaceView(frame_size.width, frame_size.height);
233+
}));
229234

230235
frame_size_ = frame_size;
231236
device_pixel_ratio_ = device_pixel_ratio;

engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_2.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,11 @@ void AndroidExternalViewEmbedder2::PrepareFlutterView(
245245
DestroySurfaces();
246246
}
247247
surface_pool_->SetFrameSize(frame_size);
248-
jni_facade_->MaybeResizeSurfaceView(frame_size.width, frame_size.height);
248+
249+
task_runners_.GetPlatformTaskRunner()->PostTask(
250+
fml::MakeCopyable([jni_facade = jni_facade_, frame_size = frame_size]() {
251+
jni_facade->MaybeResizeSurfaceView(frame_size.width, frame_size.height);
252+
}));
249253

250254
frame_size_ = frame_size;
251255
device_pixel_ratio_ = device_pixel_ratio;

engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,13 @@
1111
#include "flutter/flow/embedded_views.h"
1212
#include "flutter/flow/surface.h"
1313
#include "flutter/fml/raster_thread_merger.h"
14+
#include "flutter/fml/synchronization/waitable_event.h"
1415
#include "flutter/fml/thread.h"
16+
#include "flutter/shell/common/thread_host.h"
1517
#include "flutter/shell/platform/android/jni/jni_mock.h"
1618
#include "flutter/shell/platform/android/surface/android_surface.h"
1719
#include "flutter/shell/platform/android/surface/android_surface_mock.h"
20+
#include "flutter/testing/testing.h"
1821

1922
#include "gmock/gmock.h"
2023
#include "gtest/gtest.h"
@@ -1068,8 +1071,18 @@ TEST(AndroidExternalViewEmbedder, Teardown) {
10681071
TEST(AndroidExternalViewEmbedder, MaybeResizeSurfaceView) {
10691072
auto jni_mock = std::make_shared<JNIMock>();
10701073
auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
1074+
ThreadHost thread_host("io.flutter.test." + GetCurrentTestName() + ".",
1075+
ThreadHost::Type::kPlatform | ThreadHost::Type::kIo |
1076+
ThreadHost::Type::kUi | ThreadHost::Type::kRaster);
1077+
TaskRunners task_runners(
1078+
"test",
1079+
thread_host.platform_thread->GetTaskRunner(), // platform
1080+
thread_host.raster_thread->GetTaskRunner(), // raster
1081+
thread_host.ui_thread->GetTaskRunner(), // ui
1082+
thread_host.io_thread->GetTaskRunner() // io
1083+
);
10711084
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
1072-
android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
1085+
android_context, jni_mock, nullptr, task_runners);
10731086

10741087
fml::Thread rasterizer_thread("rasterizer");
10751088
auto raster_thread_merger =
@@ -1081,6 +1094,11 @@ TEST(AndroidExternalViewEmbedder, MaybeResizeSurfaceView) {
10811094

10821095
EXPECT_CALL(*jni_mock, MaybeResizeSurfaceView(100, 200));
10831096
embedder->PrepareFlutterView(DlISize(100, 200), 1.0);
1097+
1098+
fml::AutoResetWaitableEvent latch;
1099+
fml::TaskRunner::RunNowOrPostTask(task_runners.GetPlatformTaskRunner(),
1100+
[&latch]() { latch.Signal(); });
1101+
latch.Wait();
10841102
}
10851103

10861104
TEST(AndroidExternalViewEmbedder, TeardownDoesNotCallJNIMethod) {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
package io.flutter.embedding.android;
6+
7+
import android.content.Context;
8+
import android.content.pm.ApplicationInfo;
9+
import android.content.pm.PackageManager;
10+
import android.content.pm.PackageManager.NameNotFoundException;
11+
import android.os.Bundle;
12+
import io.flutter.Log;
13+
14+
class ContentSizingFlag {
15+
16+
private static final String TAG = "ContentSizingFlag";
17+
18+
// Default to DISABLED
19+
private static final boolean DEFAULT = false;
20+
21+
private static final String ENABLE_CONTENT_SIZING =
22+
"io.flutter.embedding.android.EnableContentSizing";
23+
24+
static boolean isEnabled(Context context) {
25+
// Ensure that the context is actually the application context.
26+
final Context appContext = context.getApplicationContext();
27+
Bundle metaData = null;
28+
try {
29+
ApplicationInfo applicationInfo =
30+
appContext
31+
.getPackageManager()
32+
.getApplicationInfo(appContext.getPackageName(), PackageManager.GET_META_DATA);
33+
metaData = applicationInfo.metaData;
34+
} catch (NameNotFoundException ex) {
35+
Log.e(TAG, "Could not get metadata");
36+
}
37+
return metaData != null ? metaData.getBoolean(ENABLE_CONTENT_SIZING, DEFAULT) : DEFAULT;
38+
}
39+
}

engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public class FlutterImageView extends View implements RenderSurface {
4747
@Nullable private Bitmap currentBitmap;
4848
@Nullable private FlutterRenderer flutterRenderer;
4949

50+
private boolean isContentSizingEnabled = false;
51+
5052
public ImageReader getImageReader() {
5153
return imageReader;
5254
}
@@ -92,11 +94,16 @@ public FlutterImageView(@NonNull Context context, @NonNull AttributeSet attrs) {
9294

9395
private void init() {
9496
setAlpha(0.0f);
97+
isContentSizingEnabled = ContentSizingFlag.isEnabled(getContext());
9598
}
9699

97100
@Override
98101
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
99-
FlutterMeasureSpec.onMeasure(widthMeasureSpec, heightMeasureSpec, this::setMeasuredDimension);
102+
if (isContentSizingEnabled) {
103+
FlutterMeasureSpec.onMeasure(widthMeasureSpec, heightMeasureSpec, this::setMeasuredDimension);
104+
} else {
105+
super.onMeasure(widthMeasureSpec, heightMeasureSpec);
106+
}
100107
}
101108

102109
private static void logW(String format, Object... args) {

engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ public class FlutterSurfaceView extends SurfaceView implements RenderSurface {
4040
private boolean isPaused = false;
4141
@Nullable private FlutterRenderer flutterRenderer;
4242

43+
private boolean isContentSizingEnabled = false;
44+
4345
private boolean shouldNotify() {
4446
return flutterRenderer != null && !isPaused;
4547
}
@@ -116,14 +118,20 @@ private void init() {
116118
setZOrderOnTop(true);
117119
}
118120

121+
isContentSizingEnabled = ContentSizingFlag.isEnabled(getContext());
122+
119123
// Grab a reference to our underlying Surface and register callbacks with that Surface so we
120124
// can monitor changes and forward those changes on to native Flutter code.
121125
getHolder().addCallback(surfaceHolderCallbackCompat);
122126
}
123127

124128
@Override
125129
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
126-
FlutterMeasureSpec.onMeasure(widthMeasureSpec, heightMeasureSpec, this::setMeasuredDimension);
130+
if (isContentSizingEnabled) {
131+
FlutterMeasureSpec.onMeasure(widthMeasureSpec, heightMeasureSpec, this::setMeasuredDimension);
132+
} else {
133+
super.onMeasure(widthMeasureSpec, heightMeasureSpec);
134+
}
127135
}
128136

129137
// This is a work around for TalkBack.

engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ public class FlutterTextureView extends TextureView implements RenderSurface {
3939
@Nullable private FlutterRenderer flutterRenderer;
4040
@Nullable private Surface renderSurface;
4141

42+
private boolean isContentSizingEnabled = false;
43+
4244
private boolean shouldNotify() {
4345
return flutterRenderer != null && !isPaused;
4446
}
@@ -116,11 +118,17 @@ private void init() {
116118
// Listen for when our underlying SurfaceTexture becomes available, changes size, or
117119
// gets destroyed, and take the appropriate actions.
118120
setSurfaceTextureListener(surfaceTextureListener);
121+
122+
isContentSizingEnabled = ContentSizingFlag.isEnabled(getContext());
119123
}
120124

121125
@Override
122126
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
123-
FlutterMeasureSpec.onMeasure(widthMeasureSpec, heightMeasureSpec, this::setMeasuredDimension);
127+
if (isContentSizingEnabled) {
128+
FlutterMeasureSpec.onMeasure(widthMeasureSpec, heightMeasureSpec, this::setMeasuredDimension);
129+
} else {
130+
super.onMeasure(widthMeasureSpec, heightMeasureSpec);
131+
}
124132
}
125133

126134
@Nullable
@@ -273,4 +281,4 @@ private void disconnectSurfaceFromRenderer() {
273281
renderSurface = null;
274282
}
275283
}
276-
}
284+
}

engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterView.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ public class FlutterView extends FrameLayout
123123
// Maximum size allowed for a content sized view.
124124
@VisibleForTesting static final int CONTENT_SIZING_MAX = 2 << 12;
125125

126+
// Flag to enable content sizing.
127+
@VisibleForTesting boolean isContentSizingEnabled = false;
128+
126129
// Internal view hierarchy references.
127130
@Nullable private FlutterSurfaceView flutterSurfaceView;
128131
@Nullable private FlutterTextureView flutterTextureView;
@@ -424,6 +427,8 @@ private void init() {
424427
addView(flutterImageView);
425428
}
426429

430+
isContentSizingEnabled = ContentSizingFlag.isEnabled(getContext());
431+
427432
// FlutterView needs to be focusable so that the InputMethodManager can interact with it.
428433
setFocusable(true);
429434
setFocusableInTouchMode(true);
@@ -521,15 +526,15 @@ protected void onSizeChanged(int width, int height, int oldWidth, int oldHeight)
521526
viewportMetrics.width = width;
522527
viewportMetrics.height = height;
523528

524-
if (heightMode == MeasureSpec.UNSPECIFIED) {
529+
if (isContentSizingEnabled && heightMode == MeasureSpec.UNSPECIFIED) {
525530
Log.d(TAG, "FlutterView height is set to wrap content - updating viewport metrics to max");
526531
viewportMetrics.minHeight = 0;
527532
viewportMetrics.maxHeight = CONTENT_SIZING_MAX;
528533
} else {
529534
viewportMetrics.minHeight = viewportMetrics.height;
530535
viewportMetrics.maxHeight = viewportMetrics.height;
531536
}
532-
if (widthMode == MeasureSpec.UNSPECIFIED) {
537+
if (isContentSizingEnabled && widthMode == MeasureSpec.UNSPECIFIED) {
533538
Log.d(TAG, "FlutterView width is set to wrap content - updating viewport metrics to max");
534539
viewportMetrics.minWidth = 0;
535540
viewportMetrics.maxWidth = CONTENT_SIZING_MAX;

engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1328,8 +1328,8 @@ public void destroyOverlaySurfaces() {
13281328
platformViewsController.destroyOverlaySurfaces();
13291329
}
13301330

1331-
// This will get called on the raster thread.
13321331
@SuppressWarnings("unused")
1332+
@UiThread
13331333
public void maybeResizeSurfaceView(int width, int height) {
13341334
for (FlutterUiResizeListener listener : flutterUiResizeListeners) {
13351335
listener.resizeEngineView(width, height);

0 commit comments

Comments
 (0)