Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Migrate iOS and Android to use pushRouteInformation #39372

Merged
merged 3 commits into from
Feb 8, 2023
Merged

Migrate iOS and Android to use pushRouteInformation #39372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
416eb72
Migrate iOS and Android to use pushRouteInformation
chunhtai Feb 3, 2023
8509693
revert
chunhtai Feb 3, 2023
5424fa3
fix test
chunhtai Feb 3, 2023
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
6 changes: 4 additions & 2 deletions shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,11 +829,13 @@ void onRequestPermissionsResult(
void onNewIntent(@NonNull Intent intent) {
ensureAlive();
if (flutterEngine != null) {
Log.v(TAG, "Forwarding onNewIntent() to FlutterEngine and sending pushRoute message.");
Log.v(
TAG,
"Forwarding onNewIntent() to FlutterEngine and sending pushRouteInformation message.");
flutterEngine.getActivityControlSurface().onNewIntent(intent);
String initialRoute = maybeGetInitialRouteFromIntent(intent);
if (initialRoute != null && !initialRoute.isEmpty()) {
flutterEngine.getNavigationChannel().pushRoute(initialRoute);
flutterEngine.getNavigationChannel().pushRouteInformation(initialRoute);
}
} else {
Log.w(TAG, "onNewIntent() invoked before FlutterFragment was attached to an Activity.");
Expand Down
9 changes: 9 additions & 0 deletions shell/platform/android/io/flutter/embedding/engine/systemchannels/NavigationChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import io.flutter.plugin.common.JSONMethodCodec;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import java.util.HashMap;
import java.util.Map;

/** TODO(mattcarroll): fill in javadoc for NavigationChannel. */
public class NavigationChannel {
Expand Down Expand Up @@ -43,6 +45,13 @@ public void pushRoute(@NonNull String route) {
channel.invokeMethod("pushRoute", route);
}

public void pushRouteInformation(@NonNull String route) {
Log.v(TAG, "Sending message to push route information '" + route + "'");
Map<String, String> message = new HashMap<>();
message.put("location", route);
channel.invokeMethod("pushRouteInformation", message);
}

public void popRoute() {
Copy link
Contributor Author

@chunhtai chunhtai Feb 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I am not sure if we can remove this or not. is this a public API? @reidbaker
This is still used by FlutterView, I am not sure if we can change the API or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to stay for now. I am not following why you want it removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh sorry, i highlighted the wrong one, I meant the pushRoute method in line 43

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you replace the usages in flutterView with your new method? If so then maybe the right pattern is to modify the existing one to have the new behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes If i am free to change this API, I can do that. I assume there is no public access to this class from third party plugin then?

Copy link
Contributor

Choose a reason for hiding this comment

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

I dont know how I would know that. I think it would be unusual for us to expose the internals of a plugin but I guess it is possible. @bparrishMines any advice you can share for how to know if this is used by a third party. I checked google3 and did not find any direct references.

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 will go ahead and merge this pr for now, we can spawn another pr to remove the push route api if we are sure we can remove it

Copy link
Contributor

@bparrishMines bparrishMines Feb 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know if this is used by a plugin, but it looks like something that could be used by Add to App.

Log.v(TAG, "Sending message to pop route.");
channel.invokeMethod("popRoute", null);
Expand Down
23 changes: 13 additions & 10 deletions ...orm/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ public void itSendsdefaultInitialRouteOnStartIfNotDeepLinkingFromIntent() {
}

@Test
public void itSendsPushRouteMessageWhenOnNewIntent() {
public void itSendsPushRouteInformationMessageWhenOnNewIntent() {
when(mockHost.shouldHandleDeeplinking()).thenReturn(true);
// Create the real object that we're testing.
FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(mockHost);
Expand All @@ -742,11 +742,11 @@ public void itSendsPushRouteMessageWhenOnNewIntent() {

// Verify that the navigation channel was given the push route message.
verify(mockFlutterEngine.getNavigationChannel(), times(1))
.pushRoute("/custom/route?query=test");
.pushRouteInformation("/custom/route?query=test");
}

@Test
public void itDoesNotSendPushRouteMessageWhenOnNewIntentIsNonHierarchicalUri() {
public void itDoesNotSendPushRouteInformationMessageWhenOnNewIntentIsNonHierarchicalUri() {
when(mockHost.shouldHandleDeeplinking()).thenReturn(true);
// Create the real object that we're testing.
FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(mockHost);
Expand All @@ -764,11 +764,12 @@ public void itDoesNotSendPushRouteMessageWhenOnNewIntentIsNonHierarchicalUri() {
delegate.onNewIntent(mockIntent);

// Verify that the navigation channel was not given a push route message.
verify(mockFlutterEngine.getNavigationChannel(), times(0)).pushRoute("mailto:[email protected]");
verify(mockFlutterEngine.getNavigationChannel(), times(0))
.pushRouteInformation("mailto:[email protected]");
}

@Test
public void itSendsPushRouteMessageWhenOnNewIntentWithQueryParameterAndFragment() {
public void itSendsPushRouteInformationMessageWhenOnNewIntentWithQueryParameterAndFragment() {
when(mockHost.shouldHandleDeeplinking()).thenReturn(true);
// Create the real object that we're testing.
FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(mockHost);
Expand All @@ -785,11 +786,11 @@ public void itSendsPushRouteMessageWhenOnNewIntentWithQueryParameterAndFragment(

// Verify that the navigation channel was given the push route message.
verify(mockFlutterEngine.getNavigationChannel(), times(1))
.pushRoute("/custom/route?query=test#fragment");
.pushRouteInformation("/custom/route?query=test#fragment");
}

@Test
public void itSendsPushRouteMessageWhenOnNewIntentWithFragmentNoQueryParameter() {
public void itSendsPushRouteInformationMessageWhenOnNewIntentWithFragmentNoQueryParameter() {
when(mockHost.shouldHandleDeeplinking()).thenReturn(true);
// Create the real object that we're testing.
FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(mockHost);
Expand All @@ -804,11 +805,12 @@ public void itSendsPushRouteMessageWhenOnNewIntentWithFragmentNoQueryParameter()
delegate.onNewIntent(mockIntent);

// Verify that the navigation channel was given the push route message.
verify(mockFlutterEngine.getNavigationChannel(), times(1)).pushRoute("/custom/route#fragment");
verify(mockFlutterEngine.getNavigationChannel(), times(1))
.pushRouteInformation("/custom/route#fragment");
}

@Test
public void itSendsPushRouteMessageWhenOnNewIntentNoQueryParameter() {
public void itSendsPushRouteInformationMessageWhenOnNewIntentNoQueryParameter() {
when(mockHost.shouldHandleDeeplinking()).thenReturn(true);
// Create the real object that we're testing.
FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(mockHost);
Expand All @@ -823,7 +825,8 @@ public void itSendsPushRouteMessageWhenOnNewIntentNoQueryParameter() {
delegate.onNewIntent(mockIntent);

// Verify that the navigation channel was given the push route message.
verify(mockFlutterEngine.getNavigationChannel(), times(1)).pushRoute("/custom/route");
verify(mockFlutterEngine.getNavigationChannel(), times(1))
.pushRouteInformation("/custom/route");
}

@Test
Expand Down
7 changes: 5 additions & 2 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 @@ -157,8 +157,11 @@ - (BOOL)openURL:(NSURL*)url {
if ([url.fragment length] != 0) {
fullRoute = [NSString stringWithFormat:@"%@#%@", fullRoute, url.fragment];
}
[flutterViewController.engine.navigationChannel invokeMethod:@"pushRoute"
arguments:fullRoute];
[flutterViewController.engine.navigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{
@"location" : fullRoute,
}];
}
}];
return YES;
Expand Down
19 changes: 10 additions & 9 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 @@ -67,8 +67,8 @@ - (void)testLaunchUrl {
openURL:[NSURL URLWithString:@"http://myApp/custom/route?query=test"]
options:@{}];
XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel invokeMethod:@"pushRoute"
arguments:@"/custom/route?query=test"]);
OCMVerify([self.mockNavigationChannel invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"/custom/route?query=test"}]);
}

- (void)testLaunchUrlWithDeepLinkingNotSet {
Expand Down Expand Up @@ -104,8 +104,9 @@ - (void)testLaunchUrlWithQueryParameterAndFragment {
openURL:[NSURL URLWithString:@"http://myApp/custom/route?query=test#fragment"]
options:@{}];
XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel invokeMethod:@"pushRoute"
arguments:@"/custom/route?query=test#fragment"]);
OCMVerify([self.mockNavigationChannel
invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"/custom/route?query=test#fragment"}]);
}

- (void)testLaunchUrlWithFragmentNoQueryParameter {
Expand All @@ -117,8 +118,8 @@ - (void)testLaunchUrlWithFragmentNoQueryParameter {
openURL:[NSURL URLWithString:@"http://myApp/custom/route#fragment"]
options:@{}];
XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel invokeMethod:@"pushRoute"
arguments:@"/custom/route#fragment"]);
OCMVerify([self.mockNavigationChannel invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"/custom/route#fragment"}]);
}

- (void)testReleasesWindowOnDealloc {
Expand All @@ -139,7 +140,7 @@ - (void)testReleasesWindowOnDealloc {

#pragma mark - Deep linking

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

Expand All @@ -151,8 +152,8 @@ - (void)testUniversalLinkPushRoute {
restorationHandler:^(NSArray<id<UIUserActivityRestoring>>* __nullable restorableObjects){
}];
XCTAssertTrue(result);
OCMVerify([self.mockNavigationChannel invokeMethod:@"pushRoute"
arguments:@"/custom/route?query=test"]);
OCMVerify([self.mockNavigationChannel invokeMethod:@"pushRouteInformation"
arguments:@{@"location" : @"/custom/route?query=test"}]);
}

@end