diff --git a/fml/macros.h b/fml/macros.h index 7de4ddf1f1f42..291e193ee0969 100644 --- a/fml/macros.h +++ b/fml/macros.h @@ -38,7 +38,13 @@ TypeName() = delete; \ FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TypeName) +#define FML_TEST_NAME(test_case_name, test_name) \ + test_case_name##_##test_name##_Test + +#define FML_TEST_CLASS(test_case_name, test_name) \ + class FML_TEST_NAME(test_case_name, test_name) + #define FML_FRIEND_TEST(test_case_name, test_name) \ - friend class test_case_name##_##test_name##_Test + friend FML_TEST_CLASS(test_case_name, test_name) #endif // FLUTTER_FML_MACROS_H_ diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index a02e0ce29499b..19688d108a647 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -492,6 +492,16 @@ void convertPaintToDlPaint() { @pragma('vm:external-name', 'ConvertPaintToDlPaint') external void _convertPaintToDlPaint(Paint paint); +/// Hooks for platform_configuration_unittests.cc +@pragma('vm:entry-point') +void _beginFrameHijack(int microseconds, int frameNumber) { + nativeBeginFrame(microseconds, frameNumber); +} + +@pragma('vm:entry-point') +@pragma('vm:external-name', 'BeginFrame') +external nativeBeginFrame(int microseconds, int frameNumber); + @pragma('vm:entry-point') void hooksTests() async { Future test(String name, FutureOr Function() testFunction) async { diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index e6a934a63c69b..a63d098726e5b 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -377,7 +377,25 @@ void PlatformConfiguration::BeginFrame(fml::TimePoint frameTime, } tonic::DartState::Scope scope(dart_state); - int64_t microseconds = (frameTime - fml::TimePoint()).ToMicroseconds(); + if (last_frame_number_ > frame_number) { + FML_LOG(ERROR) << "Frame number is out of order: " << frame_number << " < " + << last_frame_number_; + } + last_frame_number_ = frame_number; + + // frameTime is not a delta; its the timestamp of the presentation. + // This is just a type conversion. + int64_t microseconds = frameTime.ToEpochDelta().ToMicroseconds(); + if (last_microseconds_ > microseconds) { + // Do not allow time traveling frametimes + // github.com/flutter/flutter/issues/106277 + FML_LOG(ERROR) + << "Reported frame time is older than the last one; clamping. " + << microseconds << " < " << last_microseconds_ + << " ~= " << last_microseconds_ - microseconds; + microseconds = last_microseconds_; + } + last_microseconds_ = microseconds; tonic::CheckAndHandleError( tonic::DartInvoke(begin_frame_.Get(), { diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index b668b85cfd603..6d227ce347ed8 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -18,6 +18,7 @@ #include "flutter/lib/ui/window/pointer_data_packet.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/shell/common/display.h" +#include "fml/macros.h" #include "third_party/tonic/dart_persistent_value.h" #include "third_party/tonic/typed_data/dart_byte_data.h" @@ -28,6 +29,11 @@ class PlatformMessageHandler; class PlatformIsolateManager; class Scene; +// Forward declaration of friendly tests. +namespace testing { +FML_TEST_CLASS(PlatformConfigurationTest, BeginFrameMonotonic); +} + //-------------------------------------------------------------------------- /// @brief An enum for defining the different kinds of accessibility features /// that can be enabled by the platform. @@ -517,6 +523,8 @@ class PlatformConfiguration final { Dart_Handle on_error() { return on_error_.Get(); } private: + FML_FRIEND_TEST(testing::PlatformConfigurationTest, BeginFrameMonotonic); + PlatformConfigurationClient* client_; tonic::DartPersistentValue on_error_; tonic::DartPersistentValue add_view_; @@ -535,6 +543,9 @@ class PlatformConfiguration final { tonic::DartPersistentValue draw_frame_; tonic::DartPersistentValue report_timings_; + uint64_t last_frame_number_ = 0; + int64_t last_microseconds_ = 0; + // All current views' view metrics mapped from view IDs. std::unordered_map metrics_; diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 7410caeb66d6c..e181a27963dc9 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -4,17 +4,21 @@ #define FML_USED_ON_EMBEDDER -#include "flutter/lib/ui/window/platform_configuration.h" - +#include #include #include "flutter/common/task_runners.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/painting/vertices.h" +#include "flutter/lib/ui/window/platform_configuration.h" #include "flutter/runtime/dart_vm.h" #include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/testing.h" +#include "gmock/gmock.h" +#include "googletest/googletest/include/gtest/gtest.h" +#include "third_party/dart/runtime/include/dart_api.h" +#include "tonic/converter/dart_converter.h" namespace flutter { namespace testing { @@ -332,5 +336,89 @@ TEST_F(PlatformConfigurationTest, SetDartPerformanceMode) { DestroyShell(std::move(shell), task_runners); } +TEST_F(PlatformConfigurationTest, BeginFrameMonotonic) { + auto message_latch = std::make_shared(); + + PlatformConfiguration* platform; + + // Called at the load time and will be in an Dart isolate. + auto nativeValidateConfiguration = [message_latch, + &platform](Dart_NativeArguments args) { + platform = UIDartState::Current()->platform_configuration(); + + // Hijacks the `_begin_frame` in hooks.dart so we can get a callback for + // validation. + auto field = + Dart_GetField(Dart_RootLibrary(), tonic::ToDart("_beginFrameHijack")); + platform->begin_frame_.Clear(); + platform->begin_frame_.Set(UIDartState::Current(), field); + + message_latch->Signal(); + }; + AddNativeCallback("ValidateConfiguration", + CREATE_NATIVE_ENTRY(nativeValidateConfiguration)); + + std::vector frame_times; + std::vector frame_numbers; + + auto frame_latch = std::make_shared(); + + // Called for each `_begin_frame` that is hijacked. + auto nativeBeginFrame = [frame_latch, &frame_times, + &frame_numbers](Dart_NativeArguments args) { + int64_t microseconds; + uint64_t frame_number; + Dart_IntegerToInt64(Dart_GetNativeArgument(args, 0), µseconds); + Dart_IntegerToUint64(Dart_GetNativeArgument(args, 1), &frame_number); + + frame_times.push_back(microseconds); + frame_numbers.push_back(frame_number); + + if (frame_times.size() == 3) { + frame_latch->Signal(); + } + }; + AddNativeCallback("BeginFrame", CREATE_NATIVE_ENTRY(nativeBeginFrame)); + + Settings settings = CreateSettingsForFixture(); + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + std::unique_ptr shell = CreateShell(settings, task_runners); + ASSERT_TRUE(shell->IsSetup()); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("validateConfiguration"); + + shell->RunEngine(std::move(configuration), [&](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + // Wait for `nativeValidateConfiguration` to get called. + message_latch->Wait(); + + fml::TaskRunner::RunNowOrPostTask( + shell->GetTaskRunners().GetUITaskRunner(), [platform]() { + auto offset = fml::TimeDelta::FromMilliseconds(10); + auto zero = fml::TimePoint(); + auto one = zero + offset; + auto two = one + offset; + + platform->BeginFrame(zero, 1); + platform->BeginFrame(two, 2); + platform->BeginFrame(one, 3); + }); + + frame_latch->Wait(); + + ASSERT_THAT(frame_times, ::testing::ElementsAre(0, 20000, 20000)); + ASSERT_THAT(frame_numbers, ::testing::ElementsAre(1, 2, 3)); + DestroyShell(std::move(shell), task_runners); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/android/vsync_waiter_android.cc b/shell/platform/android/vsync_waiter_android.cc index b69e3389d89ec..1c46b7dbf3175 100644 --- a/shell/platform/android/vsync_waiter_android.cc +++ b/shell/platform/android/vsync_waiter_android.cc @@ -27,7 +27,9 @@ VsyncWaiterAndroid::~VsyncWaiterAndroid() = default; // |VsyncWaiter| void VsyncWaiterAndroid::AwaitVSync() { - if (impeller::android::Choreographer::IsAvailableOnPlatform()) { + const static bool use_choreographer = + impeller::android::Choreographer::IsAvailableOnPlatform(); + if (use_choreographer) { auto* weak_this = new std::weak_ptr(shared_from_this()); fml::TaskRunner::RunNowOrPostTask( task_runners_.GetUITaskRunner(), [weak_this]() {