Skip to content
Snippets Groups Projects
Unverified Commit 975a6775 authored by Victoria Ashworth's avatar Victoria Ashworth Committed by GitHub
Browse files

Make iOS Flutter framework extension-safe (#165346)

Our current
[adoption/documentation](https://docs.flutter.dev/platform-integration/ios/app-extensions)
for iOS Extensions does not currently work because it's disallowed to
nest bundles (see https://github.com/flutter/flutter/issues/142531).

As of [Xcode
13](https://developer.apple.com/documentation/xcode-release-notes/xcode-13-release-notes/#:~:text=Linking%20Swift%20packages%20from%20application%20extension%20targets%20or%20watchOS%20applications%20no%20longer%20emits%20unresolvable%20warnings%20about%20linking%20to%20libraries%20not%20safe%20for%20use%20in%20application%20extensions%2E),
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
https://github.com/flutter/flutter/issues/165086).

Fixes https://github.com/flutter/flutter/issues/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

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

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
parent 3efb8cc3
Showing
with 794 additions and 274 deletions
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment