Disallow time traveling frame times (flutter/engine#55310)

Address bad developer experience in
https://github.com/flutter/flutter/issues/106277

Leave as an error log and hope for more repro reports


```mermaid
sequenceDiagram
  Animator ->> Animator: AwaitVSync
  Animator ->> VsyncWaiter: AsyncWaitForVsync(callback)
  VsyncWaiter -> VsyncWaiterAndroid: AwaitVSync
  note over VsyncWaiterAndroid: GetUITaskRunner
  VsyncWaiterAndroid -> Choreographer: PostFrameCallback
  Choreographer -> NDK: AChoreographer_postFrameCallback64
  note over Choreographer,NDK: The time that the frame is being<br/>rendered as nanoseconds in the <br/>CLOCK_MONOTONIC time base
  NDK --> Choreographer: callback(nanos)
  Choreographer -> VsyncWaiterAndroid: callback
  note over VsyncWaiterAndroid: // Rollback suspicion<br/>if (frame_time > now) frame_time = now;
  VsyncWaiterAndroid -> VsyncWaiterAndroid: OnVsyncFromNDK(frame_nanos)
  VsyncWaiterAndroid -> VsyncWaiter: FireCallback(\n  frame_start_time,\n  target_time)
  VsyncWaiter -> Animator: callback(frame_timings_recorder)

  Animator -> Animator: BeginFrame(frame_timings_recorder)
  Animator -> Shell: OnAnimatorBeginFrame
  Shell -> Engine: BeginFrame(frame_time, frame_number)
  Engine -> RuntimeController: BeginFrame(frame_time, frame_number)
  RuntimeController -> PlatformConfiguration: BeginFrame(frame_time, frame_number)
  PlatformConfiguration -> hooks.dart: begin_frame_
```
This commit is contained in:
John McDole
2024-09-24 15:33:45 -07:00
committed by GitHub
parent d333cf4224
commit 14caa2fe19
6 changed files with 140 additions and 5 deletions

View File

@@ -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_

View File

@@ -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<void> test(String name, FutureOr<void> Function() testFunction) async {

View File

@@ -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(), {

View File

@@ -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<int64_t, ViewportMetrics> metrics_;

View File

@@ -4,17 +4,21 @@
#define FML_USED_ON_EMBEDDER
#include "flutter/lib/ui/window/platform_configuration.h"
#include <cstddef>
#include <memory>
#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<fml::AutoResetWaitableEvent>();
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<int64_t> frame_times;
std::vector<uint64_t> frame_numbers;
auto frame_latch = std::make_shared<fml::AutoResetWaitableEvent>();
// 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), &microseconds);
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> 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

View File

@@ -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<VsyncWaiter>(shared_from_this());
fml::TaskRunner::RunNowOrPostTask(
task_runners_.GetUITaskRunner(), [weak_this]() {