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

Conversation

@stuartmorgan-g
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g commented Aug 14, 2023

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

  • 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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

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;
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 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) {
Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

Nice catch.

Copy link
Member

@cbracken cbracken left a 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) {
Copy link
Member

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) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 14, 2023
@auto-submit auto-submit bot merged commit 99417da into flutter:main Aug 14, 2023
@stuartmorgan-g stuartmorgan-g deleted the macos-plugin-open-urls-delegation branch August 14, 2023 19:41
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 14, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 14, 2023
…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
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

affects: desktop autosubmit Merge PR when tree becomes green via auto submit App platform-macos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

expose addApplicationDelegate for macOS

2 participants