Skip to content

Commit 5b74a3b

Browse files
Check for a null cached image in SingleFrameCodec::getNextFrame (flutter#179483)
This could happen if the image decoder invoked its callback with a null image to signal an error. The SingleFrameCodec's status will be kComplete but its cached image will not be set. Fixes flutter#161031
1 parent 6519f99 commit 5b74a3b

File tree

3 files changed

+109
-0
lines changed

3 files changed

+109
-0
lines changed

engine/src/flutter/lib/ui/fixtures/ui_test.dart

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,46 @@ Future<void> createSingleFrameCodec() async {
111111
_finish();
112112
}
113113

114+
@pragma('vm:entry-point')
115+
Future<void> singleFrameCodecHandlesNoGpu() async {
116+
final ImmutableBuffer buffer = await ImmutableBuffer.fromUint8List(
117+
Uint8List.fromList(List<int>.filled(4, 100)),
118+
);
119+
final ImageDescriptor descriptor = ImageDescriptor.raw(
120+
buffer,
121+
width: 1,
122+
height: 1,
123+
pixelFormat: PixelFormat.rgba8888,
124+
);
125+
_turnOffGPU(true);
126+
Timer flusher = Timer.periodic(Duration(milliseconds: 1), (timer) {
127+
_flushGpuAwaitingTasks();
128+
});
129+
try {
130+
final Codec codec = await descriptor.instantiateCodec();
131+
132+
// Call getNextFrame twice. The first call will throw because the GPU has
133+
// been disabled. The second call will throw because SingleFrameCodec does
134+
// not have a cached image.
135+
for (int i = 0; i < 2; i++) {
136+
bool didThrow = false;
137+
try {
138+
final FrameInfo info = await codec.getNextFrame();
139+
} catch (e) {
140+
didThrow = true;
141+
}
142+
assert(didThrow);
143+
}
144+
145+
codec.dispose();
146+
descriptor.dispose();
147+
buffer.dispose();
148+
_finish();
149+
} finally {
150+
flusher.cancel();
151+
}
152+
}
153+
114154
@pragma('vm:external-name', 'ValidateCodec')
115155
external void _validateCodec(Codec codec);
116156

engine/src/flutter/lib/ui/painting/single_frame_codec.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) {
3535
}
3636

3737
if (status_ == Status::kComplete) {
38+
if (!cached_image_) {
39+
return tonic::ToDart("Image failed to decode");
40+
}
3841
if (!cached_image_->image()) {
3942
return tonic::ToDart("Decoded image has been disposed");
4043
}

engine/src/flutter/lib/ui/painting/single_frame_codec_unittests.cc

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313
#include "flutter/shell/common/thread_host.h"
1414
#include "flutter/testing/testing.h"
1515

16+
// CREATE_NATIVE_ENTRY is leaky by design
17+
// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
18+
19+
#pragma GCC diagnostic ignored "-Wunreachable-code"
20+
1621
namespace flutter {
1722
namespace testing {
1823

@@ -55,5 +60,66 @@ TEST_F(ShellTest, SingleFrameCodecAccuratelyReportsSize) {
5560
DestroyShell(std::move(shell), task_runners);
5661
}
5762

63+
TEST_F(ShellTest, SingleFrameCodecHandlesNoGpu) {
64+
#ifndef FML_OS_MACOSX
65+
GTEST_SKIP() << "Only works on macOS currently.";
66+
#endif
67+
68+
Settings settings = CreateSettingsForFixture();
69+
settings.enable_impeller = true;
70+
TaskRunners task_runners("test", // label
71+
GetCurrentTaskRunner(), // platform
72+
CreateNewThread(), // raster
73+
CreateNewThread(), // ui
74+
CreateNewThread() // io
75+
);
76+
std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
77+
ASSERT_TRUE(shell->IsSetup());
78+
79+
auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
80+
auto finish = [message_latch](Dart_NativeArguments args) {
81+
message_latch->Signal();
82+
};
83+
AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));
84+
85+
auto turn_off_gpu = [&](Dart_NativeArguments args) {
86+
auto handle = Dart_GetNativeArgument(args, 0);
87+
bool value = true;
88+
ASSERT_TRUE(Dart_IsBoolean(handle));
89+
Dart_BooleanValue(handle, &value);
90+
TurnOffGPU(shell.get(), value);
91+
};
92+
AddNativeCallback("TurnOffGPU", CREATE_NATIVE_ENTRY(turn_off_gpu));
93+
94+
auto flush_awaiting_tasks = [&](Dart_NativeArguments args) {
95+
fml::WeakPtr io_manager = shell->GetIOManager();
96+
task_runners.GetIOTaskRunner()->PostTask([io_manager] {
97+
if (io_manager) {
98+
std::shared_ptr<impeller::Context> impeller_context =
99+
io_manager->GetImpellerContext();
100+
// This will cause the stored tasks to overflow and start throwing them
101+
// away.
102+
for (int i = 0; i < impeller::Context::kMaxTasksAwaitingGPU; i++) {
103+
impeller_context->StoreTaskForGPU([] {}, [] {});
104+
}
105+
}
106+
});
107+
};
108+
AddNativeCallback("FlushGpuAwaitingTasks",
109+
CREATE_NATIVE_ENTRY(flush_awaiting_tasks));
110+
111+
auto configuration = RunConfiguration::InferFromSettings(settings);
112+
configuration.SetEntrypoint("singleFrameCodecHandlesNoGpu");
113+
114+
shell->RunEngine(std::move(configuration), [](auto result) {
115+
ASSERT_EQ(result, Engine::RunStatus::Success);
116+
});
117+
118+
message_latch->Wait();
119+
DestroyShell(std::move(shell), task_runners);
120+
}
121+
58122
} // namespace testing
59123
} // namespace flutter
124+
125+
// NOLINTEND(clang-analyzer-core.StackAddressEscape)

0 commit comments

Comments
 (0)