Make iOS Flutter framework extension-safe #165346

vashworth · 2025-03-17T21:59:45Z

Our current adoption/documentation for iOS Extensions does not currently work because it's disallowed to nest bundles (see #142531).

As of Xcode 13, linking to frameworks that contain non-extension-safe code no longer gives warnings (or blocks from App Store it seems). Therefore, it has become a runtime issue to ensure non-extension-safe code is not used.

Previously, we were building and shipping 2 separate Flutter.xcframeworks. One that was extension-safe and one that was not.

This PR removes the "extension_safe" framework and instead makes the entire framework extension-safe by annotating non-extension-safe code with NS_EXTENSION_UNAVAILABLE_IOS and ensuring that code is not used at runtime if the bundle is for an app extension.

This PR also disables wide gamut for app extensions to decrease the chances of crashes (see #165086).

Fixes #142531.

For reference:

App extensions were first evaluated in https://flutter.dev/go/app-extensions. Here is the reasoning why neither method described there is opportune.

Option 1 -
I did look into splitting the framework into 2 frameworks (one with all extension-safe code and one with the non-extension-safe code). However, the original idea was to use objc Categories/Extensions - this doesn’t quite fit our needs. Categories/Extensions only allow you to add new methods/properties, not override existing ones. We could hypothetically add new methods, but that would require the user to change their code to use the new methods.

I also looked into subclasses which does allow overrides, but it would also require the user to change their code to use the new class.

We could do method swizzling, but opinion of that on the internet is that it's not very safe.

I’m of the opinion that anything that requires the user to change code isn’t super feasible due to plugins.

Option 2 -
We could still do the 2 frameworks but rename one to FlutterExtentionSafe. This works without users needing to change any code (including imports like @import Flutter / #import <Flutter/Flutter.h>). I believe the reason this works is because at compile time, it finds the Flutter framework on the framework search path and it imports in the headers. Then at link time, FlutterExtentionSafe is explicitly linked so it uses that framework first when checking for symbols and since it finds all the symbols in FlutterExtentionSafe, it doesn’t need/try to auto-link the Flutter framework (despite Flutter being the framework imported).

This seems precarious to me since we’re relying on Xcode to not auto-link the Flutter framework. If for some reason Flutter framework did get auto-linked (such as the user using a symbol that’s not in the FlutterExtensionSafe framework but is in the Flutter framework - this is unlikely though), we’d get name collision issues

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.

…on_framework_v2

jmagman

Mostly minor nits except for the comment about keeping the old artifact for a release cycle, even if it's identical to Flutter.framework.

jmagman · 2025-03-21T16:13:07Z

dev/bots/suite_runners/run_verify_binaries_codesigned_tests.dart

-    'artifacts/engine/ios-profile/extension_safe/Flutter.xcframework/ios-arm64/Flutter.framework/Flutter',
-    'artifacts/engine/ios-profile/extension_safe/Flutter.xcframework/ios-arm64_x86_64-simulator/Flutter.framework/Flutter',


Sooo nice we can make this change atomically with the engine now.

jmagman · 2025-03-21T16:14:12Z

engine/src/flutter/ci/builders/mac_ios_engine.json

@@ -248,253 +248,6 @@
                    "sdk_version": "16c5032a"
                }
            }
-        },


I wonder if we should keep building these artifacts for a release (even though it doesn't really work) in case someone is using it, and then put the deprecation in the tech blog with instructions for migrating? I bet people have prototypes that "work" locally and I don't want them to be mysteriously busted.

I'm fine either way as I expect this is a relatively quick and painless fix for users, but agreed, it seems like a good idea to give people a release worth of heads-up in the release notes.

jmagman · 2025-03-21T16:16:48Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

-  if (@available(iOS 13.0, *)) {
-    [self setUpSceneLifecycleNotifications:center];
-  } else {
+  if ([FlutterSharedApplication isAvailable]) {


Dot notation for properties, including class properties.

Suggested change

if ([FlutterSharedApplication isAvailable]) {

if (FlutterSharedApplication.isAvailable) {

jmagman · 2025-03-21T16:19:05Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

-  if (@available(iOS 13.0, *)) {
-    [self setUpSceneLifecycleNotifications:center];
-  } else {
+  if ([FlutterSharedApplication isAvailable]) {
    [self setUpApplicationLifecycleNotifications:center];


Instead of the caller (in two places) needing to understand the extension and API limitations of this call, can the logic be pushed down into one setUpLifecycleNotifications: method?

jmagman · 2025-03-21T16:20:29Z

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

+/**
+ * Check whether the main bundle is an iOS App Extension.
+ */
+ (BOOL)isAppExtension;


This should be a readonly class property.

Suggested change

+ (BOOL)isAppExtension;

@property(class, nonatomic, readonly) BOOL isAppExtension;

jmagman · 2025-03-21T16:20:35Z

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

+/**
+ * Check whether the UIApplication is available. UIApplication is not available for App Extensions.
+ */
+ (BOOL)isAvailable;


jmagman · 2025-03-21T16:21:00Z

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

+/**
+ * Returns the `UIApplication.sharedApplication` is available. Otherwise returns nil.
+ */
+ (UIApplication*)uiApplication;


uiApplication with the prefix is kind of odd looking, how about sharedApplication or just application?
Also a class property.

Agreed just application.

jmagman · 2025-03-21T16:22:57Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSharedApplication.mm

+@interface FlutterSharedApplication ()
+
+ (UIApplication*)sharedApplication;
+
+@end


Why is this category necessary?

jmagman · 2025-03-21T16:23:15Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSharedApplication.mm

+@implementation FlutterSharedApplication
+
+ (BOOL)isAppExtension {
+  NSDictionary* nsExtension = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"NSExtension"];


Suggested change

NSDictionary* nsExtension = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"NSExtension"];

NSDictionary* nsExtension = [NSBundle.mainBundle objectForInfoDictionaryKey:@"NSExtension"];

jmagman · 2025-03-21T16:24:06Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterView.mm

@@ -53,6 +54,12 @@ - (MTLPixelFormat)pixelFormat {
 - (BOOL)isWideGamutSupported {
  FML_DCHECK(self.screen);

+  // Wide Gamut is not supported for iOS Extensions due to memory limitations
+  // (see https://github.com/flutter/flutter/issues/165086).


Good catch figuring out that would help with memory.

cbracken

This is exciting! Overall looks good!

cbracken · 2025-03-21T16:54:56Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm

+                                  userInfo:nil];
+  id mockEngine = OCMPartialMock(engine);
+  [[NSNotificationCenter defaultCenter] postNotification:sceneNotification];
+  [[NSNotificationCenter defaultCenter] postNotification:applicationNotification];


NSNotificationCenter.defaultCenter

cbracken · 2025-03-21T16:55:20Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm

+                                  userInfo:nil];
+  id mockEngine = OCMPartialMock(engine);
+  [[NSNotificationCenter defaultCenter] postNotification:sceneNotification];
+  [[NSNotificationCenter defaultCenter] postNotification:applicationNotification];


cbracken · 2025-03-21T17:01:49Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

-                     UISceneActivationStateBackground;
+  UIApplication* flutterApplication = [FlutterSharedApplication uiApplication];
+  if (flutterApplication != nil) {
+    _isGpuDisabled = flutterApplication.applicationState == UIApplicationStateBackground;


This is consistent with surrounding code, but this method is about half and half usage of self.foo and _foo. I did passes on a few of the classes during ARC migration, but didn't hit the engine. This is defined as a BOOL property in the public header and we're not in an initializer -- this should only be called via [engine run.*] methods. i.e. go for self.isGpuDisabled.

cbracken · 2025-03-21T17:03:47Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

-    _isGpuDisabled = NO;
+    if (@available(iOS 13.0, *)) {
+      _isGpuDisabled = self.viewController.flutterWindowSceneIfViewLoaded.activationState ==
+                       UISceneActivationStateBackground;


Feels like an opportunity to encapsulate lifecycle state somewhere so we don't need long chains of lookups. e.g. self.viewController.activationState or self.viewController.isBackground or similar.

cbracken · 2025-03-21T17:05:42Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm

+  [[NSNotificationCenter defaultCenter] postNotification:applicationNotification];
+  OCMVerify(times(1), [mockEngine sceneDidEnterBackground:[OCMArg any]]);
+  XCTAssertTrue(engine.isGpuDisabled);
+  bool switch_value = false;


Suggested change

bool switch_value = false;

BOOL gpuDisabled = NO;

cbracken · 2025-03-21T18:20:17Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

-              }]];
-#endif
+      UIApplication* flutterApplication = [FlutterSharedApplication uiApplication];
+      NSSet<UIScene*>* scenes;


Suggestion to guarantee we never hand a nil scenes to requestGeometryUpdateForWindowScenes::

Suggested change

NSSet<UIScene*>* scenes;

NSSet<UIScene*>* scenes = [NSSet set];

cbracken · 2025-03-21T18:20:50Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+                                                       id scene, NSDictionary* bindings) {
+              return [scene isKindOfClass:[UIWindowScene class]];
+            }]];
+      } else {


Suggested change

} else {

} else if (self.flutterWindowSceneIfViewLoaded) {

cbracken · 2025-03-21T18:21:47Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+        scenes = self.flutterWindowSceneIfViewLoaded
+                     ? [NSSet setWithObject:self.flutterWindowSceneIfViewLoaded]
+                     : [NSSet set];


Suggested change

scenes = self.flutterWindowSceneIfViewLoaded

? [NSSet setWithObject:self.flutterWindowSceneIfViewLoaded]

: [NSSet set];

scenes = [NSSet setWithObject:self.flutterWindowSceneIfViewLoaded];

cbracken · 2025-03-21T18:22:55Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm

+                                  userInfo:nil];
+  id mockVC = OCMPartialMock(flutterViewController);
+  [[NSNotificationCenter defaultCenter] postNotification:sceneNotification];
+  [[NSNotificationCenter defaultCenter] postNotification:applicationNotification];


Here and below: NSNotificationCenter.defaultCenter

cbracken · 2025-03-21T18:25:04Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSharedApplicationTest.mm

+  OCMStub([mockBundle objectForInfoDictionaryKey:@"NSExtension"]).andReturn(@{
+    @"NSExtensionPointIdentifier" : @"com.apple.share-services"
+  });
+  XCTAssertTrue([FlutterSharedApplication isAppExtension]);


Here and below: dot notation.

vashworth added 5 commits March 17, 2025 10:59

Exclude non-extension-safe code at runtime

d6c75ee

update tests

0c1d9b0

fix text input

bca9ec1

remove extension_safe framework stuff

f1fd04c

fix formatting

3c4fec6

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 Mar 17, 2025

vashworth added 4 commits March 17, 2025 17:23

disable wide gamut for extensions, add some comments

bfb90e3

fix licenses, clean up some code

e695938

update test

605e056

Merge remote-tracking branch 'upstream/master' into prototype_extensi…

160da46

…on_framework_v2

vashworth marked this pull request as ready for review March 19, 2025 16:08

vashworth requested a review from matanlurey as a code owner March 19, 2025 16:08

vashworth requested review from cbracken and removed request for matanlurey March 19, 2025 16:08

vashworth mentioned this pull request Mar 20, 2025

Scroll behaviour in Flutter view within an iOS share extension is broken #164670

Open

jmagman reviewed Mar 21, 2025

View reviewed changes

cbracken reviewed Mar 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make iOS Flutter framework extension-safe #165346

Make iOS Flutter framework extension-safe #165346

vashworth commented Mar 17, 2025 •

edited

Loading

jmagman left a comment

jmagman Mar 21, 2025

jmagman Mar 21, 2025

cbracken Mar 21, 2025

jmagman Mar 21, 2025

jmagman Mar 21, 2025

jmagman Mar 21, 2025

jmagman Mar 21, 2025

jmagman Mar 21, 2025

cbracken Mar 21, 2025

jmagman Mar 21, 2025

jmagman Mar 21, 2025

jmagman Mar 21, 2025

cbracken left a comment

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

cbracken Mar 21, 2025

		'artifacts/engine/ios-profile/extension_safe/Flutter.xcframework/ios-arm64/Flutter.framework/Flutter',
		'artifacts/engine/ios-profile/extension_safe/Flutter.xcframework/ios-arm64_x86_64-simulator/Flutter.framework/Flutter',

	if ([FlutterSharedApplication isAvailable]) {
	if (FlutterSharedApplication.isAvailable) {

	+ (BOOL)isAppExtension;
	@property(class, nonatomic, readonly) BOOL isAppExtension;

	NSDictionary* nsExtension = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"NSExtension"];
	NSDictionary* nsExtension = [NSBundle.mainBundle objectForInfoDictionaryKey:@"NSExtension"];

	NSSet<UIScene> scenes;
	NSSet<UIScene> scenes = [NSSet set];

Make iOS Flutter framework extension-safe #165346

Are you sure you want to change the base?

Make iOS Flutter framework extension-safe #165346

Conversation

vashworth commented Mar 17, 2025 • edited Loading

Pre-launch Checklist

jmagman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbracken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vashworth commented Mar 17, 2025 •

edited

Loading