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: https://github.com/flutter/flutter/issues/156177
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
ec22d90ba3