Made platform message responses thread-safe for macos. #37607

gaaclarke · 2022-11-14T23:40:22Z

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.

gaaclarke · 2022-11-17T20:45:15Z

I just noticed the CI failures here. They are apropos. I'll drop this back to a draft until I get a chance to investigate them.

flutter-dashboard · 2022-11-17T20:50:47Z

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.

gaaclarke · 2022-11-17T22:37:37Z

The failure isn't in the test per se, but it appears to be a bug in the embedder that manifests with how the test is executing. Filed an issue here: flutter/flutter#115599

gaaclarke · 2022-11-18T19:15:34Z

This PR is blocked on flutter/flutter#115599 (which has an outstanding PR to fix it)

gaaclarke · 2022-11-18T21:10:39Z

@a-wallen @cbracken The underlying issue with the tests and accessibility has been resolved. This is ready for review now that that has been fixed.

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm

…ter/engine#37607)

…115881) * c05e7ff9b [Impeller] opt float/sampler into relaxed precision for gles (flutter/engine#37828) * 256dc78f6 Reland "[web] Avoid returning int from js interop classes." (flutter/engine#37627) * 5ab29c95a [web] Move unicode properties to third_party (flutter/engine#37440) * 10da1a1f0 Remove setSampler from FragmentShader (flutter/engine#37839) * a800e7650 [Impeller] make VerticesGeometry delegate to the DL class (flutter/engine#37835) * 1067cd2b7 Roll Skia from 3b2d9e4bf668 to 3bd2fe46f6d2 (2 revisions) (flutter/engine#37848) * 2d8e53925 change cloneImageElement() to return clone every time (flutter/engine#37811) * cefb954e0 fix pixel ratio (flutter/engine#37268) * c6b2ced1e Set nested clip nodes (flutter/engine#37850) * c7ecca866 Roll Skia from 3bd2fe46f6d2 to c098e3c5d932 (2 revisions) (flutter/engine#37851) * afac22d6c Made platform message responses threadsafe for macos. (flutter/engine#37607) * a805efffb fix docs analysis error on setImageSampler (flutter/engine#37852)

…lutter#115881) * c05e7ff9b [Impeller] opt float/sampler into relaxed precision for gles (flutter/engine#37828) * 256dc78f6 Reland "[web] Avoid returning int from js interop classes." (flutter/engine#37627) * 5ab29c95a [web] Move unicode properties to third_party (flutter/engine#37440) * 10da1a1f0 Remove setSampler from FragmentShader (flutter/engine#37839) * a800e7650 [Impeller] make VerticesGeometry delegate to the DL class (flutter/engine#37835) * 1067cd2b7 Roll Skia from 3b2d9e4bf668 to 3bd2fe46f6d2 (2 revisions) (flutter/engine#37848) * 2d8e53925 change cloneImageElement() to return clone every time (flutter/engine#37811) * cefb954e0 fix pixel ratio (flutter/engine#37268) * c6b2ced1e Set nested clip nodes (flutter/engine#37850) * c7ecca866 Roll Skia from 3bd2fe46f6d2 to c098e3c5d932 (2 revisions) (flutter/engine#37851) * afac22d6c Made platform message responses threadsafe for macos. (flutter/engine#37607) * a805efffb fix docs analysis error on setImageSampler (flutter/engine#37852)

This updates the guidelines about threading and the responses to platform channels. Once the following PRs are on `main` all official platforms (minus web where it doesn't make sense) support thread-safe responses. issue: flutter/flutter#93945 Do no land until the following are on stable: 1) flutter/engine#37689 1) flutter/engine#37607 1) flutter/engine#36909 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> Co-authored-by: Parker Lougheed <[email protected]>

@goderbauer

* Adding state restoration pages (#8424) Fixes #2004 Fixes another issue that I can't find atm. [Staged link](https://sz-flutter-2.web.app/development/platform-integration/android/restore-state-android) @goderbauer, there are questions for you in this PR. cc @goderbauer --------- Co-authored-by: Parker Lougheed <[email protected]> * Fix typo "priori" -> "prior" (#8573) _Description of what this PR is changing or adding, and why:_ _Issues fixed by this PR (if any): Fix typo in `src/resources/inside-flutter.md:589` - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/master/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: Brett Morgan <[email protected]> Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> * Replace Webby mention with I/O in banner (#8627) The Webby voting has ended. This PR removes the Webby mention and reintroduces the I/O call to action. <img width="559" alt="Screenshot of banner" src="https://user-images.githubusercontent.com/18372958/234385170-785d7be7-9b39-4752-b398-95a7e7f987a7.png"> Co-authored-by: Brett Morgan <[email protected]> * [Proposal] Breakup development directory (#8624) This pull request extracts all subcategories from `/development` to top-level entries, to match similar entries like "Deployment" and "Testing and debugging". The subcategories under Development are perhaps the most important categories for learning Flutter, but they were hidden under Development. This made them harder to navigate, with smaller text, and with deeper links and breadcrumbs. Work done: - Pulled subdirectories out of `/development` - Updated all old redirects and links to new destination - Introduce new redirects so old links keep working - Add some of the new top-level dividers to visually distinguish content - Enable breadcrumbs in moved content - Enable breadcrumbs within "Deployment" - Moved "Add to app" below "Deployment" - Add a short title for Add to app This is part of incremental work, and will be followed up with breaking up and reorganization "User interface", adjusting titles of content, and adding some cookbooks to the sidenav. Staged: https://flutter-docs-prod--pr8624-feature-breakup-deve-00ees3e9.web.app/ * Deprecate `describeEnum`. (#8571) Tied to flutter/flutter#125016 --------- Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> * Moving migration guides to the release directory (#8629) Part of the IA cleanup, moving migration guides to the /release directory and removing them from the sidenav. cc @parlough --------- Co-authored-by: Parker Lougheed <[email protected]> * flavors.md - Updated path of "New Scheme" in the XCode menu. (#8599) Updated path of "New Scheme" in the XCode menu. ![image](https://user-images.githubusercontent.com/4278331/233380485-da5efb42-5ea7-47e1-883e-6a949299332a.png) **IMPORTANT:** Due to work on the docs.flutter.dev infrastructure, **all open pull requests will be closed April 26.** If your PR needs to be merged by April 26, please say that in your PR. Otherwise, please [file an issue](https://github.com/flutter/website/issues/new/choose) about the needed change, and (if you submit a PR) be prepared to recreate the PR May 10 or later. --- _Description of what this PR is changing or adding, and why:_ _Issues fixed by this PR (if any):_ - [ ] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [ ] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [ ] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. * Bump site-shared from `04a5353` to `74292e2` (#8630) * Fix apostrophe in contextual-survey-metadata.json (#8631) Changing apostrophe character in description --- Makes it so that we can parse the json in dart code in the response * Document the new `canvasKitVariant` runtime configuration (#8475) Add documentation for the new [`canvasKitVariant`](https://github.com/flutter/engine/blob/0776f38b87137ad2535d77e91a79b8b6c80f16fb/lib/web_ui/lib/src/engine/configuration.dart#L221-L224) runtime configuration. Closes flutter/flutter#123048 - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/master/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: Anthony Sansone <[email protected]> * Adding wireless debugging information to the docs (#8456) We've added support for wireless debugging of iOS devices. This PR adds documentation for setting it up. To do: - [x] Add in information about IPv4 and IPv6 to `flutter attach` page - [ ] Specify the Flutter release where this feature is available - [x] See if there's any information needed for Android wireless debugging _Issues fixed by this PR (if any):_ #8425 - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/master/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: Victoria Ashworth <[email protected]> Co-authored-by: Brett Morgan <[email protected]> Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> * Adaptation information for inputs and app bars (#8509) This PR adds some information on how to adapt styling for input widgets with .adaptive() constructors, as well as top app bars. Note that I am not sure of the best way to style the tables or size the images. Also, I have added some commented out sections that should be added when stable release goes live. Fixes: #8428 - [X] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [X] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [X] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/master/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. @MitchellGoodwin could you take a quick peak at the code and make sure it looks okay? @InMatrix feel free to propose any edits! --------- Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> * Adapting bottom navigation bar (#8541) This adds to our platform adaptation documentation to add a section on tab bars. This fixes this issue: https://github.com/flutter/website/issues/8540. Builds on top of this PR: #8509 - [X] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [X] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [X] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/master/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> Co-authored-by: Anthony Sansone <[email protected]> * Updated Impeller details (#8607) Fixes #8608 --------- Co-authored-by: Loïc Sharma <[email protected]> * Enable build checks and tests in next branch (#8609) * Update widget catalog to show Material 3 widgets (#8574) Fixes #8432. Site changes are viewable at the staging site: https://flutter-site-73ed1.web.app/development/ui/widgets/. Primary changes: - Addition of Material 3 Components card [(view)](https://flutter-site-73ed1.web.app/development/ui/widgets/). - New Material 3 page showing M3 widgets as displayed in matching categories to material.io/components. This also includes a note about Material 3 becoming the default - this text is not final and can be iterated on in review. - Widget cards in the M3 page have a hover effect applied. - In the widgets overview page, Material now links to M3, and contains a link to the previous M2 widgets page. General notes: - Material 2 page ~~remains unchanged~~ has a notice about Material 3. - No light/dark modes - this was explored but decided against, with the possibility of returning to it if the site undergoes a site-wide dark mode addition. - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/master/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. * Updated the threading notice for platform channel responses. (#7901) This updates the guidelines about threading and the responses to platform channels. Once the following PRs are on `main` all official platforms (minus web where it doesn't make sense) support thread-safe responses. issue: flutter/flutter#93945 Do no land until the following are on stable: 1) flutter/engine#37689 1) flutter/engine#37607 1) flutter/engine#36909 - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> Co-authored-by: Parker Lougheed <[email protected]> * Update PR Template for Website Freeze (#8632) --------- Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> Co-authored-by: Parker Lougheed <[email protected]> Co-authored-by: 失魂魚 <[email protected]> Co-authored-by: Brett Morgan <[email protected]> Co-authored-by: Bernardo Ferrari <[email protected]> Co-authored-by: Dimitris Paxinos <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Elias Yishak <[email protected]> Co-authored-by: Mouad Debbar <[email protected]> Co-authored-by: Anthony Sansone <[email protected]> Co-authored-by: Leigha Jarett <[email protected]> Co-authored-by: Victoria Ashworth <[email protected]> Co-authored-by: Loïc Sharma <[email protected]> Co-authored-by: Eilidh Southren <[email protected]> Co-authored-by: gaaclarke <[email protected]>

flutter-dashboard bot added the platform-macos label Nov 14, 2022

gaaclarke force-pushed the macos-threadsafe-reply branch from 7b0db06 to 7143826 Compare November 14, 2022 23:52

gaaclarke marked this pull request as ready for review November 14, 2022 23:55

gaaclarke requested review from cbracken and a-wallen November 14, 2022 23:55

gaaclarke marked this pull request as draft November 17, 2022 20:50

gaaclarke mentioned this pull request Nov 18, 2022

macos embedder crashes when pumping the event loop with running engine flutter/flutter#115599

Closed

gaaclarke force-pushed the macos-threadsafe-reply branch from 7143826 to bc1824c Compare November 18, 2022 19:18

Made platform message responses threadsafe for macos.

87b0a71

gaaclarke force-pushed the macos-threadsafe-reply branch from bc1824c to 87b0a71 Compare November 18, 2022 21:09

gaaclarke marked this pull request as ready for review November 18, 2022 21:09

a-wallen reviewed Nov 18, 2022

View reviewed changes

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm Show resolved Hide resolved

gaaclarke requested a review from a-wallen November 22, 2022 21:02

a-wallen approved these changes Nov 22, 2022

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 22, 2022

auto-submit bot merged commit afac22d into flutter:main Nov 22, 2022

engine-flutter-autoroll mentioned this pull request Nov 22, 2022

Roll Flutter Engine from 37e2aaa901e8 to afac22d6c640 (11 revisions) flutter/flutter#115878

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022

afac22d6c Made platform message responses threadsafe for macos. (flut…

fd6028a

…ter/engine#37607)

engine-flutter-autoroll mentioned this pull request Nov 22, 2022

Roll Flutter Engine from 37e2aaa901e8 to a805efffba54 (12 revisions) flutter/flutter#115881

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022

afac22d6c Made platform message responses threadsafe for macos. (flut…

1cdfacf

…ter/engine#37607)

gaaclarke mentioned this pull request Dec 2, 2022

Updated the threading notice for platform channel responses. flutter/website#7901

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Made platform message responses thread-safe for macos. #37607

Made platform message responses thread-safe for macos. #37607

Uh oh!

gaaclarke commented Nov 14, 2022 •

edited

Loading

Uh oh!

gaaclarke commented Nov 17, 2022

Uh oh!

flutter-dashboard bot commented Nov 17, 2022

Uh oh!

gaaclarke commented Nov 17, 2022

Uh oh!

gaaclarke commented Nov 18, 2022

Uh oh!

gaaclarke commented Nov 18, 2022

Uh oh!

Uh oh!

Uh oh!

Made platform message responses thread-safe for macos. #37607

Made platform message responses thread-safe for macos. #37607

Uh oh!

Conversation

gaaclarke commented Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gaaclarke commented Nov 17, 2022

Uh oh!

flutter-dashboard bot commented Nov 17, 2022

Uh oh!

gaaclarke commented Nov 17, 2022

Uh oh!

gaaclarke commented Nov 18, 2022

Uh oh!

gaaclarke commented Nov 18, 2022

Uh oh!

Uh oh!

Uh oh!

gaaclarke commented Nov 14, 2022 •

edited

Loading