Add tests and move to class variable

Author: jtmcdole

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_

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

lib/ui/window/platform_configuration.cc

@@ -377,18 +377,25 @@ void PlatformConfiguration::BeginFrame(fml::TimePoint frameTime,
   }
   tonic::DartState::Scope scope(dart_state);
 
-  int64_t microseconds = frameTime.ToEpochDelta().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;
 
-  static int64_t last_microseconds = 0;
-  if (last_microseconds > microseconds) {
+  // 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;
+        << microseconds << " < " << last_microseconds_
+        << " ~= " << last_microseconds_ - microseconds;
+    microseconds = last_microseconds_;
   }
+  last_microseconds_ = microseconds;
 
   tonic::CheckAndHandleError(
       tonic::DartInvoke(begin_frame_.Get(), {

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_;
+  int64_t last_microseconds_;
+
   // All current views' view metrics mapped from view IDs.
   std::unordered_map<int64_t, ViewportMetrics> metrics_;
 

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