From 55e8d11c5150e111bcaed3c9303556c031c9c925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=BE=E9=94=A6=E5=92=8C=28=E6=99=BA=E9=B9=B0=29?= Date: Tue, 30 May 2023 18:11:34 +0800 Subject: [PATCH 1/4] Fix: invalid time-point comparison between each from different clock source --- shell/common/shell_unittests.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 2edc3b173d82b..2f5ce0a92b7f7 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -676,10 +676,14 @@ static void CheckFrameTimings(const std::vector& timings, } TEST_F(ShellTest, ReportTimingsIsCalled) { - fml::TimePoint start = fml::TimePoint::Now(); auto settings = CreateSettingsForFixture(); std::unique_ptr shell = CreateShell(settings); + // We MUST put |start| after |CreateShell| because the clock source will be + // reset through |TimePoint::SetClockSource()| in + // |DartVMInitializer::Initialize()| within |CreateShell()|. + fml::TimePoint start = fml::TimePoint::Now(); + // Create the surface needed by rasterizer PlatformViewNotifyCreated(shell.get()); @@ -726,9 +730,14 @@ TEST_F(ShellTest, ReportTimingsIsCalled) { } TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { + auto settings = CreateSettingsForFixture(); + std::unique_ptr shell = CreateShell(settings); + + // We MUST put |start| after |CreateShell()| because the clock source will be + // reset through |TimePoint::SetClockSource()| in + // |DartVMInitializer::Initialize()| within |CreateShell()|. fml::TimePoint start = fml::TimePoint::Now(); - auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent timingLatch; FrameTiming timing; @@ -745,8 +754,6 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { timingLatch.Signal(); }; - std::unique_ptr shell = CreateShell(settings); - // Create the surface needed by rasterizer PlatformViewNotifyCreated(shell.get()); From 3c0ae52cfe1702f1924459453a7d880a462676e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=BE=E9=94=A6=E5=92=8C=28=E6=99=BA=E9=B9=B0=29?= Date: Wed, 31 May 2023 10:43:53 +0800 Subject: [PATCH 2/4] The builder machine is too fast. --- shell/common/shell_unittests.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 2f5ce0a92b7f7..775ef1c713078 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -5,9 +5,11 @@ #define FML_USED_ON_EMBEDDER #include +#include #include #include #include +#include #include #include @@ -51,6 +53,8 @@ #include "flutter/vulkan/vulkan_application.h" // nogncheck #endif +using namespace std::chrono_literals; + // CREATE_NATIVE_ENTRY is leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) @@ -733,6 +737,9 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { auto settings = CreateSettingsForFixture(); std::unique_ptr shell = CreateShell(settings); + // Wait to make |start| bigger than zero + std::this_thread::sleep_for(1ms); + // We MUST put |start| after |CreateShell()| because the clock source will be // reset through |TimePoint::SetClockSource()| in // |DartVMInitializer::Initialize()| within |CreateShell()|. From e7519c049f49de19dc1febef1829b43a0ce12419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=BE=E9=94=A6=E5=92=8C=28=E6=99=BA=E9=B9=B0=29?= Date: Wed, 31 May 2023 11:18:08 +0800 Subject: [PATCH 3/4] Fix infinite waiting. --- shell/common/shell_unittests.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 775ef1c713078..ec1d17c509adc 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -735,6 +735,15 @@ TEST_F(ShellTest, ReportTimingsIsCalled) { TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { auto settings = CreateSettingsForFixture(); + + FrameTiming timing; + fml::AutoResetWaitableEvent timingLatch; + settings.frame_rasterized_callback = [&timing, + &timingLatch](const FrameTiming& t) { + timing = t; + timingLatch.Signal(); + }; + std::unique_ptr shell = CreateShell(settings); // Wait to make |start| bigger than zero @@ -745,9 +754,6 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { // |DartVMInitializer::Initialize()| within |CreateShell()|. fml::TimePoint start = fml::TimePoint::Now(); - fml::AutoResetWaitableEvent timingLatch; - FrameTiming timing; - for (auto phase : FrameTiming::kPhases) { timing.Set(phase, fml::TimePoint()); // Check that the time points are initially smaller than start, so @@ -755,12 +761,6 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { ASSERT_TRUE(timing.Get(phase) < start); } - settings.frame_rasterized_callback = [&timing, - &timingLatch](const FrameTiming& t) { - timing = t; - timingLatch.Signal(); - }; - // Create the surface needed by rasterizer PlatformViewNotifyCreated(shell.get()); From ba4aed534af26f800a57ba8d2032df6022445413 Mon Sep 17 00:00:00 2001 From: toneyzeng Date: Fri, 2 Jun 2023 16:33:48 +0800 Subject: [PATCH 4/4] Update shell_unittests.cc moving namespace to the right scope --- shell/common/shell_unittests.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index ec1d17c509adc..7f9473a2b1a2d 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -53,8 +53,6 @@ #include "flutter/vulkan/vulkan_application.h" // nogncheck #endif -using namespace std::chrono_literals; - // CREATE_NATIVE_ENTRY is leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) @@ -747,6 +745,7 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { std::unique_ptr shell = CreateShell(settings); // Wait to make |start| bigger than zero + using namespace std::chrono_literals; std::this_thread::sleep_for(1ms); // We MUST put |start| after |CreateShell()| because the clock source will be