Build iOS with -fapplication-extension flag #40972

cyanglaz · 2023-04-06T19:22:19Z

original PR: #40332

Add extension_safe flag to build the engine variant that is extension safe.
Based on the above flag, compile out code that contains SharedApplication.

For testing, copy_and_verify_framework_module produces a warning message if the engine is not built with extension_safe when asked to. This warning message does not fail build on its own. A new CI step needs to be added to capture the warning message and fail.

fixes flutter/flutter#124286, flutter/flutter#124290, flutter/flutter#124289

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.

flutter-dashboard · 2023-04-06T19:22:23Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

cyanglaz · 2023-04-06T19:25:07Z

common/config.gni

    "-Werror=undeclared-selector",
  ]
+  if (extension_safe) {
+    flutter_cflags_objc += [ "-fapplication-extension" ]


macOS doesn't fail when building with this flag, but macOS does contain NSApplication code. I'm confused here.

@jmagman @cbracken

I didn't get this working either, might take more investigation to see how what flags Xcode is passing through when it shows the warning.

I tested the locally built mac engine.
Without with this flag, I got the warning when linking a share extension to the framework.
With this flag, there is no warning.
So it seemed like it is just working (which clearly is not since the mac os code does contain NSApplication.)

Will investigate more.
(I should have marked this PR as draft, it is not intended for reviewing yet before this macOS mystery is resolved)

I looked into the headers, on iOS:

@property(class, nonatomic, readonly) UIApplication *sharedApplication NS_EXTENSION_UNAVAILABLE_IOS("Use view controller based solutions where appropriate instead.");
The shared application is marked as extension unavailable.

However, on macOS it is not.
@property (class, readonly, strong) __kindof NSApplication *sharedApplication;

In the document: https://developer.apple.com/library/archive/documentation/General/Conceptual/ExtensibilityPG/ExtensionOverview.html#//apple_ref/doc/uid/TP40014214-CH2-SW6 only linked to the UIApplication.sharedApplication, didn't mention NSApplication.

I create a sample mac framework having Require Only App-Extension-safe API turned on and having NSApplication.SharedApplication. It successfully compiled and a share extension can link to it without warning.

So I believe NSApplication.sharedApplication is compatible with extensions? So on mac, as long as the -fapplication-extension is in the build flags and there is no compile error, it will work.

So I believe NSApplication.sharedApplication is compatible with extensions? So on mac, as long as the -fapplication-extension is in the build flags and there is no compile error, it will work.

!!!
From my reading of the docs when they said sharedApplication without specifying an object I thought they meant either UIApplication or NSApplication.

Access a sharedApplication object, and so cannot use any of the methods on that object

If we don't need to do anything other than documentation for macOS that would be amazing!

And also you could pull that very simple macOS flag-only work out of this so it would start working in the next stable.

By "next stable" I just mean it can be available now-ish without needing to wait for all the iOS work to be correct.

jmagman · 2023-04-07T00:51:19Z

common/config.gni

  # Whether to build host-side development artifacts.
  flutter_build_engine_artifacts = true
+
+  extension_safe = false


Maybe darwin_extension_safe and a comment? There are more "extension" concepts this could be confused with.

jmagman · 2023-04-07T00:52:35Z

shell/platform/darwin/ios/BUILD.gn


+  if (extension_safe) {
+    ldflags = [
+      "-F$framework_search_path",


Surprised this wasn't needed before... maybe copy_and_verify_framework_module isn't actually working (ignoring the extension stuff)?

jmagman · 2023-04-07T00:58:13Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm

+#if !APPLICATION_EXTENSION_API_ONLY
  // Checks if the top status bar should be visible. This platform ignores all
  // other overlays



I didn't do this in my prototype PR, but ideally we could get rid of as many sharedApplication usages as possible.
-[UIApplication statusBarHidden] is actually deprecated in iOS 13:

@property(readonly, nonatomic,getter=isStatusBarHidden) BOOL statusBarHidden API_UNAVAILABLE(tvos) API_DEPRECATED("Use the statusBarManager property of the window scene instead.", ios(2.0, 13.0));

Can you instead swap this to using the new API when 13 is @available, and fall back to sharedApplication statusBarHidden for older versions only? Probably best to do this in its own review though this can sit on in case it causes issues. I don't trust our testing coverage here.

From the [_engine.get() viewController] I think you could get .viewIfLoaded.window.windowScene?

I'm not sure if it's meaningful to have a status bar in an extension, though.

jmagman · 2023-04-07T01:00:36Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm

+#if !APPLICATION_EXTENSION_API_ONLY
    // Note: -[UIApplication setStatusBarStyle] is deprecated in iOS9
    // in favor of delegating to the view controller
    [[UIApplication sharedApplication] setStatusBarStyle:statusBarStyle];


setStatusBarStyle was deprecated in iOS 9 so we wouldn't even need a fallback or the macro (minimum is now iOS 11), if we can get a view controller here ([_engine.get() viewController]?)

- (void)setStatusBarStyle:(UIStatusBarStyle)statusBarStyle animated:(BOOL)animated API_DEPRECATED("Use -[UIViewController preferredStatusBarStyle]", ios(2.0, 9.0)) API_UNAVAILABLE(tvos);

Though I don't really understand the delegateToViewController logic or why we wouldn't want to pass it along to the embedder... #2732

jmagman · 2023-04-07T01:09:07Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm

+#if !APPLICATION_EXTENSION_API_ONLY
    UIViewController* rootViewController =
        [UIApplication sharedApplication].keyWindow.rootViewController;
    if (engineViewController != rootViewController) {


Not sure if we still need to check something here in the share extension to avoid regressing #30939.

jmagman · 2023-04-07T01:10:14Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+      UIWindowScene* windowScene = self.viewIfLoaded.window.windowScene;
+      [self performOrientationUpdateOnWindowScene:windowScene];


I think you could just use this logic instead of anything in the #else, I didn't test it though so in my PR I only used it in the APPLICATION_EXTENSION_API_ONLY check.

jmagman · 2023-04-07T01:11:38Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

    } else {
+#if !APPLICATION_EXTENSION_API_ONLY
      UIInterfaceOrientationMask currentInterfaceOrientation =
          1 << [[UIApplication sharedApplication] statusBarOrientation];


If available you could check the self.viewIfLoaded.window.windowScene.statusBarOrientation since this was deprecated in iOS 13.

@property(readonly, nonatomic) UIInterfaceOrientation statusBarOrientation API_UNAVAILABLE(tvos) API_DEPRECATED("Use the interfaceOrientation property of the window scene instead.", ios(2.0, 13.0));

jmagman · 2023-04-07T01:13:05Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+  UIContentSizeCategory category =
+      self.mainScreenIfViewLoaded.traitCollection.preferredContentSizeCategory;


I think you can just use this in all cases since this is all available on iOS 10+, but I didn't test it so I put it in the macro for safety in my PR.

jmagman · 2023-04-07T01:15:13Z

common/config.gni

    "-Werror=undeclared-selector",
  ]
+  if (extension_safe) {
+    flutter_cflags_objc += [ "-fapplication-extension" ]


I didn't get this working either, might take more investigation to see how what flags Xcode is passing through when it shows the warning.

flutter-dashboard · 2023-04-07T17:11:20Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

jmagman · 2023-04-12T23:00:52Z

shell/platform/darwin/ios/BUILD.gn

  output_name = "Flutter"

-  ldflags = [ "-Wl,-install_name,@rpath/Flutter.framework/Flutter" ]
+  ldflags = [ "-Wl,-install_name,&_flutter_framework_dir/Flutter" ]


Suggested change

ldflags = [ "-Wl,-install_name,&_flutter_framework_dir/Flutter" ]

ldflags = [ "-Wl,-install_name,$_flutter_framework_dir/Flutter" ]

Build iOS and macOS Obj-C with -fapplication-extension flag Linker flag Find the App.framework asset URL from the app extension Remove load draft extension safe built flag format gn format macos test fix clang tidy rename flag add linker flag format draft format: add LINE EOF fix typo format fix run search path fix copying fix build.gn format format

flutter-dashboard bot added platform-ios platform-macos labels Apr 6, 2023

cyanglaz requested a review from jmagman April 6, 2023 19:22

flutter-dashboard bot added the needs tests label Apr 6, 2023

cyanglaz requested a review from cbracken April 6, 2023 19:22

cyanglaz commented Apr 6, 2023

View reviewed changes

cyanglaz changed the title ~~Build iOS Obj-C with -fapplication-extension flag~~ Build iOS and macOS Obj-C with -fapplication-extension flag Apr 6, 2023

jmagman reviewed Apr 7, 2023

View reviewed changes

cyanglaz marked this pull request as draft April 7, 2023 17:10

jmagman mentioned this pull request Apr 11, 2023

Build iOS and macOS Obj-C with -fapplication-extension flag #40332

Closed

jmagman reviewed Apr 12, 2023

View reviewed changes

cyanglaz force-pushed the app_extension branch 2 times, most recently from f1154c2 to 5761631 Compare April 17, 2023 20:25

cyanglaz changed the title ~~Build iOS and macOS Obj-C with -fapplication-extension flag~~ Build iOS with -fapplication-extension flag Apr 17, 2023

cyanglaz force-pushed the app_extension branch from 5761631 to d3d4d79 Compare April 18, 2023 22:52

cyanglaz force-pushed the app_extension branch from d3d4d79 to 8b04fb8 Compare May 11, 2023 20:20

cyanglaz force-pushed the app_extension branch from 8b04fb8 to d6c17ed Compare May 19, 2023 17:34

cyanglaz closed this Jul 25, 2023

		UIWindowScene* windowScene = self.viewIfLoaded.window.windowScene;
		[self performOrientationUpdateOnWindowScene:windowScene];

		UIContentSizeCategory category =
		self.mainScreenIfViewLoaded.traitCollection.preferredContentSizeCategory;

	ldflags = [ "-Wl,-install_name,&_flutter_framework_dir/Flutter" ]
	ldflags = [ "-Wl,-install_name,$_flutter_framework_dir/Flutter" ]

Build iOS with -fapplication-extension flag #40972

Build iOS with -fapplication-extension flag #40972

Uh oh!

Conversation

cyanglaz commented Apr 6, 2023

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Apr 6, 2023

Uh oh!

cyanglaz Apr 6, 2023 • 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!

cyanglaz Apr 7, 2023 • 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!

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!

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!

flutter-dashboard bot commented Apr 7, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cyanglaz Apr 6, 2023 •

edited

Loading

cyanglaz Apr 7, 2023 •

edited

Loading