Skip to content

Commit 54cbbf1

Browse files
authored
Fix: invalid time-point comparison between each from different clock source (flutter#42409)
Sometimes the time point created late will less than the time point create earlier since the CLOCK SOURCE has changed. We must make sure the CLOCK SOURCE (fml::TimePoint) is always keep the same while we are testing. Corrects the test case is needed before we fix the problem completely.
1 parent 5b54580 commit 54cbbf1

1 file changed

Lines changed: 25 additions & 12 deletions

File tree

shell/common/shell_unittests.cc

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
#define FML_USED_ON_EMBEDDER
66

77
#include <algorithm>
8+
#include <chrono>
89
#include <ctime>
910
#include <future>
1011
#include <memory>
12+
#include <thread>
1113
#include <utility>
1214
#include <vector>
1315

@@ -676,10 +678,14 @@ static void CheckFrameTimings(const std::vector<FrameTiming>& timings,
676678
}
677679

678680
TEST_F(ShellTest, ReportTimingsIsCalled) {
679-
fml::TimePoint start = fml::TimePoint::Now();
680681
auto settings = CreateSettingsForFixture();
681682
std::unique_ptr<Shell> shell = CreateShell(settings);
682683

684+
// We MUST put |start| after |CreateShell| because the clock source will be
685+
// reset through |TimePoint::SetClockSource()| in
686+
// |DartVMInitializer::Initialize()| within |CreateShell()|.
687+
fml::TimePoint start = fml::TimePoint::Now();
688+
683689
// Create the surface needed by rasterizer
684690
PlatformViewNotifyCreated(shell.get());
685691

@@ -726,19 +732,10 @@ TEST_F(ShellTest, ReportTimingsIsCalled) {
726732
}
727733

728734
TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) {
729-
fml::TimePoint start = fml::TimePoint::Now();
730-
731735
auto settings = CreateSettingsForFixture();
732-
fml::AutoResetWaitableEvent timingLatch;
733-
FrameTiming timing;
734-
735-
for (auto phase : FrameTiming::kPhases) {
736-
timing.Set(phase, fml::TimePoint());
737-
// Check that the time points are initially smaller than start, so
738-
// CheckFrameTimings will fail if they're not properly set later.
739-
ASSERT_TRUE(timing.Get(phase) < start);
740-
}
741736

737+
FrameTiming timing;
738+
fml::AutoResetWaitableEvent timingLatch;
742739
settings.frame_rasterized_callback = [&timing,
743740
&timingLatch](const FrameTiming& t) {
744741
timing = t;
@@ -747,6 +744,22 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) {
747744

748745
std::unique_ptr<Shell> shell = CreateShell(settings);
749746

747+
// Wait to make |start| bigger than zero
748+
using namespace std::chrono_literals;
749+
std::this_thread::sleep_for(1ms);
750+
751+
// We MUST put |start| after |CreateShell()| because the clock source will be
752+
// reset through |TimePoint::SetClockSource()| in
753+
// |DartVMInitializer::Initialize()| within |CreateShell()|.
754+
fml::TimePoint start = fml::TimePoint::Now();
755+
756+
for (auto phase : FrameTiming::kPhases) {
757+
timing.Set(phase, fml::TimePoint());
758+
// Check that the time points are initially smaller than start, so
759+
// CheckFrameTimings will fail if they're not properly set later.
760+
ASSERT_TRUE(timing.Get(phase) < start);
761+
}
762+
750763
// Create the surface needed by rasterizer
751764
PlatformViewNotifyCreated(shell.get());
752765

0 commit comments

Comments
 (0)