Listen for display refresh changes and report them correctly

ci/licenses_golden/licenses_flutter

@@ -694,6 +694,7 @@ FILE: ../../../flutter/shell/common/canvas_spy_unittests.cc
 FILE: ../../../flutter/shell/common/context_options.cc
 FILE: ../../../flutter/shell/common/context_options.h
 FILE: ../../../flutter/shell/common/dart_native_benchmarks.cc
+FILE: ../../../flutter/shell/common/display.cc
 FILE: ../../../flutter/shell/common/display.h
 FILE: ../../../flutter/shell/common/display_manager.cc
 FILE: ../../../flutter/shell/common/display_manager.h
@@ -773,6 +774,8 @@ FILE: ../../../flutter/shell/platform/android/AndroidManifest.xml
 FILE: ../../../flutter/shell/platform/android/android_context_gl.cc
 FILE: ../../../flutter/shell/platform/android/android_context_gl.h
 FILE: ../../../flutter/shell/platform/android/android_context_gl_unittests.cc
+FILE: ../../../flutter/shell/platform/android/android_display.cc
+FILE: ../../../flutter/shell/platform/android/android_display.h
 FILE: ../../../flutter/shell/platform/android/android_environment_gl.cc
 FILE: ../../../flutter/shell/platform/android/android_environment_gl.h
 FILE: ../../../flutter/shell/platform/android/android_exports.lst

shell/common/BUILD.gn

@@ -66,6 +66,7 @@ source_set("common") {
     "canvas_spy.h",
     "context_options.cc",
     "context_options.h",
+    "display.cc",
     "display.h",
     "display_manager.cc",
     "display_manager.h",

shell/common/display.cc

@@ -0,0 +1,11 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/common/display.h"
+
+namespace flutter {
+double Display::GetRefreshRate() const {
+  return refresh_rate_;
+}
+}  // namespace flutter

shell/common/display.h

@@ -7,6 +7,8 @@
 
 #include <optional>
 
+#include "flutter/fml/macros.h"
+
 namespace flutter {
 
 /// Unique ID per display that is stable until the Flutter application restarts.
@@ -36,18 +38,20 @@ class Display {
   explicit Display(double refresh_rate)
       : display_id_({}), refresh_rate_(refresh_rate) {}
 
-  ~Display() = default;
+  virtual ~Display() = default;
 
   // Get the display's maximum refresh rate in the unit of frame per second.
   // Return `kUnknownDisplayRefreshRate` if the refresh rate is unknown.
-  double GetRefreshRate() const { return refresh_rate_; }
+  virtual double GetRefreshRate() const;
 
   /// Returns the `DisplayId` of the display.
   std::optional<DisplayId> GetDisplayId() const { return display_id_; }
 
  private:
   std::optional<DisplayId> display_id_;
   double refresh_rate_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(Display);
 };
 
 }  // namespace flutter

shell/common/display_manager.cc

@@ -18,30 +18,31 @@ double DisplayManager::GetMainDisplayRefreshRate() const {
   if (displays_.empty()) {
     return kUnknownDisplayRefreshRate;
   } else {
-    return displays_[0].GetRefreshRate();
+    return displays_[0]->GetRefreshRate();
   }
 }
 
-void DisplayManager::HandleDisplayUpdates(DisplayUpdateType update_type,
-                                          std::vector<Display> displays) {
+void DisplayManager::HandleDisplayUpdates(
+    DisplayUpdateType update_type,
+    std::vector<std::unique_ptr<Display>> displays) {
   std::scoped_lock lock(displays_mutex_);
   CheckDisplayConfiguration(displays);
   switch (update_type) {
     case DisplayUpdateType::kStartup:
       FML_CHECK(displays_.empty());
-      displays_ = displays;
+      displays_ = std::move(displays);
       return;
     default:
       FML_CHECK(false) << "Unknown DisplayUpdateType.";
   }
 }
 
 void DisplayManager::CheckDisplayConfiguration(
-    std::vector<Display> displays) const {
+    const std::vector<std::unique_ptr<Display>>& displays) const {
   FML_CHECK(!displays.empty());
   if (displays.size() > 1) {
     for (auto& display : displays) {
-      FML_CHECK(display.GetDisplayId().has_value());
+      FML_CHECK(display->GetDisplayId().has_value());
     }
   }
 }

shell/common/display_manager.h

@@ -40,18 +40,19 @@ class DisplayManager {
 
   /// Handles the display updates.
   void HandleDisplayUpdates(DisplayUpdateType update_type,
-                            std::vector<Display> displays);
+                            std::vector<std::unique_ptr<Display>> displays);
 
  private:
   /// Guards `displays_` vector.
   mutable std::mutex displays_mutex_;
-  std::vector<Display> displays_;
+  std::vector<std::unique_ptr<Display>> displays_;
 
   /// Checks that the provided display configuration is valid. Currently this
   /// ensures that all the displays have an id in the case there are multiple
   /// displays. In case where there is a single display, it is valid for the
   /// display to not have an id.
-  void CheckDisplayConfiguration(std::vector<Display> displays) const;
+  void CheckDisplayConfiguration(
+      const std::vector<std::unique_ptr<Display>>& displays) const;
 };
 
 }  // namespace flutter

shell/common/shell.cc

@@ -1867,8 +1867,8 @@ void Shell::SetGpuAvailability(GpuAvailability availability) {
 }
 
 void Shell::OnDisplayUpdates(DisplayUpdateType update_type,
-                             std::vector<Display> displays) {
-  display_manager_->HandleDisplayUpdates(update_type, displays);
+                             std::vector<std::unique_ptr<Display>> displays) {
+  display_manager_->HandleDisplayUpdates(update_type, std::move(displays));
 }
 
 fml::TimePoint Shell::GetCurrentTimePoint() {

shell/common/shell.h

@@ -373,7 +373,7 @@ class Shell final : public PlatformView::Delegate,
   /// @brief      Notifies the display manager of the updates.
   ///
   void OnDisplayUpdates(DisplayUpdateType update_type,
-                        std::vector<Display> displays);
+                        std::vector<std::unique_ptr<Display>> displays);
 
   //----------------------------------------------------------------------------
   /// @brief Queries the `DisplayManager` for the main display refresh rate.

shell/platform/android/BUILD.gn

@@ -64,6 +64,8 @@ source_set("flutter_shell_native_src") {
     "$root_build_dir/flutter_icu/icudtl.o",
     "android_context_gl.cc",
     "android_context_gl.h",
+    "android_display.cc",
+    "android_display.h",
     "android_environment_gl.cc",
     "android_environment_gl.h",
     "android_external_texture_gl.cc",

shell/platform/android/android_display.cc

@@ -0,0 +1,19 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/platform/android/android_display.h"
+#include "android_display.h"
+
+namespace flutter {
+
+AndroidDisplay::AndroidDisplay(
+    std::shared_ptr<PlatformViewAndroidJNI> jni_facade)
+    : Display(jni_facade->GetDisplayRefreshRate()),
+      jni_facade_(std::move(jni_facade)) {}
+
+double AndroidDisplay::GetRefreshRate() const {
+  return jni_facade_->GetDisplayRefreshRate();
+}
+
+}  // namespace flutter

shell/platform/android/android_display.h

@@ -0,0 +1,33 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_DISPLAY_H_
+#define FLUTTER_SHELL_PLATFORM_ANDROID_DISPLAY_H_
+
+#include <cstdint>
+
+#include "flutter/fml/macros.h"
+#include "flutter/shell/common/display.h"
+#include "flutter/shell/platform/android/jni/platform_view_android_jni.h"
+
+namespace flutter {
+
+/// A |Display| that listens to refresh rate changes.
+class AndroidDisplay : public Display {
+ public:
+  explicit AndroidDisplay(std::shared_ptr<PlatformViewAndroidJNI> jni_facade);
+  ~AndroidDisplay() = default;
+
+  // |Display|
+  double GetRefreshRate() const override;
+
+ private:
+  std::shared_ptr<PlatformViewAndroidJNI> jni_facade_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(AndroidDisplay);
+};
+
+}  // namespace flutter
+
+#endif  // FLUTTER_SHELL_PLATFORM_ANDROID_DISPLAY_H_

shell/platform/android/android_shell_holder.cc

@@ -25,6 +25,7 @@
 #include "flutter/shell/common/rasterizer.h"
 #include "flutter/shell/common/run_configuration.h"
 #include "flutter/shell/common/thread_host.h"
+#include "flutter/shell/platform/android/android_display.h"
 #include "flutter/shell/platform/android/android_image_generator.h"
 #include "flutter/shell/platform/android/context/android_context.h"
 #include "flutter/shell/platform/android/platform_view_android.h"
@@ -61,8 +62,10 @@ AndroidShellHolder::AndroidShellHolder(
                 .enable_software_rendering  // use software rendering
         );
         weak_platform_view = platform_view_android->GetWeakPtr();
-        auto display = Display(jni_facade->GetDisplayRefreshRate());
-        shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display});
+        std::vector<std::unique_ptr<Display>> displays;
+        displays.push_back(std::make_unique<AndroidDisplay>(jni_facade));
+        shell.OnDisplayUpdates(DisplayUpdateType::kStartup,
+                               std::move(displays));
         return platform_view_android;
       };
 
@@ -209,8 +212,10 @@ std::unique_ptr<AndroidShellHolder> AndroidShellHolder::Spawn(
             android_context          // Android context
         );
         weak_platform_view = platform_view_android->GetWeakPtr();
-        auto display = Display(jni_facade->GetDisplayRefreshRate());
-        shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display});
+        std::vector<std::unique_ptr<Display>> displays;
+        displays.push_back(std::make_unique<AndroidDisplay>(jni_facade));
+        shell.OnDisplayUpdates(DisplayUpdateType::kStartup,
+                               std::move(displays));
         return platform_view_android;
       };
 

shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

@@ -186,9 +186,15 @@ private static native void nativeInit(
   // END methods related to FlutterLoader
 
   @Nullable private static AsyncWaitForVsyncDelegate asyncWaitForVsyncDelegate;
-  // This should also be updated by FlutterView when it is attached to a Display.
-  // The initial value of 0.0 indicates unknown refresh rate.
-  private static float refreshRateFPS = 0.0f;
+
+  /**
+   * This value is updated by the VsyncWaiter when it is initialized.
+   *
+   * <p>On API 17+, it is updated whenever the default display refresh rate changes.
+   *
+   * <p>It is defaulted to 60.
+   */
+  private static float refreshRateFPS = 60.0f;
 
   // This is set from native code via JNI.
   @Nullable private static String observatoryUri;
@@ -216,19 +222,34 @@ public static String getObservatoryUri() {
     return observatoryUri;
   }
 
-  public static void setRefreshRateFPS(float refreshRateFPS) {
-    if (FlutterJNI.setRefreshRateFPSCalled) {
-      Log.w(TAG, "FlutterJNI.setRefreshRateFPS called more than once");
-    }
-
+  /**
+   * Notifies the engine about the refresh rate of the display when the API level is below 30.
+   *
+   * <p>For API 30 and above, this value is ignored.
+   *
+   * <p>Calling this method multiple times will update the refresh rate for the next vsync period.
+   * However, callers should avoid calling {@link android.view.Display#getRefreshRate} frequently,
+   * since it is expensive on some vendor implementations.
+   *
+   * @param refreshRateFPS The refresh rate in nanoseconds.
+   */
+  public void setRefreshRateFPS(float refreshRateFPS) {
+    // This is ok because it only ever tracks the refresh rate of the main
+    // display. If we ever need to support the refresh rate of other displays
+    // on Android we will need to refactor this. Static lookup makes things a
+    // bit easier on the C++ side.
     FlutterJNI.refreshRateFPS = refreshRateFPS;
-    FlutterJNI.setRefreshRateFPSCalled = true;
   }
 
-  private static boolean setRefreshRateFPSCalled = false;
-
-  // TODO(mattcarroll): add javadocs
-  public static void setAsyncWaitForVsyncDelegate(@Nullable AsyncWaitForVsyncDelegate delegate) {
+  /**
+   * The Android vsync waiter implementation in C++ needs to know when a vsync signal arrives, which
+   * is obtained via Java API. The delegate set here is called on the C++ side when the engine is
+   * ready to wait for the next vsync signal. The delegate is expected to add a postFrameCallback to
+   * the {@link android.view.Choreographer}, and call {@link nativeOnVsync} to notify the engine.
+   *
+   * @param delegate The delegate that will call the engine back on the next vsync signal.
+   */
+  public void setAsyncWaitForVsyncDelegate(@Nullable AsyncWaitForVsyncDelegate delegate) {
     asyncWaitForVsyncDelegate = delegate;
   }
 
@@ -243,9 +264,15 @@ private static void asyncWaitForVsync(final long cookie) {
     }
   }
 
-  // TODO(mattcarroll): add javadocs
-  public static native void nativeOnVsync(
-      long frameDelayNanos, long refreshPeriodNanos, long cookie);
+  /**
+   * Notifies the engine that the Choreographer has signaled a vsync.
+   *
+   * @param frameDelayNanos The time in nanoseconds when the frame started being rendered,
+   *     subtracted from the {@link System#nanoTime} timebase.
+   * @param refreshPeriodNanos The display refresh period in nanoseconds.
+   * @param cookie An opaque handle to the C++ VSyncWaiter object.
+   */
+  public native void nativeOnVsync(long frameDelayNanos, long refreshPeriodNanos, long cookie);
 
   // TODO(mattcarroll): add javadocs
   @NonNull
@@ -337,8 +364,7 @@ public long performNativeAttach(@NonNull FlutterJNI flutterJNI) {
    * #attachToNative()}.
    *
    * <p>Static methods that should be only called once such as {@link #init(Context, String[],
-   * String, String, String, long)} or {@link #setRefreshRateFPS(float)} shouldn't be called again
-   * on the spawned FlutterJNI instance.
+   * String, String, String, long)} shouldn't be called again on the spawned FlutterJNI instance.
    */
   @UiThread
   @NonNull

shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java

@@ -15,7 +15,6 @@
 import android.os.Handler;
 import android.os.Looper;
 import android.os.SystemClock;
-import android.view.Display;
 import android.view.WindowManager;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -148,18 +147,19 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se
       initStartTimestampMillis = SystemClock.uptimeMillis();
       flutterApplicationInfo = ApplicationInfoLoader.load(appContext);
 
-      float fps;
-      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-        final DisplayManager dm = appContext.getSystemService(DisplayManager.class);
-        final Display primaryDisplay = dm.getDisplay(Display.DEFAULT_DISPLAY);
-        fps = primaryDisplay.getRefreshRate();
+      VsyncWaiter waiter;
+      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1 /* 17 */) {
+        final DisplayManager dm =
+            (DisplayManager) appContext.getSystemService(Context.DISPLAY_SERVICE);
+        waiter = VsyncWaiter.getInstance(dm, flutterJNI);
       } else {
-        fps =
+        float fps =
             ((WindowManager) appContext.getSystemService(Context.WINDOW_SERVICE))
                 .getDefaultDisplay()
                 .getRefreshRate();
+        waiter = VsyncWaiter.getInstance(fps, flutterJNI);
       }
-      VsyncWaiter.getInstance(fps).init();
+      waiter.init();
 
       // Use a background thread for initialization tasks that require disk access.
       Callable<InitResult> initTask =