Started bridging the scene delegate to the lifecycle delegate for shortcuts #170180

gaaclarke · Jun 6, 2025

testing: There integration tests in the plugins repo.
fixes: #169928

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…rtcuts

jmagman · Jun 6, 2025

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterAppDelegate_Internal.h


 @interface FlutterAppDelegate ()

+@property(nonatomic, strong, nullable) FlutterPluginAppLifeCycleDelegate* lifeCycleDelegate;


keep the readwrite implementation property as-is, add readonly to this.

I removed the direct usage of the lifecycle delegate and instead started calling directly into the app delegate as we discussed in our meeting. That's a bit more scary but I think it technically is the safer thing to do.

could you share a bit more details (for me and future readers) on app delegate vs life cycle delegate decision? Thx!

Jenn pointed out that users could be subclassing FlutterAppDelegate and override application:performActionForShortcutItem:completionHandler: and be performing their own logic, like say analytics for the usage of shortcuts.

Talking directly the the lifecycle delegate would break them, but fix the plugin problems. So, I decided to call the delegate directly.

gaaclarke · Jun 6, 2025

quick actions tested locally (since the test won't run as part of presubmit), it works:

hellohuanlin

looks reasonable. can we add some unit tests?

gaaclarke · Jun 6, 2025

looks reasonable. can we add some unit tests?

We have integration tests. We can't add unit tests because it relies on the application delegate being a FlutterAppDelegate which we don't support in our unit test harness.

hellohuanlin · Jun 7, 2025

We can't add unit tests because it relies on the application delegate being a FlutterAppDelegate which we don't support in our unit test harness.

Could you explain more? OCMock should be able to mock the app delegate (I may misunderstand the problem tho).

id mockApplication = OCMClassMock([UIApplication class]);
OCMStub([mockApplication shared]).andReturn(mockApplication);
id mockAppDelegate = OCMClassMock([FlutterAppDelegate class]);
OCMStub([mockApplication delegate]).andReturn(mockApplication)

gaaclarke · Jun 9, 2025

Could you explain more? OCMock should be able to mock the app delegate (I may misunderstand the problem tho).

Thanks, I forgot you could mock like that. Done.

auto-submit · Jun 9, 2025

autosubmit label was removed for flutter/flutter/170180, because - The status or check suite Linux gradle_plugin_fat_apk_test has failed. Please fix the issues identified (or deflake) before re-applying this label.

gaaclarke · Jun 9, 2025

rerunning Linux gradle_plugin_fat_apk_test, issue filed

gaaclarke · Jun 10, 2025

@hellohuanlin friendly ping

hellohuanlin

I have some concerns around relaying scene delegate calls to app delegate, mainly the fact that we are changing Apple's behavior that may surprise us in the future. See more details here: #170037 (comment)

If there's no other way around, I'm fine with relaying the calls, but just wanna hear your thoughts on it first.

hellohuanlin · Jun 10, 2025

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSceneDelegate.m

+  if ([appDelegate isKindOfClass:[FlutterAppDelegate class]]) {
+    [appDelegate application:FlutterSharedApplication.application
+        performActionForShortcutItem:shortcutItem
+                   completionHandler:completionHandler];


If we do it for one method, we should probably do it for all methods for minimal surprise.

This method is special where apple has specifically called out that they will not call the old app delegate method. Things like "will resign active" are still called. The goal of this PR is to fix broken behavior.

Things like "will resign active" are still called.

IIRC from my experiment a few days ago, "will resign active" are not called

Same as "background/foreground" callbacks (there are some discussions around it a few days ago in the chat channel)

Related: #170178

I think we should be test driven. I don't think we have any failing tests related to resign active. We should double check, but I think that should be addressed in a different PR.

also reported in #170037

I think that should be addressed in a different PR.

Either way sounds good to me. I just wanna have a consistent behavior - if we relay 1 call, we should relay all similar calls.

AquaGeek · Jun 10, 2025

I have some concerns around relaying scene delegate calls to app delegate, mainly the fact that we are changing Apple's behavior that may surprise us in the future. See more details here: #170037 (comment)

If there's no other way around, I'm fine with relaying the calls, but just wanna hear your thoughts on it first.

I agree. This feels like the kind of thing that addresses the short-term pain of everyone having to move their code to the scene delegate (or add the relay to their own code) but will create headaches long-term. I would much rather see Flutter take the stance that code should be moved to align with platform expectations.

Another approach would be to spin up a subclass that does the relaying. Teams that don't want that functionality would simply use the superclass instead.

gaaclarke · Jun 10, 2025

but will create headaches long-term.

@AquaGeek what headaches are you thinking of? Can you explain further?

gaaclarke · Jun 10, 2025

I have some concerns around relaying scene delegate calls to app delegate, mainly the fact that we are changing Apple's behavior that may surprise us in the future. See more details here: #170037 (comment)

If there's no other way around, I'm fine with relaying the calls, but just wanna hear your thoughts on it first.

@hellohuanlin What do you see the alternative being?

Here's the options I see:

Update the migration guide
Bridge FlutterSceneDelegate to FlutterAppDelegate (current state of PR)
Duplicate the logic from FlutterAppDelegate in FlutterSceneDelegate (original state of PR)

We chose to do 2 because 3 could end up breaking users without any runtime help. I'm not quite sure what the problem we are trying to avoid is. I think there is just a general sense that maybe something bad can happen, but I couldn't think of one. All of this happens under the hood and we can swap it around if we want.

hellohuanlin · Jun 10, 2025

I think there is just a general sense that maybe something bad can happen, but I couldn't think of one.

This resonate with my last paragraph in #170037 (comment)

Here's the options I see...

Maybe it is the least worst option we have (unless we can think of a better option?)

I'm not against this. I just wanna make sure we are aware of this departure of apple's practice, and get it well documented if we do so.

This reverts commit 10b32cd.

gaaclarke · Jun 11, 2025

I switched back to talking the the lifecycle delegate directly. While I think in general we should make our decisions based on concrete concerns and tests, not calling the FlutterAppDelegate isn't technically a breaking change even if it can break some customers. So, I'm fine doing the more reserved solution of talking to the delegate directly.

Keeping the logic inline with Apple's will be more understandable at least for the odd scenarios where it may matter.

hellohuanlin · Jun 12, 2025

What do you think about we firstly start with relaying the calls, and adding the warning in tooling against app delegate usage (apple's deprecation warning isn't sufficient as we discussed yesterday), and then remove the relay in the future? This allows a smoother transition for developers.

gaaclarke · Jun 12, 2025

What do you think about we firstly start with relaying the calls, and adding the warning in tooling against app delegate usage (apple's deprecation warning isn't sufficient as we discussed yesterday), and then remove the relay in the future? This allows a smoother transition for developers.

I don't think it's necessary. The people this effects is low, there is no longer an automatic migration and and it's not a Breaking Change. I think that will just complicate the migration. Who knows what decision will be made in the future but migrating may never be an automatic thing considering the events of the last few days. I plan on filing an issue for the static warning like we discussed yesterday as part of resolving this PR, so that idea doesn't get lost.

hellohuanlin · Jun 12, 2025

Sounds good. I think i'm comfortable with this. Before stamping this, @jmagman FYI this reverts to the original implementation that you had concerns: #170180 (comment)

Started bridging the scene delegate to the lifecycle delegate for sho…

d09061b

…rtcuts

github-actions bot added platform-ios iOS applications specifically engine flutter/engine repository. See also e: labels. team-ios Owned by iOS platform team labels Jun 6, 2025

vashworth mentioned this pull request Jun 6, 2025

☂ Adopting UIScene lifecycle in Flutter #170171

Open

jmagman reviewed Jun 6, 2025

View reviewed changes

gaaclarke added 2 commits June 6, 2025 14:59

jenn feedback

10b32cd

reverted unnecessary changes

89f4a57

gaaclarke marked this pull request as ready for review June 6, 2025 22:17

gaaclarke requested a review from a team as a code owner June 6, 2025 22:17

gaaclarke requested review from jmagman and vashworth June 6, 2025 22:17

hellohuanlin reviewed Jun 6, 2025

View reviewed changes

hellohuanlin mentioned this pull request Jun 9, 2025

[iOS] Applifecycle events such as applicationDidBecomeActive and applicationWillResignActive are not called on master version #170037

Open

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 9, 2025

gaaclarke mentioned this pull request Jun 9, 2025

[Reland3] Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (#169276) #169365

Merged

9 tasks

added unit test

1bde78c

gaaclarke requested a review from hellohuanlin June 9, 2025 20:35

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 9, 2025

licenses

3fbe26d

gaaclarke mentioned this pull request Jun 10, 2025

Deeplinks on iOS fail silently after UISceneDelegate migration #170350

Open

hellohuanlin reviewed Jun 10, 2025

View reviewed changes

gaaclarke requested a review from hellohuanlin June 11, 2025 15:23

gaaclarke added 2 commits June 11, 2025 08:25

Revert "jenn feedback"

8d5e475

This reverts commit 10b32cd.

updated test for talking to the delegate directly

73c54ec


		@interface FlutterAppDelegate ()

		@property(nonatomic, strong, nullable) FlutterPluginAppLifeCycleDelegate* lifeCycleDelegate;

Search code, repositories, users, issues, pull requests...

Started bridging the scene delegate to the lifecycle delegate for shortcuts #170180

Are you sure you want to change the base?

Started bridging the scene delegate to the lifecycle delegate for shortcuts #170180

Uh oh!

Conversation

gaaclarke commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

jmagman Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jun 6, 2025

Uh oh!

hellohuanlin left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jun 6, 2025

Uh oh!

hellohuanlin commented Jun 7, 2025

Uh oh!

gaaclarke commented Jun 9, 2025

Uh oh!

auto-submit bot commented Jun 9, 2025

Uh oh!

gaaclarke commented Jun 9, 2025

Uh oh!

gaaclarke commented Jun 10, 2025

Uh oh!

hellohuanlin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AquaGeek commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaaclarke commented Jun 10, 2025

Uh oh!

gaaclarke commented Jun 10, 2025

Uh oh!

hellohuanlin commented Jun 10, 2025

Uh oh!

gaaclarke commented Jun 11, 2025

Uh oh!

hellohuanlin commented Jun 12, 2025

Uh oh!

gaaclarke commented Jun 12, 2025

Uh oh!

hellohuanlin commented Jun 12, 2025

Uh oh!

Uh oh!

gaaclarke commented Jun 6, 2025 •

edited

Loading

jmagman Jun 6, 2025 •

edited

Loading

hellohuanlin Jun 6, 2025 •

edited

Loading

hellohuanlin Jun 10, 2025 •

edited

Loading

AquaGeek commented Jun 10, 2025 •

edited

Loading