[webview_flutter_wkwebview] Implement one callback method for review of the design (flutter#5700)

Author: bparrishMines

packages/webview_flutter/webview_flutter_wkwebview/example/ios/RunnerTests/FWFInstanceManagerTests.m

@@ -3,38 +3,40 @@
 // found in the LICENSE file.
 
 #import <XCTest/XCTest.h>
+
 @import webview_flutter_wkwebview;
+@import webview_flutter_wkwebview.Test;
 
 @interface FWFInstanceManagerTests : XCTestCase
 @end
 
 @implementation FWFInstanceManagerTests
-- (void)testAddInstanceCreatedFromDart {
+- (void)testAddDartCreatedInstance {
   FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init];
   NSObject *object = [[NSObject alloc] init];
 
-  [instanceManager addDartCreatedInstance:object withIdentifier:5];
-  XCTAssertEqualObjects([instanceManager instanceForIdentifier:5], object);
-  XCTAssertEqual([instanceManager identifierForInstance:object], 5);
+  [instanceManager addDartCreatedInstance:object withIdentifier:0];
+  XCTAssertEqualObjects([instanceManager instanceForIdentifier:0], object);
+  XCTAssertEqual([instanceManager identifierWithStrongReferenceForInstance:object], 0);
 }
 
-- (void)testRemoveInstance {
+- (void)testAddHostCreatedInstance {
   FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init];
   NSObject *object = [[NSObject alloc] init];
-  [instanceManager addDartCreatedInstance:object withIdentifier:5];
+  [instanceManager addHostCreatedInstance:object];
 
-  [instanceManager removeInstance:object];
-  XCTAssertNil([instanceManager instanceForIdentifier:5]);
-  XCTAssertEqual([instanceManager identifierForInstance:object], NSNotFound);
+  long identifier = [instanceManager identifierWithStrongReferenceForInstance:object];
+  XCTAssertNotEqual(identifier, NSNotFound);
+  XCTAssertEqualObjects([instanceManager instanceForIdentifier:identifier], object);
 }
 
 - (void)testRemoveInstanceWithIdentifier {
   FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init];
   NSObject *object = [[NSObject alloc] init];
-  [instanceManager addDartCreatedInstance:object withIdentifier:5];
 
-  [instanceManager removeInstanceWithIdentifier:5];
-  XCTAssertNil([instanceManager instanceForIdentifier:5]);
-  XCTAssertEqual([instanceManager identifierForInstance:object], NSNotFound);
+  [instanceManager addDartCreatedInstance:object withIdentifier:0];
+
+  XCTAssertEqualObjects([instanceManager removeInstanceWithIdentifier:0], object);
+  XCTAssertEqual([instanceManager strongInstanceCount], 0);
 }
 @end

packages/webview_flutter/webview_flutter_wkwebview/example/ios/RunnerTests/FWFNavigationDelegateHostApiTests.m

@@ -14,8 +14,9 @@ @interface FWFNavigationDelegateHostApiTests : XCTestCase
 @implementation FWFNavigationDelegateHostApiTests
 - (void)testCreateWithIdentifier {
   FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init];
-  FWFNavigationDelegateHostApiImpl *hostAPI =
-      [[FWFNavigationDelegateHostApiImpl alloc] initWithInstanceManager:instanceManager];
+  FWFNavigationDelegateHostApiImpl *hostAPI = [[FWFNavigationDelegateHostApiImpl alloc]
+      initWithBinaryMessenger:OCMProtocolMock(@protocol(FlutterBinaryMessenger))
+              instanceManager:instanceManager];
 
   FlutterError *error;
   [hostAPI createWithIdentifier:@0 error:&error];
@@ -25,4 +26,34 @@ - (void)testCreateWithIdentifier {
   XCTAssertTrue([navigationDelegate conformsToProtocol:@protocol(WKNavigationDelegate)]);
   XCTAssertNil(error);
 }
+
+- (void)testDidFinishNavigation {
+  FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init];
+  FWFNavigationDelegateHostApiImpl *hostAPI = [[FWFNavigationDelegateHostApiImpl alloc]
+      initWithBinaryMessenger:OCMProtocolMock(@protocol(FlutterBinaryMessenger))
+              instanceManager:instanceManager];
+
+  FlutterError *error;
+  [hostAPI createWithIdentifier:@0 error:&error];
+  FWFNavigationDelegate *navigationDelegate =
+      (FWFNavigationDelegate *)[instanceManager instanceForIdentifier:0];
+  id mockDelegate = OCMPartialMock(navigationDelegate);
+
+  FWFNavigationDelegateFlutterApiImpl *flutterAPI = [[FWFNavigationDelegateFlutterApiImpl alloc]
+      initWithBinaryMessenger:OCMProtocolMock(@protocol(FlutterBinaryMessenger))
+              instanceManager:instanceManager];
+  id mockFlutterApi = OCMPartialMock(flutterAPI);
+
+  OCMStub([mockDelegate navigationDelegateAPI]).andReturn(mockFlutterApi);
+
+  WKWebView *mockWebView = OCMClassMock([WKWebView class]);
+  OCMStub([mockWebView URL]).andReturn([NSURL URLWithString:@"https://flutter.dev/"]);
+  [instanceManager addDartCreatedInstance:mockWebView withIdentifier:1];
+
+  [mockDelegate webView:mockWebView didFinishNavigation:OCMClassMock([WKNavigation class])];
+  OCMVerify([mockFlutterApi didFinishNavigationForDelegateWithIdentifier:@0
+                                                       webViewIdentifier:@1
+                                                                     URL:@"https://flutter.dev/"
+                                                              completion:OCMOCK_ANY]);
+}
 @end

packages/webview_flutter/webview_flutter_wkwebview/example/ios/RunnerTests/FWFObjectHostApiTests.m

@@ -76,7 +76,10 @@ - (void)testDispose {
 
   FlutterError *error;
   [hostAPI disposeObjectWithIdentifier:@0 error:&error];
-  XCTAssertEqual([instanceManager identifierForInstance:object], NSNotFound);
+  // Only the strong reference is removed, so the weak reference will remain until object is set to
+  // nil.
+  object = nil;
+  XCTAssertFalse([instanceManager containsInstance:object]);
   XCTAssertNil(error);
 }
 @end

packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFGeneratedWebKitApis.h

@@ -303,9 +303,6 @@ NSObject<FlutterMessageCodec> *FWFWKNavigationDelegateHostApiGetCodec(void);
 
 @protocol FWFWKNavigationDelegateHostApi
 - (void)createWithIdentifier:(NSNumber *)identifier error:(FlutterError *_Nullable *_Nonnull)error;
-- (void)setDidFinishNavigationForDelegateWithIdentifier:(NSNumber *)identifier
-                                     functionIdentifier:(nullable NSNumber *)functionIdentifier
-                                                  error:(FlutterError *_Nullable *_Nonnull)error;
 @end
 
 extern void FWFWKNavigationDelegateHostApiSetup(
@@ -317,7 +314,7 @@ NSObject<FlutterMessageCodec> *FWFWKNavigationDelegateFlutterApiGetCodec(void);
 
 @interface FWFWKNavigationDelegateFlutterApi : NSObject
 - (instancetype)initWithBinaryMessenger:(id<FlutterBinaryMessenger>)binaryMessenger;
-- (void)didFinishNavigationForDelegateWithIdentifier:(NSNumber *)functionIdentifier
+- (void)didFinishNavigationForDelegateWithIdentifier:(NSNumber *)identifier
                                    webViewIdentifier:(NSNumber *)webViewIdentifier
                                                  URL:(nullable NSString *)url
                                           completion:(void (^)(NSError *_Nullable))completion;
@@ -343,13 +340,11 @@ NSObject<FlutterMessageCodec> *FWFNSObjectHostApiGetCodec(void);
 extern void FWFNSObjectHostApiSetup(id<FlutterBinaryMessenger> binaryMessenger,
                                     NSObject<FWFNSObjectHostApi> *_Nullable api);
 
-/// The codec used by FWFFunctionFlutterApi.
-NSObject<FlutterMessageCodec> *FWFFunctionFlutterApiGetCodec(void);
+/// The codec used by FWFNSObjectFlutterApi.
+NSObject<FlutterMessageCodec> *FWFNSObjectFlutterApiGetCodec(void);
 
-@interface FWFFunctionFlutterApi : NSObject
+@interface FWFNSObjectFlutterApi : NSObject
 - (instancetype)initWithBinaryMessenger:(id<FlutterBinaryMessenger>)binaryMessenger;
-- (void)disposeFunctionWithIdentifier:(NSNumber *)identifier
-                           completion:(void (^)(NSError *_Nullable))completion;
 @end
 /// The codec used by FWFWKWebViewHostApi.
 NSObject<FlutterMessageCodec> *FWFWKWebViewHostApiGetCodec(void);

packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFGeneratedWebKitApis.m

@@ -1150,32 +1150,6 @@ void FWFWKNavigationDelegateHostApiSetup(id<FlutterBinaryMessenger> binaryMessen
       [channel setMessageHandler:nil];
     }
   }
-  {
-    FlutterBasicMessageChannel *channel = [[FlutterBasicMessageChannel alloc]
-           initWithName:@"dev.flutter.pigeon.WKNavigationDelegateHostApi.setDidFinishNavigation"
-        binaryMessenger:binaryMessenger
-                  codec:FWFWKNavigationDelegateHostApiGetCodec()];
-    if (api) {
-      NSCAssert(
-          [api respondsToSelector:@selector
-               (setDidFinishNavigationForDelegateWithIdentifier:functionIdentifier:error:)],
-          @"FWFWKNavigationDelegateHostApi api (%@) doesn't respond to "
-          @"@selector(setDidFinishNavigationForDelegateWithIdentifier:functionIdentifier:error:)",
-          api);
-      [channel setMessageHandler:^(id _Nullable message, FlutterReply callback) {
-        NSArray *args = message;
-        NSNumber *arg_identifier = GetNullableObjectAtIndex(args, 0);
-        NSNumber *arg_functionIdentifier = GetNullableObjectAtIndex(args, 1);
-        FlutterError *error;
-        [api setDidFinishNavigationForDelegateWithIdentifier:arg_identifier
-                                          functionIdentifier:arg_functionIdentifier
-                                                       error:&error];
-        callback(wrapResult(nil, error));
-      }];
-    } else {
-      [channel setMessageHandler:nil];
-    }
-  }
 }
 @interface FWFWKNavigationDelegateFlutterApiCodecReader : FlutterStandardReader
 @end
@@ -1222,7 +1196,7 @@ - (instancetype)initWithBinaryMessenger:(NSObject<FlutterBinaryMessenger> *)bina
   }
   return self;
 }
-- (void)didFinishNavigationForDelegateWithIdentifier:(NSNumber *)arg_functionIdentifier
+- (void)didFinishNavigationForDelegateWithIdentifier:(NSNumber *)arg_identifier
                                    webViewIdentifier:(NSNumber *)arg_webViewIdentifier
                                                  URL:(nullable NSString *)arg_url
                                           completion:(void (^)(NSError *_Nullable))completion {
@@ -1232,7 +1206,7 @@ - (void)didFinishNavigationForDelegateWithIdentifier:(NSNumber *)arg_functionIde
              binaryMessenger:self.binaryMessenger
                        codec:FWFWKNavigationDelegateFlutterApiGetCodec()];
   [channel sendMessage:@[
-    arg_functionIdentifier ?: [NSNull null], arg_webViewIdentifier ?: [NSNull null],
+    arg_identifier ?: [NSNull null], arg_webViewIdentifier ?: [NSNull null],
     arg_url ?: [NSNull null]
   ]
                  reply:^(id reply) {
@@ -1373,43 +1347,43 @@ void FWFNSObjectHostApiSetup(id<FlutterBinaryMessenger> binaryMessenger,
     }
   }
 }
-@interface FWFFunctionFlutterApiCodecReader : FlutterStandardReader
+@interface FWFNSObjectFlutterApiCodecReader : FlutterStandardReader
 @end
-@implementation FWFFunctionFlutterApiCodecReader
+@implementation FWFNSObjectFlutterApiCodecReader
 @end
 
-@interface FWFFunctionFlutterApiCodecWriter : FlutterStandardWriter
+@interface FWFNSObjectFlutterApiCodecWriter : FlutterStandardWriter
 @end
-@implementation FWFFunctionFlutterApiCodecWriter
+@implementation FWFNSObjectFlutterApiCodecWriter
 @end
 
-@interface FWFFunctionFlutterApiCodecReaderWriter : FlutterStandardReaderWriter
+@interface FWFNSObjectFlutterApiCodecReaderWriter : FlutterStandardReaderWriter
 @end
-@implementation FWFFunctionFlutterApiCodecReaderWriter
+@implementation FWFNSObjectFlutterApiCodecReaderWriter
 - (FlutterStandardWriter *)writerWithData:(NSMutableData *)data {
-  return [[FWFFunctionFlutterApiCodecWriter alloc] initWithData:data];
+  return [[FWFNSObjectFlutterApiCodecWriter alloc] initWithData:data];
 }
 - (FlutterStandardReader *)readerWithData:(NSData *)data {
-  return [[FWFFunctionFlutterApiCodecReader alloc] initWithData:data];
+  return [[FWFNSObjectFlutterApiCodecReader alloc] initWithData:data];
 }
 @end
 
-NSObject<FlutterMessageCodec> *FWFFunctionFlutterApiGetCodec() {
+NSObject<FlutterMessageCodec> *FWFNSObjectFlutterApiGetCodec() {
   static dispatch_once_t sPred = 0;
   static FlutterStandardMessageCodec *sSharedObject = nil;
   dispatch_once(&sPred, ^{
-    FWFFunctionFlutterApiCodecReaderWriter *readerWriter =
-        [[FWFFunctionFlutterApiCodecReaderWriter alloc] init];
+    FWFNSObjectFlutterApiCodecReaderWriter *readerWriter =
+        [[FWFNSObjectFlutterApiCodecReaderWriter alloc] init];
     sSharedObject = [FlutterStandardMessageCodec codecWithReaderWriter:readerWriter];
   });
   return sSharedObject;
 }
 
-@interface FWFFunctionFlutterApi ()
+@interface FWFNSObjectFlutterApi ()
 @property(nonatomic, strong) NSObject<FlutterBinaryMessenger> *binaryMessenger;
 @end
 
-@implementation FWFFunctionFlutterApi
+@implementation FWFNSObjectFlutterApi
 
 - (instancetype)initWithBinaryMessenger:(NSObject<FlutterBinaryMessenger> *)binaryMessenger {
   self = [super init];
@@ -1418,17 +1392,6 @@ - (instancetype)initWithBinaryMessenger:(NSObject<FlutterBinaryMessenger> *)bina
   }
   return self;
 }
-- (void)disposeFunctionWithIdentifier:(NSNumber *)arg_identifier
-                           completion:(void (^)(NSError *_Nullable))completion {
-  FlutterBasicMessageChannel *channel = [FlutterBasicMessageChannel
-      messageChannelWithName:@"dev.flutter.pigeon.FunctionFlutterApi.dispose"
-             binaryMessenger:self.binaryMessenger
-                       codec:FWFFunctionFlutterApiGetCodec()];
-  [channel sendMessage:@[ arg_identifier ?: [NSNull null] ]
-                 reply:^(id reply) {
-                   completion(nil);
-                 }];
-}
 @end
 @interface FWFWKWebViewHostApiCodecReader : FlutterStandardReader
 @end

packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFInstanceManager.h

@@ -6,17 +6,30 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+typedef void (^FWFOnDeallocCallback)(long identifier);
+
 /**
- * Maintains instances to intercommunicate with Dart objects.
+ * Maintains instances used to communicate with the corresponding objects in Dart.
  *
  * When an instance is added with an identifier, either can be used to retrieve the other.
+ *
+ * Added instances are added as a weak reference and a strong reference. When the strong reference
+ * is removed with `removeStrongReferenceWithIdentifier:` and the weak reference is deallocated,
+ * the `deallocCallback` is made with the instance's identifier. However, if the strong reference is
+ * removed and then the identifier is retrieved with the intention to pass the identifier to Dart
+ * (e.g. calling  `identifierForInstance:identifierWillBePassedToFlutter:` with
+ * `identifierWillBePassedToFlutter` set to YES), the strong reference to the instance is recreated.
+ * The strong reference will then need to be removed manually again.
+ *
+ * Accessing and inserting to an InstanceManager is thread safe.
  */
 @interface FWFInstanceManager : NSObject
+@property(readonly) FWFOnDeallocCallback deallocCallback;
+- (instancetype)initWithDeallocCallback:(FWFOnDeallocCallback)callback;
 // TODO(bparrishMines): Pairs should not be able to be overwritten and this feature
-// should be replaced with a call to clear the manager in the event of a hot restart
-// instead.
+// should be replaced with a call to clear the manager in the event of a hot restart.
 /**
- * Adds a new instance to the manager that was instantiated by Dart.
+ * Adds a new instance that was instantiated from Dart.
  *
  * If an instance or identifier has already been added, it will be replaced by the new values. The
  * Dart InstanceManager is considered the source of truth and has the capability to overwrite stored
@@ -28,43 +41,57 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)addDartCreatedInstance:(NSObject *)instance withIdentifier:(long)instanceIdentifier;
 
 /**
- * Removes the instance paired with a given identifier from the manager.
- *
- * @param instanceIdentifier The identifier paired to an instance.
+ * Adds a new instance that was instantiated from the host platform.
  *
- * @return The removed instance if the manager contains the given instanceIdentifier, otherwise
- * nil.
+ * @param instance The instance to be stored.
+ * @return The unique identifier stored with instance.
  */
-- (nullable NSObject *)removeInstanceWithIdentifier:(long)instanceIdentifier;
+- (long)addHostCreatedInstance:(nonnull NSObject *)instance;
 
 /**
- * Removes the instance from the manager.
+ * Removes `instanceIdentifier` and its associated strongly referenced instance, if present, from
+ * the manager.
  *
- * @param instance The instance to be removed from the manager.
+ * @param instanceIdentifier The identifier paired to an instance.
  *
- * @return The identifier of the removed instance if the manager contains the given instance,
- * otherwise NSNotFound.
+ * @return The removed instance if the manager contains the given instanceIdentifier, otherwise
+ * nil.
  */
-- (long)removeInstance:(NSObject *)instance;
+- (nullable NSObject *)removeInstanceWithIdentifier:(long)instanceIdentifier;
 
 /**
- * Retrieves the instance paired with identifier.
+ * Retrieves the instance associated with identifier.
  *
  * @param instanceIdentifier  The identifier paired to an instance.
  *
- * @return The paired instance if the manager contains the given instanceIdentifier,
+ * @return The  instance associated with `instanceIdentifier` if the manager contains the value,
  * otherwise nil.
  */
 - (nullable NSObject *)instanceForIdentifier:(long)instanceIdentifier;
 
 /**
  * Retrieves the identifier paired with an instance.
  *
+ * If the manager contains `instance`, as a strong or weak reference, the strong reference to
+ * `instance` will be recreated and will need to be removed again with
+ * `removeInstanceWithIdentifier:`.
+ *
+ * This method also expects the Dart `InstanceManager` to have, or recreate, a weak reference to the
+ * instance the identifier is associated with once it receives it.
+ *
  * @param instance An instance that may be stored in the manager.
  *
- * @return The paired identifer if the manager contains the given instance, otherwise NSNotFound.
+ * @return The identifier associated with `instance` if the manager contains the value, otherwise
+ * NSNotFound.
+ */
+- (long)identifierWithStrongReferenceForInstance:(nonnull NSObject *)instance;
+
+/**
+ * Returns whether this manager contains the given `instance`.
+ *
+ * @return Whether this manager contains the given `instance`.
  */
-- (long)identifierForInstance:(NSObject *)instance;
+- (BOOL)containsInstance:(nonnull NSObject *)instance;
 @end
 
 NS_ASSUME_NONNULL_END