[video_player] Fix iOS crash with multiple players (flutter#4202)

This PR fixes crash in `video_player` package on iOS. 

flutter#124937

### Detailed description

I can observe the crash when displaying a couple of the different players (different URLs) inside a `ListView`. The crash happens inside of `AVFoundation` framework:

```objc
[AVPlayer _createAndConfigureFigPlayerWithType:completionHandler:]
```

In order to debug the issue, I ran the application using the plugin with `Zombie Objects` inspection turned on. The `Zombie Objects` reports the following issue:

```
*** -[FLTVideoPlayer retainWeakReference]: message sent to deallocated instance 0x6030009b2e10
```

This, in conjunction with the `NSKeyValueWillChange` line present in the stack trace led me to believe, that culprit is sending a KVO notification to the `FLTVideoPlayer` instance that's deallocated.

Next, I examined the plugin code and identified one property that doesn't have the KVO removed. In `addObserversForItem:player:` method in `FLTVideoPlayerPlugin.m`:

```
  [player addObserver:self
           forKeyPath:@"rate"
              options:NSKeyValueObservingOptionInitial | NSKeyValueObservingOptionNew
              context:rateContext];
```

The observer for `@"rate"` is never cleaned up. To be entirely sure that the issue comes from KVO that's not cleaned up, I've added the following class:

```
@implementation EmptyObserver

- (void)observeValueForKeyPath:(NSString *)path
                      ofObject:(id)object
                        change:(NSDictionary *)change
                       context:(void *)context {}

@EnD
```

and registered the observation as follows (notice that `EmptyObserver` is never retained and deallocated instantly):

```
  [player addObserver:[[EmptyObserver alloc] init]
           forKeyPath:@"rate"
              options:NSKeyValueObservingOptionInitial | NSKeyValueObservingOptionNew
              context:rateContext];
```

The exception I got seems to be matching the underlying issue:

```
*** -[EmptyObserver retainWeakReference]: message sent to deallocated instance 0x6020001ceb70
```

This means the fix for the issue is to add the following to `disposeSansEventChannel` method:

```
[self.player removeObserver:self forKeyPath:@"rate"];
```

After applying the patch, I can no longer crash the player.

Author: turekj

packages/video_player/video_player_avfoundation/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 2.4.9
+
+* Fixes the iOS crash when using multiple players on the same screen.
+  See: https://github.com/flutter/flutter/issues/124937
+
 ## 2.4.8
 
 * Fixes missing `isPlaybackLikelyToKeepUp` check for iOS video player `bufferingEnd` event and `bufferingStart` event.

packages/video_player/video_player_avfoundation/example/ios/RunnerTests/VideoPlayerTests.m

@@ -14,6 +14,8 @@ @interface FLTVideoPlayer : NSObject <FlutterStreamHandler>
 @property(readonly, nonatomic) AVPlayer *player;
 @property(readonly, nonatomic) AVPlayerLayer *playerLayer;
 @property(readonly, nonatomic) int64_t position;
+
+- (void)onTextureUnregistered:(NSObject<FlutterTexture> *)texture;
 @end
 
 @interface FLTVideoPlayerPlugin (Test) <FLTAVFoundationVideoPlayerApi>
@@ -442,6 +444,110 @@ - (void)testSeekToleranceWhenSeekingToEnd {
   return initializationEvent;
 }
 
+// Checks whether [AVPlayer rate] KVO observations are correctly detached.
+// - https://github.com/flutter/flutter/issues/124937
+//
+// Failing to de-register results in a crash in [AVPlayer willChangeValueForKey:].
+- (void)testDoesNotCrashOnRateObservationAfterDisposal {
+  NSObject<FlutterPluginRegistry> *registry =
+      (NSObject<FlutterPluginRegistry> *)[[UIApplication sharedApplication] delegate];
+  NSObject<FlutterPluginRegistrar> *registrar =
+      [registry registrarForPlugin:@"testDoesNotCrashOnRateObservationAfterDisposal"];
+
+  AVPlayer *avPlayer = nil;
+  __weak FLTVideoPlayer *player = nil;
+
+  // Autoreleasepool is needed to simulate conditions of FLTVideoPlayer deallocation.
+  @autoreleasepool {
+    FLTVideoPlayerPlugin *videoPlayerPlugin =
+        (FLTVideoPlayerPlugin *)[[FLTVideoPlayerPlugin alloc] initWithRegistrar:registrar];
+
+    FlutterError *error;
+    [videoPlayerPlugin initialize:&error];
+    XCTAssertNil(error);
+
+    FLTCreateMessage *create = [FLTCreateMessage
+        makeWithAsset:nil
+                  uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
+          packageName:nil
+           formatHint:nil
+          httpHeaders:@{}];
+    FLTTextureMessage *textureMessage = [videoPlayerPlugin create:create error:&error];
+    XCTAssertNil(error);
+    XCTAssertNotNil(textureMessage);
+
+    player = videoPlayerPlugin.playersByTextureId[textureMessage.textureId];
+    XCTAssertNotNil(player);
+    avPlayer = player.player;
+
+    [videoPlayerPlugin dispose:textureMessage error:&error];
+    XCTAssertNil(error);
+  }
+
+  // [FLTVideoPlayerPlugin dispose:error:] selector is dispatching the [FLTVideoPlayer dispose] call
+  // with a 1-second delay keeping a strong reference to the player. The polling ensures the player
+  // was truly deallocated.
+  [self expectationForPredicate:[NSPredicate predicateWithFormat:@"self != nil"]
+            evaluatedWithObject:player
+                        handler:nil];
+  [self waitForExpectationsWithTimeout:10.0 handler:nil];
+
+  [avPlayer willChangeValueForKey:@"rate"];  // No assertions needed. Lack of crash is a success.
+}
+
+// During the hot reload:
+//  1. `[FLTVideoPlayer onTextureUnregistered:]` gets called.
+//  2. `[FLTVideoPlayerPlugin initialize:]` gets called.
+//
+// Both of these methods dispatch [FLTVideoPlayer dispose] on the main thread
+// leading to a possible crash when de-registering observers twice.
+- (void)testHotReloadDoesNotCrash {
+  NSObject<FlutterPluginRegistry> *registry =
+      (NSObject<FlutterPluginRegistry> *)[[UIApplication sharedApplication] delegate];
+  NSObject<FlutterPluginRegistrar> *registrar =
+      [registry registrarForPlugin:@"testHotReloadDoesNotCrash"];
+
+  __weak FLTVideoPlayer *player = nil;
+
+  // Autoreleasepool is needed to simulate conditions of FLTVideoPlayer deallocation.
+  @autoreleasepool {
+    FLTVideoPlayerPlugin *videoPlayerPlugin =
+        (FLTVideoPlayerPlugin *)[[FLTVideoPlayerPlugin alloc] initWithRegistrar:registrar];
+
+    FlutterError *error;
+    [videoPlayerPlugin initialize:&error];
+    XCTAssertNil(error);
+
+    FLTCreateMessage *create = [FLTCreateMessage
+        makeWithAsset:nil
+                  uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
+          packageName:nil
+           formatHint:nil
+          httpHeaders:@{}];
+    FLTTextureMessage *textureMessage = [videoPlayerPlugin create:create error:&error];
+    XCTAssertNil(error);
+    XCTAssertNotNil(textureMessage);
+
+    player = videoPlayerPlugin.playersByTextureId[textureMessage.textureId];
+    XCTAssertNotNil(player);
+
+    [player onTextureUnregistered:nil];
+    XCTAssertNil(error);
+
+    [videoPlayerPlugin initialize:&error];
+    XCTAssertNil(error);
+  }
+
+  // [FLTVideoPlayerPlugin dispose:error:] selector is dispatching the [FLTVideoPlayer dispose] call
+  // with a 1-second delay keeping a strong reference to the player. The polling ensures the player
+  // was truly deallocated.
+  [self expectationForPredicate:[NSPredicate predicateWithFormat:@"self != nil"]
+            evaluatedWithObject:player
+                        handler:nil];
+  [self waitForExpectationsWithTimeout:10.0
+                               handler:nil];  // No assertions needed. Lack of crash is a success.
+}
+
 - (void)validateTransformFixForOrientation:(UIImageOrientation)orientation {
   AVAssetTrack *track = [[FakeAVAssetTrack alloc] initWithOrientation:orientation];
   CGAffineTransform t = FLTGetStandardizedTransformForTrack(track);

packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m

@@ -505,6 +505,14 @@ - (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments
 /// is useful for the case where the Engine is in the process of deconstruction
 /// so the channel is going to die or is already dead.
 - (void)disposeSansEventChannel {
+  // This check prevents the crash caused by removing the KVO observers twice.
+  // When performing a Hot Restart, the leftover players are disposed once directly
+  // by [FLTVideoPlayerPlugin initialize:] method and then disposed again by
+  // [FLTVideoPlayer onTextureUnregistered:] call leading to possible over-release.
+  if (_disposed) {
+    return;
+  }
+
   _disposed = YES;
   [_playerLayer removeFromSuperlayer];
   [_displayLink invalidate];
@@ -514,6 +522,7 @@ - (void)disposeSansEventChannel {
   [currentItem removeObserver:self forKeyPath:@"presentationSize"];
   [currentItem removeObserver:self forKeyPath:@"duration"];
   [currentItem removeObserver:self forKeyPath:@"playbackLikelyToKeepUp"];
+  [self.player removeObserver:self forKeyPath:@"rate"];
 
   [self.player replaceCurrentItemWithPlayerItem:nil];
   [[NSNotificationCenter defaultCenter] removeObserver:self];

packages/video_player/video_player_avfoundation/pubspec.yaml

@@ -2,7 +2,7 @@ name: video_player_avfoundation
 description: iOS implementation of the video_player plugin.
 repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_avfoundation
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
-version: 2.4.8
+version: 2.4.9
 
 environment:
   sdk: ">=2.18.0 <4.0.0"