Migrate vsync_waiter_ios to ARC

Author: jmagman

shell/platform/darwin/ios/BUILD.gn

@@ -76,14 +76,20 @@ source_set("flutter_framework_source_arc") {
     "framework/Source/KeyCodeMap_Internal.h",
     "framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.h",
     "framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.mm",
+    "framework/Source/vsync_waiter_ios.h",
+    "framework/Source/vsync_waiter_ios.mm",
   ]
 
   frameworks = [
     "UIKit.framework",
     "IOSurface.framework",
   ]
 
-  deps += [ "//flutter/shell/platform/embedder:embedder_as_internal_library" ]
+  deps += [
+    "//flutter/common:common",
+    "//flutter/shell/common",
+    "//flutter/shell/platform/embedder:embedder_as_internal_library",
+  ]
 }
 
 source_set("flutter_framework_source") {
@@ -145,8 +151,6 @@ source_set("flutter_framework_source") {
     "framework/Source/platform_message_response_darwin.mm",
     "framework/Source/profiler_metrics_ios.h",
     "framework/Source/profiler_metrics_ios.mm",
-    "framework/Source/vsync_waiter_ios.h",
-    "framework/Source/vsync_waiter_ios.mm",
     "ios_context.h",
     "ios_context.mm",
     "ios_context_metal_impeller.h",

shell/platform/darwin/ios/framework/Source/VsyncWaiterIosTest.mm

@@ -127,4 +127,20 @@ - (void)testAwaitAndPauseWillWorkCorrectly {
   XCTAssertTrue(link.isPaused);
 }
 
+- (void)testReleasesLinkOnInvalidation {
+  __weak CADisplayLink* weakLink;
+  @autoreleasepool {
+    auto thread_task_runner = CreateNewThread("VsyncWaiterIosTest");
+    VSyncClient* vsyncClient = [[VSyncClient alloc]
+        initWithTaskRunner:thread_task_runner
+                  callback:[](std::unique_ptr<flutter::FrameTimingsRecorder> recorder) {}];
+
+    weakLink = [vsyncClient getDisplayLink];
+    XCTAssertNotNil(weakLink);
+    [vsyncClient invalidate];
+  }
+  // VSyncClient has released the CADisplayLink.
+  XCTAssertNil(weakLink);
+}
+
 @end

shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.h

@@ -8,8 +8,6 @@
 #include <QuartzCore/CADisplayLink.h>
 
 #include "flutter/fml/macros.h"
-#include "flutter/fml/memory/weak_ptr.h"
-#include "flutter/fml/platform/darwin/scoped_nsobject.h"
 #include "flutter/shell/common/variable_refresh_rate_reporter.h"
 #include "flutter/shell/common/vsync_waiter.h"
 
@@ -73,15 +71,12 @@ class VsyncWaiterIOS final : public VsyncWaiter, public VariableRefreshRateRepor
   // |VariableRefreshRateReporter|
   double GetRefreshRate() const override;
 
-  // Made public for testing.
-  fml::scoped_nsobject<VSyncClient> GetVsyncClient() const;
-
   // |VsyncWaiter|
   // Made public for testing.
   void AwaitVSync() override;
 
  private:
-  fml::scoped_nsobject<VSyncClient> client_;
+  VSyncClient* client_;
   double max_refresh_rate_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(VsyncWaiterIOS);

shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.mm

@@ -27,41 +27,36 @@
     const fml::TimePoint target_time = recorder->GetVsyncTargetTime();
     FireCallback(start_time, target_time, true);
   };
-  client_ =
-      fml::scoped_nsobject{[[VSyncClient alloc] initWithTaskRunner:task_runners_.GetUITaskRunner()
-                                                          callback:callback]};
+  client_ = [[VSyncClient alloc] initWithTaskRunner:task_runners_.GetUITaskRunner()
+                                           callback:callback];
   max_refresh_rate_ = [DisplayLinkManager displayRefreshRate];
 }
 
 VsyncWaiterIOS::~VsyncWaiterIOS() {
   // This way, we will get no more callbacks from the display link that holds a weak (non-nilling)
   // reference to this C++ object.
-  [client_.get() invalidate];
+  [client_ invalidate];
 }
 
 void VsyncWaiterIOS::AwaitVSync() {
   double new_max_refresh_rate = [DisplayLinkManager displayRefreshRate];
   if (fabs(new_max_refresh_rate - max_refresh_rate_) > kRefreshRateDiffToIgnore) {
     max_refresh_rate_ = new_max_refresh_rate;
-    [client_.get() setMaxRefreshRate:max_refresh_rate_];
+    [client_ setMaxRefreshRate:max_refresh_rate_];
   }
-  [client_.get() await];
+  [client_ await];
 }
 
 // |VariableRefreshRateReporter|
 double VsyncWaiterIOS::GetRefreshRate() const {
-  return [client_.get() getRefreshRate];
-}
-
-fml::scoped_nsobject<VSyncClient> VsyncWaiterIOS::GetVsyncClient() const {
-  return client_;
+  return [client_ getRefreshRate];
 }
 
 }  // namespace flutter
 
 @implementation VSyncClient {
   flutter::VsyncWaiter::Callback callback_;
-  fml::scoped_nsobject<CADisplayLink> display_link_;
+  CADisplayLink* _displayLink;
   double current_refresh_rate_;
 }
 
@@ -73,17 +68,15 @@ - (instancetype)initWithTaskRunner:(fml::RefPtr<fml::TaskRunner>)task_runner
     current_refresh_rate_ = [DisplayLinkManager displayRefreshRate];
     _allowPauseAfterVsync = YES;
     callback_ = std::move(callback);
-    display_link_ = fml::scoped_nsobject<CADisplayLink> {
-      [[CADisplayLink displayLinkWithTarget:self selector:@selector(onDisplayLink:)] retain]
-    };
-    display_link_.get().paused = YES;
+    _displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(onDisplayLink:)];
+    _displayLink.paused = YES;
 
     [self setMaxRefreshRate:[DisplayLinkManager displayRefreshRate]];
 
-    task_runner->PostTask([client = [self retain]]() {
-      [client->display_link_.get() addToRunLoop:[NSRunLoop currentRunLoop]
-                                        forMode:NSRunLoopCommonModes];
-      [client release];
+    // Strongly retain the the captured link until it is added to the runloop.
+    CADisplayLink* localDisplayLink = _displayLink;
+    task_runner->PostTask([localDisplayLink]() {
+      [localDisplayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSRunLoopCommonModes];
     });
   }
 
@@ -97,19 +90,19 @@ - (void)setMaxRefreshRate:(double)refreshRate {
   double maxFrameRate = fmax(refreshRate, 60);
   double minFrameRate = fmax(maxFrameRate / 2, 60);
   if (@available(iOS 15.0, *)) {
-    display_link_.get().preferredFrameRateRange =
+    _displayLink.preferredFrameRateRange =
         CAFrameRateRangeMake(minFrameRate, maxFrameRate, maxFrameRate);
   } else {
-    display_link_.get().preferredFramesPerSecond = maxFrameRate;
+    _displayLink.preferredFramesPerSecond = maxFrameRate;
   }
 }
 
 - (void)await {
-  display_link_.get().paused = NO;
+  _displayLink.paused = NO;
 }
 
 - (void)pause {
-  display_link_.get().paused = YES;
+  _displayLink.paused = YES;
 }
 
 - (void)onDisplayLink:(CADisplayLink*)link {
@@ -130,40 +123,37 @@ - (void)onDisplayLink:(CADisplayLink*)link {
 
   recorder->RecordVsync(frame_start_time, frame_target_time);
   if (_allowPauseAfterVsync) {
-    display_link_.get().paused = YES;
+    link.paused = YES;
   }
   callback_(std::move(recorder));
 }
 
 - (void)invalidate {
-  [display_link_.get() invalidate];
+  [_displayLink invalidate];
+  _displayLink = nil;  // Break retain cycle.
 }
 
 - (void)dealloc {
-  [self invalidate];
-
-  [super dealloc];
+  [_displayLink invalidate];
 }
 
 - (double)getRefreshRate {
   return current_refresh_rate_;
 }
 
 - (CADisplayLink*)getDisplayLink {
-  return display_link_.get();
+  return _displayLink;
 }
 
 @end
 
 @implementation DisplayLinkManager
 
 + (double)displayRefreshRate {
-  fml::scoped_nsobject<CADisplayLink> display_link = fml::scoped_nsobject<CADisplayLink> {
-    [[CADisplayLink displayLinkWithTarget:[[[DisplayLinkManager alloc] init] autorelease]
-                                 selector:@selector(onDisplayLink:)] retain]
-  };
-  display_link.get().paused = YES;
-  auto preferredFPS = display_link.get().preferredFramesPerSecond;
+  CADisplayLink* displayLink = [CADisplayLink displayLinkWithTarget:[[[self class] alloc] init]
+                                                           selector:@selector(onDisplayLink:)];
+  displayLink.paused = YES;
+  auto preferredFPS = displayLink.preferredFramesPerSecond;
 
   // From Docs:
   // The default value for preferredFramesPerSecond is 0. When this value is 0, the preferred