Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[deep link][ios] Update openURL method to reflect the result from framework #52643

Merged
merged 31 commits into from
Jul 2, 2024
Merged

[deep link][ios] Update openURL method to reflect the result from framework #52643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0d37280
Update FlutterAppDelegate.mm
hangyujin May 7, 2024
ebcebc3
Update FlutterAppDelegate.mm
hangyujin May 8, 2024
1c0b1ae
remove completion
hangyujin May 9, 2024
efb8d7f
lint
hangyujin May 9, 2024
4bc49e9
Update FlutterAppDelegate.mm
hangyujin May 9, 2024
84c455c
Update FlutterAppDelegate.mm
hangyujin May 9, 2024
9f5dc68
Update FlutterAppDelegate.mm
hangyujin May 9, 2024
6c5334b
Update FlutterAppDelegateTest.mm
hangyujin May 9, 2024
acf89e3
Update FlutterAppDelegateTest.mm
hangyujin May 9, 2024
6a750af
Update FlutterAppDelegateTest.mm
hangyujin May 9, 2024
bbd7271
Update FlutterAppDelegate.mm
hangyujin May 9, 2024
14a3864
Update FlutterAppDelegateTest.mm
hangyujin May 9, 2024
d54846f
Update FlutterAppDelegateTest.mm
hangyujin May 9, 2024
5cba4ff
resolve comments
hangyujin May 14, 2024
f8c981e
Update FlutterAppDelegate.mm
hangyujin May 14, 2024
73df547
Update FlutterAppDelegate.mm
hangyujin May 14, 2024
bbb545a
Update FlutterAppDelegateTest.mm
hangyujin May 14, 2024
dd3fab6
runloop
hangyujin May 14, 2024
c88757a
Update FlutterAppDelegate.mm
hangyujin May 16, 2024
30088a9
Update FlutterAppDelegate.mm
hangyujin May 16, 2024
590eedd
Update FlutterAppDelegate.mm
hangyujin May 17, 2024
2e916d8
throw url back to ios
hangyujin Jun 18, 2024
a69612b
format
hangyujin Jun 18, 2024
8983622
lint
hangyujin Jun 25, 2024
a277951
lint
hangyujin Jun 25, 2024
8207b3e
lint
hangyujin Jun 25, 2024
0a43d07
lint
hangyujin Jun 25, 2024
8136379
resolve comments
hangyujin Jun 27, 2024
5c5b591
Update FlutterAppDelegate.mm
hangyujin Jun 28, 2024
0ea42e9
Update FlutterAppDelegateTest.mm
hangyujin Jul 1, 2024
e5450fd
Update FlutterAppDelegateTest.mm
hangyujin Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 34 additions & 29 deletions shell/platform/darwin/ios/framework/Source/FlutterAppDelegate.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,44 +134,47 @@ - (void)userNotificationCenter:(UNUserNotificationCenter*)center
}
}

- (BOOL)openURL:(NSURL*)url {
- (BOOL)isFlutterDeepLinkingEnabled {
NSNumber* isDeepLinkingEnabled =
[[NSBundle mainBundle] objectForInfoDictionaryKey:@"FlutterDeepLinkingEnabled"];
if (!isDeepLinkingEnabled.boolValue) {
// Not set or NO.
return NO;
} else {
FlutterViewController* flutterViewController = [self rootFlutterViewController];
if (flutterViewController) {
[flutterViewController.engine
waitForFirstFrame:3.0
callback:^(BOOL didTimeout) {
if (didTimeout) {
FML_LOG(ERROR)
<< "Timeout waiting for the first frame when launching an URL.";
} else {
[flutterViewController.engine.navigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{
@"location" : url.absoluteString ?: [NSNull null],
}];
}
}];
return YES;
} else {
FML_LOG(ERROR) << "Attempting to open an URL without a Flutter RootViewController.";
return NO;
}
}
// if not set, return NO
return isDeepLinkingEnabled ? [isDeepLinkingEnabled boolValue] : NO;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this can be just isDeepLinkingEnabled.boolValue since sending message to nil will return NO.

Copy link
Contributor Author

@hangyujin hangyujin May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will keep this and flip NO to YES in #52350

}

// This method is called when opening an URL with custom schemes.
hangyujin marked this conversation as resolved.
Show resolved Hide resolved
- (BOOL)application:(UIApplication*)application
openURL:(NSURL*)url
options:(NSDictionary<UIApplicationOpenURLOptionsKey, id>*)options {
if ([_lifeCycleDelegate application:application openURL:url options:options]) {
return YES;
}
return [self openURL:url];

// Relaying to the system here will case an infinite loop, so we don't do it here.
return [self handleOpenURL:url options:options relayToSystemIfUnhandled:NO];
}

// Helper function for opening an URL, either with a custom scheme or a http/https scheme.
- (BOOL)handleOpenURL:(NSURL*)url
options:(NSDictionary<UIApplicationOpenURLOptionsKey, id>*)options
relayToSystemIfUnhandled:(BOOL)throwBack {
if (![self isFlutterDeepLinkingEnabled]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still ned this check if we go delegate to other selector in line 194?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just add this so the return value reflect the value of the flag.

return NO;
}

FlutterViewController* flutterViewController = [self rootFlutterViewController];
if (flutterViewController) {
[flutterViewController sendDeepLinkToFramework:url
completionHandler:^(BOOL success) {
if (!success && throwBack) {
// throw it back to iOS
[UIApplication.sharedApplication openURL:url];
}
}];
} else {
FML_LOG(ERROR) << "Attempting to open an URL without a Flutter RootViewController.";
return NO;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: If you are iffy about using BOOL flag for completeness, a semaphore version can be (pseudocode):

__block BOOL openURLSuccess = NO;
dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
[self openURL: url ... {
    openURLSuccess = success;
    dispatch_semaphore_signal(semaphore)
}];

while (dispatch_semaphore_wait(semaphore, DISPATCH_TIME_NOW)) {
    [NSRunLoop.currentRunLoop runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
}

Though a BOOL flag would be equally as good, since callback happens on main thread. I don't have a preference here.

Copy link
Contributor

@hellohuanlin hellohuanlin May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I just saw @jmagman's comment on semaphore.

To use semaphore on the same thread, you can't block it due to deadlock, as jenn previously commented. Instead, you pass in DISPATCH_TIME_NOW in dispatch_semaphore_wait, and then kick off the runloop so it can pick up the completion handler.

return YES;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this method get the result from completion handler only?

Also is implementing this selector optional if we already implement the one with completionhandler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this method get the result from completion handler only?>>

this application:openURL:options: method is non-async, i don't think its return value can be set from the completion handler so i used the deep link flag value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But should this method return before the completion handler is complete? Before, this method didn't return until pushRouteInformation was actually sent, whereas now it would return instantly.
It seems like you'd want to spin the runloop until the completion handler was done, and then return the value from the completion handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the best practice to "spin the runloop"?

I updated the code to wait for the completion handler but got failures :

Failures for clang-tidy on /Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterAppDelegate.mm:
/Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterAppDelegate.mm:178:3: error: Waiting on a callback using a semaphore creates useless threads and is subject to priority inversion; consider using a synchronous API or changing the caller to be asynchronous [clang-analyzer-optin.performance.GCDAntipattern,-warnings-as-errors]
178 | dispatch_semaphore_wait(semaphore, 5 * NSEC_PER_SEC);

}

- (BOOL)application:(UIApplication*)application handleOpenURL:(NSURL*)url {
Expand Down Expand Up @@ -204,6 +207,7 @@ - (void)application:(UIApplication*)application
completionHandler:completionHandler];
}

// This method is called when opening an URL with a http/https scheme.
- (BOOL)application:(UIApplication*)application
continueUserActivity:(NSUserActivity*)userActivity
restorationHandler:
Expand All @@ -214,7 +218,8 @@ - (BOOL)application:(UIApplication*)application
restorationHandler:restorationHandler]) {
return YES;
}
return [self openURL:userActivity.webpageURL];

return [self handleOpenURL:userActivity.webpageURL options:@{} relayToSystemIfUnhandled:YES];
}

#pragma mark - FlutterPluginRegistry methods. All delegating to the rootViewController
Expand Down
37 changes: 22 additions & 15 deletions shell/platform/darwin/ios/framework/Source/FlutterAppDelegateTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,18 @@ - (void)testLaunchUrl {
OCMStub([self.mockMainBundle objectForInfoDictionaryKey:@"FlutterDeepLinkingEnabled"])
.andReturn(@YES);

OCMStub([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route?query=test"}])
.andReturn(@YES);

BOOL result =
[self.appDelegate application:[UIApplication sharedApplication]
openURL:[NSURL URLWithString:@"http://myApp/custom/route?query=test"]
options:@{}];

XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route?query=test"}]);
OCMVerifyAll(self.mockNavigationChannel);
}

- (void)testLaunchUrlWithDeepLinkingNotSet {
Expand Down Expand Up @@ -99,29 +103,31 @@ - (void)testLaunchUrlWithDeepLinkingDisabled {
- (void)testLaunchUrlWithQueryParameterAndFragment {
OCMStub([self.mockMainBundle objectForInfoDictionaryKey:@"FlutterDeepLinkingEnabled"])
.andReturn(@YES);

OCMStub([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route?query=test#fragment"}])
.andReturn(@YES);
BOOL result = [self.appDelegate
application:[UIApplication sharedApplication]
openURL:[NSURL URLWithString:@"http://myApp/custom/route?query=test#fragment"]
options:@{}];
XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route?query=test#fragment"}]);
OCMVerifyAll(self.mockNavigationChannel);
}

- (void)testLaunchUrlWithFragmentNoQueryParameter {
OCMStub([self.mockMainBundle objectForInfoDictionaryKey:@"FlutterDeepLinkingEnabled"])
.andReturn(@YES);

OCMStub([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route#fragment"}])
.andReturn(@YES);
BOOL result =
[self.appDelegate application:[UIApplication sharedApplication]
openURL:[NSURL URLWithString:@"http://myApp/custom/route#fragment"]
options:@{}];
XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route#fragment"}]);
OCMVerifyAll(self.mockNavigationChannel);
}

- (void)testReleasesWindowOnDealloc {
Expand All @@ -145,7 +151,10 @@ - (void)testReleasesWindowOnDealloc {
- (void)testUniversalLinkPushRouteInformation {
OCMStub([self.mockMainBundle objectForInfoDictionaryKey:@"FlutterDeepLinkingEnabled"])
.andReturn(@YES);

OCMStub([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route?query=test"}])
.andReturn(@YES);
NSUserActivity* userActivity = [[NSUserActivity alloc] initWithActivityType:@"com.example.test"];
userActivity.webpageURL = [NSURL URLWithString:@"http://myApp/custom/route?query=test"];
BOOL result = [self.appDelegate
Expand All @@ -154,9 +163,7 @@ - (void)testUniversalLinkPushRouteInformation {
restorationHandler:^(NSArray<id<UIUserActivityRestoring>>* __nullable restorableObjects){
}];
XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"http://myApp/custom/route?query=test"}]);
OCMVerifyAll(self.mockNavigationChannel);
}

@end
27 changes: 27 additions & 0 deletions shell/platform/darwin/ios/framework/Source/FlutterViewController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1863,6 +1863,33 @@ - (void)handlePressEvent:(FlutterUIPressProxy*)press
[self.keyboardManager handlePress:press nextAction:next];
}

- (void)sendDeepLinkToFramework:(NSURL*)url completionHandler:(void (^)(BOOL success))completion {
[_engine.get()
waitForFirstFrame:3.0
callback:^(BOOL didTimeout) {
if (didTimeout) {
FML_LOG(ERROR) << "Timeout waiting for the first frame when launching an URL.";
completion(NO);
} else {
// invove the method and get the result
[[_engine.get() navigationChannel]
invokeMethod:@"pushRouteInformation"
arguments:@{
@"location" : url.absoluteString ?: [NSNull null],
}
result:^(id _Nullable result) {
BOOL success =
[result isKindOfClass:[NSNumber class]] && [result boolValue];
if (!success) {
// Logging the error if the result is not successful
FML_LOG(ERROR) << "Failed to handle route information in Flutter.";
}
completion(success);
}];
}
}];
}

// The documentation for presses* handlers (implemented below) is entirely
// unclear about how to handle the case where some, but not all, of the presses
// are handled here. I've elected to call super separately for each of the
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ typedef void (^FlutterKeyboardAnimationCallback)(fml::TimePoint);
// handled.
- (void)handlePressEvent:(FlutterUIPressProxy*)press
nextAction:(void (^)())nextAction API_AVAILABLE(ios(13.4));
- (void)sendDeepLinkToFramework:(NSURL*)url completionHandler:(void (^)(BOOL success))completion;
- (void)addInternalPlugins;
- (void)deregisterNotifications;
- (int32_t)accessibilityFlags;

- (BOOL)supportsShowingSystemContextMenu;

@end

#endif // FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FRAMEWORK_SOURCE_FLUTTERVIEWCONTROLLER_INTERNAL_H_