-
Notifications
You must be signed in to change notification settings - Fork 6k
Add application:openURLs: forwarding on macOS #44689
Add application:openURLs: forwarding on macOS #44689
Conversation
Wires `application:openURLs:` into the exisiting delegation system, allowing plugins to handle URL callbacks (as on iOS). Since there is no notification-based version of this delegate method, this adds it directly to the app delegate, restructring the helper class slightly to allow internal sharing of the delegate list. Part of flutter/flutter#41471
| * The application menu in the menu bar. | ||
| */ | ||
| @property(weak, nonatomic) IBOutlet NSMenu* applicationMenu; | ||
| @property(weak, nonatomic, nullable) IBOutlet NSMenu* applicationMenu; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed adding these when I added nullability annotations to the split-out delegate above, which causes warnings about not having full nullability annotation.
| - (void)removeDelegate:(NSObject<FlutterAppLifecycleDelegate>*)delegate { | ||
| NSUInteger index = [[_delegates allObjects] indexOfObject:delegate]; | ||
| if (index >= 0) { | ||
| if (index != NSNotFound) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed while I was looking at the existing structure; the previous code was wrong since index is unsigned. NSNotFound is the unsigned version of -1, but not actually negative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
shell/platform/darwin/macos/framework/Source/FlutterAppLifecycleDelegate.mm
Show resolved
Hide resolved
cbracken
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall lgtm barring one question on the removed nil check.
| EXPECT_EQ([delegate receivedURLs], URLs); | ||
| } | ||
|
|
||
| TEST(FlutterAppDelegateTest, OperURLsStopsAfterHandled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice - thanks for adding this one.
| - (void)removeDelegate:(NSObject<FlutterAppLifecycleDelegate>*)delegate { | ||
| NSUInteger index = [[_delegates allObjects] indexOfObject:delegate]; | ||
| if (index >= 0) { | ||
| if (index != NSNotFound) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
shell/platform/darwin/macos/framework/Source/FlutterAppLifecycleDelegate.mm
Show resolved
Hide resolved
…132522) flutter/engine@56a0e27...785b249 2023-08-14 [email protected] Update CompositionAwareMixin to correctly compute composingBase in Web engine (flutter/engine#44139) 2023-08-14 [email protected] Add application:openURLs: forwarding on macOS (flutter/engine#44689) 2023-08-14 [email protected] Roll Skia from a4377099b25a to 69ea58157190 (1 revision) (flutter/engine#44692) 2023-08-14 [email protected] Update embedder_semantics_update.h imports to include flutter namespace (flutter/engine#44670) 2023-08-14 [email protected] Roll Dart SDK from 3295a353980a to d445f5a18876 (1 revision) (flutter/engine#44691) 2023-08-14 [email protected] Revert "Make run_tests.py assert that the ios test dylib is at least as new as libFlutter.dylib" (flutter/engine#44690) 2023-08-14 [email protected] Roll Skia from 1cf6f71c8120 to a4377099b25a (1 revision) (flutter/engine#44688) 2023-08-14 [email protected] Roll Fuchsia Mac SDK from JKTVoxVrHjZjBaxG4... to uIGMbDkXoIcpqWjgR... (flutter/engine#44687) 2023-08-14 [email protected] [Impeller] Conditionally set command debug info. (flutter/engine#44274) 2023-08-14 [email protected] Migrate more GL calls of GrBackend* (flutter/engine#44682) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from JKTVoxVrHjZj to uIGMbDkXoIcp If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Wires `application:openURLs:` into the exisiting delegation system, allowing plugins to handle URL callbacks (as on iOS). Since there is no notification-based version of this delegate method, this adds it directly to the app delegate, restructring the helper class slightly to allow internal sharing of the delegate list. Fixes flutter/flutter#41471 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Wires
application:openURLs:into the exisiting delegation system, allowing plugins to handle URL callbacks (as on iOS).Since there is no notification-based version of this delegate method, this adds it directly to the app delegate, restructring the helper class slightly to allow internal sharing of the delegate list.
Fixes flutter/flutter#41471
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.