Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add macOS support for plugin value publishing #45502

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

stuartmorgan
Copy link
Contributor

@stuartmorgan stuartmorgan commented Sep 6, 2023

These directly copy the iOS APIs, to minimize the branching needed in plugins with shared implementation code, and to facilitate the long-term goal of merging the iOS and macOS plugin headers. This does mean replicating the unfortunately non-idiomatic behavior of having valuePublishedByPlugin: sometimes return nil and sometimes return NSNull, instead of distinguishing between nil cases (if that's actually even necessary here) via a more specific API. In isolation I would definitely not design the API with this behavior, but consistency with iOS is the more important factor.

(Eventually I think we'll need a sort of "v2" of iOS plugin APIs since there are a number of strange behaviors that we're currently stuck with, but migrating iOS and macOS together to a new set of APIs won't be any harder than doing just iOS, and in the short to medium term consistency will help the ecosystem more that trying to pre-create better APIs as macOS-only.)

Also fixes FlutterEngineRegistrar to have a weak pointer to the engine. This should really already have been the case since plugins can retain the registrar, creating a likely cycle; it's now a guaranteed cycle (and failed unit tests designed to find cycles) without that since the engine itself is now keeping references to them.

Fixes flutter/flutter#124721

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 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.

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.

lgtm!

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 6, 2023
@auto-submit auto-submit bot merged commit 59c9505 into flutter:main Sep 6, 2023
26 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 7, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 7, 2023
…sions) (#134188)

Manual roll requested by zra@google.com

flutter/engine@2c69d05...75437a3

2023-09-07 zanderso@users.noreply.github.com Revert "Enforce the rule of calling `FlutterView.Render`" (flutter/engine#45525)
2023-09-07 skia-flutter-autoroll@skia.org Roll Skia from 2b76d1113497 to 9e86d3f6239a (1 revision) (flutter/engine#45521)
2023-09-07 bdero@google.com [Impeller] Gaussian blur: Remove the current blur style implementation. (flutter/engine#45520)
2023-09-06 skia-flutter-autoroll@skia.org Roll Skia from 9c9757c5d17d to 2b76d1113497 (2 revisions) (flutter/engine#45518)
2023-09-06 skia-flutter-autoroll@skia.org Roll Skia from 0039caadd635 to 9c9757c5d17d (1 revision) (flutter/engine#45516)
2023-09-06 skia-flutter-autoroll@skia.org Roll ANGLE from 00daa451320c to 60b56591dee5 (1 revision) (flutter/engine#45517)
2023-09-06 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 8dgICHnG28wNHzoz3... to SCoDb2m_zQDLrMhwT... (flutter/engine#45514)
2023-09-06 skia-flutter-autoroll@skia.org Roll Skia from a274609c442c to 0039caadd635 (2 revisions) (flutter/engine#45513)
2023-09-06 stuartmorgan@google.com Add macOS support for plugin value publishing (flutter/engine#45502)
2023-09-06 skia-flutter-autoroll@skia.org Roll Skia from e5ed4ffaaaa4 to a274609c442c (2 revisions) (flutter/engine#45510)
2023-09-06 zanderso@users.noreply.github.com Roll Fuchsia with license script fix (flutter/engine#45498)
2023-09-06 dkwingsmt@users.noreply.github.com Enforce the rule of calling `FlutterView.Render` (flutter/engine#45300)
2023-09-06 skia-flutter-autoroll@skia.org Roll ANGLE from 7b0bb0f6e785 to 00daa451320c (1 revision) (flutter/engine#45507)
2023-09-06 skia-flutter-autoroll@skia.org Roll Skia from 59e54ccf25a4 to e5ed4ffaaaa4 (4 revisions) (flutter/engine#45506)
2023-09-06 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from dFe-t1SosqZwU5lZR... to hHwU6r12A0sy5Bq-0... (flutter/engine#45505)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from z9uQ0mXwjKFQ to SCoDb2m_zQDL
  fuchsia/sdk/core/mac-amd64 from dFe-t1SosqZw to hHwU6r12A0sy

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 rmistry@google.com,zra@google.com 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: desktop autosubmit Merge PR when tree becomes green via auto submit App platform-macos
Projects
None yet
2 participants