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

Commit d462cfd

Browse files
dnfielddnfield
authored
Remove flake inducing timeouts (#25847)
1 parent 74ac010 commit d462cfd

File tree

3 files changed

+38
-84
lines changed

3 files changed

+38
-84
lines changed

‎shell/common/shell.cc‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,10 +1762,16 @@ fml::Status Shell::WaitForFirstFrame(fml::TimeDelta timeout) {
17621762
"because it is responsible for generating the frame.");
17631763
}
17641764

1765+
// Check for overflow.
1766+
auto now = std::chrono::steady_clock::now();
1767+
auto max_duration = std::chrono::steady_clock::time_point::max() - now;
1768+
auto desired_duration = std::chrono::milliseconds(timeout.ToMilliseconds());
1769+
auto duration =
1770+
now + (desired_duration > max_duration ? max_duration : desired_duration);
1771+
17651772
std::unique_lock<std::mutex> lock(waiting_for_first_frame_mutex_);
1766-
bool success = waiting_for_first_frame_condition_.wait_for(
1767-
lock, std::chrono::milliseconds(timeout.ToMilliseconds()),
1768-
[&waiting_for_first_frame = waiting_for_first_frame_] {
1773+
bool success = waiting_for_first_frame_condition_.wait_until(
1774+
lock, duration, [&waiting_for_first_frame = waiting_for_first_frame_] {
17691775
return !waiting_for_first_frame.load();
17701776
});
17711777
if (success) {

‎shell/common/shell.h‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,16 @@ class Shell final : public PlatformView::Delegate,
298298
bool base64_encode);
299299

300300
//----------------------------------------------------------------------------
301-
/// @brief Pauses the calling thread until the first frame is presented.
301+
/// @brief Pauses the calling thread until the first frame is presented.
302302
///
303-
/// @return 'kOk' when the first frame has been presented before the timeout
304-
/// successfully, 'kFailedPrecondition' if called from the GPU or UI
305-
/// thread, 'kDeadlineExceeded' if there is a timeout.
303+
/// @param[in] timeout The duration to wait before timing out. If this
304+
/// duration would cause an overflow when added to
305+
/// std::chrono::steady_clock::now(), this method will
306+
/// wait indefinitely for the first frame.
307+
///
308+
/// @return 'kOk' when the first frame has been presented before the
309+
/// timeout successfully, 'kFailedPrecondition' if called from the
310+
/// GPU or UI thread, 'kDeadlineExceeded' if there is a timeout.
306311
///
307312
fml::Status WaitForFirstFrame(fml::TimeDelta timeout);
308313

‎shell/common/shell_unittests.cc‎

Lines changed: 20 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) {
672672
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
673673
auto sk_picture = recorder.finishRecordingAsPicture();
674674
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
675-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
675+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
676676
auto picture_layer = std::make_shared<PictureLayer>(
677677
SkPoint::Make(10, 10),
678678
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -727,7 +727,7 @@ TEST_F(ShellTest,
727727
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
728728
auto sk_picture = recorder.finishRecordingAsPicture();
729729
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
730-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
730+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
731731
auto picture_layer = std::make_shared<PictureLayer>(
732732
SkPoint::Make(10, 10),
733733
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -779,7 +779,7 @@ TEST_F(ShellTest,
779779
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
780780
auto sk_picture = recorder.finishRecordingAsPicture();
781781
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
782-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
782+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
783783
auto picture_layer = std::make_shared<PictureLayer>(
784784
SkPoint::Make(10, 10),
785785
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -788,9 +788,9 @@ TEST_F(ShellTest,
788788

789789
PumpOneFrame(shell.get(), 100, 100, builder);
790790

791-
auto result =
792-
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
793-
ASSERT_TRUE(result.ok());
791+
auto result = shell->WaitForFirstFrame(fml::TimeDelta::Max());
792+
ASSERT_TRUE(result.ok()) << "Result: " << static_cast<int>(result.code())
793+
<< ": " << result.message();
794794

795795
ASSERT_TRUE(raster_thread_merger->IsEnabled());
796796

@@ -800,7 +800,6 @@ TEST_F(ShellTest,
800800
// Validate the platform view can be recreated and destroyed again
801801
ValidateShell(shell.get());
802802
ASSERT_TRUE(raster_thread_merger->IsEnabled());
803-
804803
DestroyShell(std::move(shell));
805804
}
806805

@@ -853,7 +852,7 @@ TEST_F(ShellTest,
853852
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
854853
auto sk_picture = recorder.finishRecordingAsPicture();
855854
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
856-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
855+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
857856
auto picture_layer = std::make_shared<PictureLayer>(
858857
SkPoint::Make(10, 10),
859858
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -928,7 +927,7 @@ TEST_F(ShellTest,
928927
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
929928
auto sk_picture = recorder.finishRecordingAsPicture();
930929
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
931-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
930+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
932931
auto picture_layer = std::make_shared<PictureLayer>(
933932
SkPoint::Make(10, 10),
934933
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -1001,7 +1000,7 @@ TEST_F(ShellTest,
10011000
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
10021001
auto sk_picture = recorder.finishRecordingAsPicture();
10031002
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
1004-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
1003+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
10051004
auto picture_layer = std::make_shared<PictureLayer>(
10061005
SkPoint::Make(10, 10),
10071006
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -1049,7 +1048,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithoutRasterThreadMerger) {
10491048
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
10501049
auto sk_picture = recorder.finishRecordingAsPicture();
10511050
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
1052-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
1051+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
10531052
auto picture_layer = std::make_shared<PictureLayer>(
10541053
SkPoint::Make(10, 10),
10551054
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -1120,7 +1119,7 @@ TEST_F(ShellTest,
11201119
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
11211120
auto sk_picture = recorder.finishRecordingAsPicture();
11221121
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
1123-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
1122+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
11241123
auto picture_layer = std::make_shared<PictureLayer>(
11251124
SkPoint::Make(10, 10),
11261125
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -1263,57 +1262,6 @@ TEST(SettingsTest, FrameTimingSetsAndGetsProperly) {
12631262
}
12641263
}
12651264

1266-
#if FLUTTER_RELEASE
1267-
TEST_F(ShellTest, ReportTimingsIsCalledLaterInReleaseMode) {
1268-
#else
1269-
TEST_F(ShellTest, ReportTimingsIsCalledSoonerInNonReleaseMode) {
1270-
#endif
1271-
fml::TimePoint start = fml::TimePoint::Now();
1272-
auto settings = CreateSettingsForFixture();
1273-
std::unique_ptr<Shell> shell = CreateShell(settings);
1274-
1275-
// Create the surface needed by rasterizer
1276-
PlatformViewNotifyCreated(shell.get());
1277-
1278-
auto configuration = RunConfiguration::InferFromSettings(settings);
1279-
ASSERT_TRUE(configuration.IsValid());
1280-
configuration.SetEntrypoint("reportTimingsMain");
1281-
1282-
// Wait for 2 reports: the first one is the immediate callback of the first
1283-
// frame; the second one will exercise the batching logic.
1284-
fml::CountDownLatch reportLatch(2);
1285-
std::vector<int64_t> timestamps;
1286-
auto nativeTimingCallback = [&reportLatch,
1287-
&timestamps](Dart_NativeArguments args) {
1288-
Dart_Handle exception = nullptr;
1289-
timestamps = tonic::DartConverter<std::vector<int64_t>>::FromArguments(
1290-
args, 0, exception);
1291-
reportLatch.CountDown();
1292-
};
1293-
AddNativeCallback("NativeReportTimingsCallback",
1294-
CREATE_NATIVE_ENTRY(nativeTimingCallback));
1295-
RunEngine(shell.get(), std::move(configuration));
1296-
1297-
PumpOneFrame(shell.get());
1298-
PumpOneFrame(shell.get());
1299-
1300-
reportLatch.Wait();
1301-
DestroyShell(std::move(shell));
1302-
1303-
fml::TimePoint finish = fml::TimePoint::Now();
1304-
fml::TimeDelta elapsed = finish - start;
1305-
1306-
#if FLUTTER_RELEASE
1307-
// Our batch time is 1000ms. Hopefully the 800ms limit is relaxed enough to
1308-
// make it not too flaky.
1309-
ASSERT_TRUE(elapsed >= fml::TimeDelta::FromMilliseconds(800));
1310-
#else
1311-
// Our batch time is 100ms. Hopefully the 500ms limit is relaxed enough to
1312-
// make it not too flaky.
1313-
ASSERT_TRUE(elapsed <= fml::TimeDelta::FromMilliseconds(500));
1314-
#endif
1315-
}
1316-
13171265
TEST_F(ShellTest, ReportTimingsIsCalledImmediatelyAfterTheFirstFrame) {
13181266
auto settings = CreateSettingsForFixture();
13191267
std::unique_ptr<Shell> shell = CreateShell(settings);
@@ -1392,8 +1340,7 @@ TEST_F(ShellTest, WaitForFirstFrame) {
13921340

13931341
RunEngine(shell.get(), std::move(configuration));
13941342
PumpOneFrame(shell.get());
1395-
fml::Status result =
1396-
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
1343+
fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::Max());
13971344
ASSERT_TRUE(result.ok());
13981345

13991346
DestroyShell(std::move(shell));
@@ -1411,8 +1358,7 @@ TEST_F(ShellTest, WaitForFirstFrameZeroSizeFrame) {
14111358

14121359
RunEngine(shell.get(), std::move(configuration));
14131360
PumpOneFrame(shell.get(), {1.0, 0.0, 0.0});
1414-
fml::Status result =
1415-
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
1361+
fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::Zero());
14161362
ASSERT_FALSE(result.ok());
14171363
ASSERT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded);
14181364

@@ -1430,8 +1376,7 @@ TEST_F(ShellTest, WaitForFirstFrameTimeout) {
14301376
configuration.SetEntrypoint("emptyMain");
14311377

14321378
RunEngine(shell.get(), std::move(configuration));
1433-
fml::Status result =
1434-
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(10));
1379+
fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::Zero());
14351380
ASSERT_FALSE(result.ok());
14361381
ASSERT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded);
14371382

@@ -1450,11 +1395,10 @@ TEST_F(ShellTest, WaitForFirstFrameMultiple) {
14501395

14511396
RunEngine(shell.get(), std::move(configuration));
14521397
PumpOneFrame(shell.get());
1453-
fml::Status result =
1454-
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
1398+
fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::Max());
14551399
ASSERT_TRUE(result.ok());
14561400
for (int i = 0; i < 100; ++i) {
1457-
result = shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1));
1401+
result = shell->WaitForFirstFrame(fml::TimeDelta::Zero());
14581402
ASSERT_TRUE(result.ok());
14591403
}
14601404

@@ -1481,13 +1425,12 @@ TEST_F(ShellTest, WaitForFirstFrameInlined) {
14811425
PumpOneFrame(shell.get());
14821426
fml::AutoResetWaitableEvent event;
14831427
task_runner->PostTask([&shell, &event] {
1484-
fml::Status result =
1485-
shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000));
1428+
fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::Max());
14861429
ASSERT_FALSE(result.ok());
14871430
ASSERT_EQ(result.code(), fml::StatusCode::kFailedPrecondition);
14881431
event.Signal();
14891432
});
1490-
ASSERT_FALSE(event.WaitWithTimeout(fml::TimeDelta::FromMilliseconds(1000)));
1433+
ASSERT_FALSE(event.WaitWithTimeout(fml::TimeDelta::Max()));
14911434

14921435
DestroyShell(std::move(shell), std::move(task_runners));
14931436
}
@@ -1816,7 +1759,7 @@ TEST_F(ShellTest, Screenshot) {
18161759
SkPaint(SkColor4f::FromColor(SK_ColorRED)));
18171760
auto sk_picture = recorder.finishRecordingAsPicture();
18181761
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
1819-
this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
1762+
this->GetCurrentTaskRunner(), fml::TimeDelta::Zero());
18201763
auto picture_layer = std::make_shared<PictureLayer>(
18211764
SkPoint::Make(10, 10),
18221765
flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
@@ -2106,7 +2049,7 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) {
21062049
// 1. Construct a picture and a picture layer to be raster cached.
21072050
sk_sp<SkPicture> picture = MakeSizedPicture(10, 10);
21082051
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
2109-
GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
2052+
GetCurrentTaskRunner(), fml::TimeDelta::Zero());
21102053
auto picture_layer = std::make_shared<PictureLayer>(
21112054
SkPoint::Make(0, 0),
21122055
flutter::SkiaGPUObject<SkPicture>({MakeSizedPicture(100, 100), queue}),

0 commit comments

Comments
 (0)