iOS: Improve thread safety of first frame callback (flutter/engine#55966)

Previously, we dispatched a task to a background thread to wait for the first frame to be ready. In the background task, we then throw a task on the main thread that invokes that user-provided callback to notify them that the first frame is ready.

During ARC migration, we ensured that self was strongly-captured in the task that runs on the main thread, in order to prevent the possibility that the last remaining reference to the FlutterEngine instance be the one on the background thread.

However, with the previous code, it's possible that the callback is dispatched to the main thread, executes, and completes before the block on the background thread finishes. In that case, the last remaining FlutterEngine reference may *still* be the one on the background thread.

In order to prevent this scenario, we use `dispatch_group_notify` to ensure that the tasks are executed sequentially, and that the callback task is not dispatched to the main thread until the background task has completed. Further, we capture FlutterEngine strongly in the second task to ensure that *it* is the last remaining task.

Why do we need to capture self strongly in the task fired to the main queue? Imagine the following:
* We queue up the first task on the background thread.
* We queue up the callback task on the main thread to be executed only after the background task completes.
* The task on the background thread starts running.
* That task captures weakSelf strongly and checks it -- it's not nil, yay. 
* That task now makes the blocking first frame call. Time passes. While that time passes all other strong references to self go away. That second block only captured self weakly, so the background task now has the only remaining strong reference. Uh oh!
* We hit the end of the background task block and since we have the last remaining strong reference, we dealloc FlutterEngine... on the background thread.
* KABOOM.

No changes to tests since this is a fix to a race condition that doesn't affect app semantics, isn't testable as written, and adding shims to improve testability will likely make some already subtle/difficult-to-reason-about code even more complex and difficult to reason about.

Issue: flutter#156177

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: cbracken

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

@@ -1359,34 +1359,37 @@ - (void)onLocaleUpdated:(NSNotification*)notification {
 - (void)waitForFirstFrame:(NSTimeInterval)timeout
                  callback:(void (^_Nonnull)(BOOL didTimeout))callback {
   dispatch_queue_t queue = dispatch_get_global_queue(QOS_CLASS_BACKGROUND, 0);
+  dispatch_group_t group = dispatch_group_create();
+
   __weak FlutterEngine* weakSelf = self;
-  dispatch_async(queue, ^{
+  __block BOOL didTimeout = NO;
+  dispatch_group_async(group, queue, ^{
     FlutterEngine* strongSelf = weakSelf;
     if (!strongSelf) {
       return;
     }
 
     fml::TimeDelta waitTime = fml::TimeDelta::FromMilliseconds(timeout * 1000);
-    BOOL didTimeout =
+    didTimeout =
         strongSelf.shell.WaitForFirstFrame(waitTime).code() == fml::StatusCode::kDeadlineExceeded;
-    dispatch_async(dispatch_get_main_queue(), ^{
-      // Capture strongSelf to ensure that destruction does not occur on a background thread.
-      //
-      // The containing block, executed on a background thread, strongly captures self, then makes a
-      // blocking call to self.shell.WaitForFirstFrame(). If, during this time, all other instances
-      // of self are released, the containing block's reference would be the last one, resulting in
-      // `[FlutterEngine dealloc]` being called when it goes out of scope at the end of that block,
-      // on a background thread. FlutterEngine owns a reference to a PlatformViewsController, which
-      // owns a WeakPtrFactory whose destructor asserts that it be freed on the platform thread. To
-      // avoid this, we capture strongSelf in the current block, which is executed on the platform
-      // thread.
-      //
-      // strongSelf is never nil here since it's a strong reference that's verified non-nil above,
-      // but we use a conditional check to avoid and unused expression compiler warning.
-      if (strongSelf) {
-        callback(didTimeout);
-      }
-    });
+  });
+
+  // Only execute the main queue task once the background task has completely finished executing.
+  dispatch_group_notify(group, dispatch_get_main_queue(), ^{
+    // Strongly capture self on the task dispatched to the main thread.
+    //
+    // When we capture weakSelf strongly in the above block on a background thread, we risk the
+    // possibility that all other strong references to FlutterEngine go out of scope while the block
+    // executes and that the engine is dealloc'ed at the end of the above block on a background
+    // thread. FlutterEngine is not safe to release on any thread other than the main thread.
+    //
+    // self is never nil here since it's a strong reference that's verified non-nil above, but we
+    // use a conditional check to avoid an unused expression compiler warning.
+    FlutterEngine* strongSelf = self;
+    if (!strongSelf) {
+      return;
+    }
+    callback(didTimeout);
   });
 }