Skip to content

iOS: Improve FlutterEngineGroup tests - #191563

Merged
cbracken merged 1 commit into
flutter:masterfrom
cbracken:add-enginegrouptests
Aug 24, 2026
Merged

cbracken merged 1 commit into
flutter:masterfrom
cbracken:add-enginegrouptests

Conversation

@cbracken

Copy link
Copy Markdown
Member

Adds tests for [FlutterEngine spawn] that do checks at the flutter::Shell level.

testSpawn already verified that a spawned engine shares its spawner's thread host this adds tests that ensure that spawned engines share a single Dart VM, shared task runners, and that the GPU enabled/disabled state is correctly set on the underlying Shell, not just on the FlutterEngine itself.

This fills in some test coverage gaps before we start moving iOS towards the embedder API.

Issue: #112232

Pre-launch Checklist

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

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@cbracken
cbracken requested a review from a team as a code owner August 24, 2026 03:40
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Aug 24, 2026
@github-actions github-actions Bot added platform-ios iOS applications specifically engine flutter/engine related. See also e: labels. team-ios Owned by iOS platform team labels Aug 24, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds unit tests to FlutterEngineGroupTest.mm to verify that spawned engines correctly share the Dart VM and task runners, and that the GPU-disabled state is properly propagated. The review feedback suggests simplifying the SyncSwitch handlers in the GPU-disabled test by removing the redundant SetIfFalse handler.

Adds tests for `[FlutterEngine spawn]` that do checks at the
`flutter::Shell` level.

`testSpawn` already verified that a spawned engine shares its spawner's thread
host this adds tests that ensure that spawned engines share a single
Dart VM, shared task runners, and that the GPU enabled/disabled state is
correctly set on the underlying `Shell`, not just on the `FlutterEngine`
itself.

This fills in some test coverage gaps before we start moving iOS towards
the embedder API.

Issue: flutter#112232
@cbracken
cbracken force-pushed the add-enginegrouptests branch from a7dc22d to 868fd5c Compare August 24, 2026 03:49
XCTAssertEqual(&spawner.threadHost, &spawnee.threadHost);
XCTAssertEqual(spawner.isGpuDisabled, spawnee.isGpuDisabled);

// Verify the shell came up with GPU disabled.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious - when will gpu be enabled for the shell? Before enabling it, engine.gpuDisabled & engine.shell.gpuDisabled are out of sync? Would it make more sense to have 1 single source of truth?

@cbracken cbracken Aug 24, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be that setting it was broken -- even though we relied on it in tests, we always set it such that it matched what the engine did by default.

These days, it always matches what Shell has. Or to be more precise, until Shell has been created, FlutterEngine is the source of truth and we use its value to set the initial value for Shell. When we create the shell, we set it to match what FlutterEngine has. From shell creation onwards they always match.

The only reason for the ivar is that _shell doesn't exist until FlutterEngine run/spawn.

Long-term it makes sense to just get rid of the property on FlutterEngine altogether. We can expose internal API to set this on startup for tests and always read off shell.

@cbracken cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 24, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 24, 2026
@auto-submit

auto-submit Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/191563, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@cbracken
cbracken added this pull request to the merge queue Aug 24, 2026
Merged via the queue into flutter:master with commit 3eabedf Aug 24, 2026
23 checks passed
@cbracken
cbracken deleted the add-enginegrouptests branch August 24, 2026 09:03
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Aug 24, 2026
flutter/flutter@65c9a8d...9a82789

2026-08-24 [email protected] Roll Packages from 252bb33 to df2ba94 (5 revisions) (flutter/flutter#191578)
2026-08-24 [email protected] Roll Dart SDK from cb056da580b6 to 78bbc37b6ff6 (13 revisions) (flutter/flutter#191574)
2026-08-24 [email protected] Roll Skia from 50452b13f05b to 57968d087d18 (1 revision) (flutter/flutter#191572)
2026-08-24 [email protected] Move more RSE rendering cases to complex_rse.frag (flutter/flutter#191235)
2026-08-24 [email protected] Roll Skia from 925e13559532 to 50452b13f05b (2 revisions) (flutter/flutter#191570)
2026-08-24 [email protected] iOS: Add stylus pointer tests (flutter/flutter#191564)
2026-08-24 [email protected] iOS: Improve FlutterEngineGroup tests (flutter/flutter#191563)
2026-08-24 [email protected] Roll Skia from c78782c7c1c2 to 925e13559532 (5 revisions) (flutter/flutter#191568)
2026-08-24 [email protected] Roll Skia from 886302c7c68e to c78782c7c1c2 (1 revision) (flutter/flutter#191566)
2026-08-24 [email protected] Revert "Improve initial size and placement logic for popups and toolt…ips on Linux (#189994) (flutter/flutter#191498)
2026-08-23 [email protected] Roll Fuchsia Linux SDK from Cqs-NyeELd60hqv1e... to 8Xu4ujBJniC0nQGx3... (flutter/flutter#191561)
2026-08-23 [email protected] Fixes MSAA on the linux embedder. (flutter/flutter#191379)
2026-08-23 [email protected] Remove Material/Cupertino references in error messages (flutter/flutter#191305)
2026-08-23 [email protected] Roll Skia from a1825062f832 to 886302c7c68e (1 revision) (flutter/flutter#191559)
2026-08-23 [email protected] Roll Skia from ad2106c0bb64 to a1825062f832 (1 revision) (flutter/flutter#191555)
2026-08-22 [email protected] Roll Fuchsia Linux SDK from ic6GjOSn-KN508XyK... to Cqs-NyeELd60hqv1e... (flutter/flutter#191547)
2026-08-22 [email protected] [WebParagraph] Fixing minor problem with getBoxesForRange (flutter/flutter#191419)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
jagadeesh8682 pushed a commit to jagadeesh8682/packages that referenced this pull request Sep 2, 2026
…r#12578)

flutter/flutter@65c9a8d...9a82789

2026-08-24 [email protected] Roll Packages from 252bb33 to df2ba94 (5 revisions) (flutter/flutter#191578)
2026-08-24 [email protected] Roll Dart SDK from cb056da580b6 to 78bbc37b6ff6 (13 revisions) (flutter/flutter#191574)
2026-08-24 [email protected] Roll Skia from 50452b13f05b to 57968d087d18 (1 revision) (flutter/flutter#191572)
2026-08-24 [email protected] Move more RSE rendering cases to complex_rse.frag (flutter/flutter#191235)
2026-08-24 [email protected] Roll Skia from 925e13559532 to 50452b13f05b (2 revisions) (flutter/flutter#191570)
2026-08-24 [email protected] iOS: Add stylus pointer tests (flutter/flutter#191564)
2026-08-24 [email protected] iOS: Improve FlutterEngineGroup tests (flutter/flutter#191563)
2026-08-24 [email protected] Roll Skia from c78782c7c1c2 to 925e13559532 (5 revisions) (flutter/flutter#191568)
2026-08-24 [email protected] Roll Skia from 886302c7c68e to c78782c7c1c2 (1 revision) (flutter/flutter#191566)
2026-08-24 [email protected] Revert "Improve initial size and placement logic for popups and toolt…ips on Linux (#189994) (flutter/flutter#191498)
2026-08-23 [email protected] Roll Fuchsia Linux SDK from Cqs-NyeELd60hqv1e... to 8Xu4ujBJniC0nQGx3... (flutter/flutter#191561)
2026-08-23 [email protected] Fixes MSAA on the linux embedder. (flutter/flutter#191379)
2026-08-23 [email protected] Remove Material/Cupertino references in error messages (flutter/flutter#191305)
2026-08-23 [email protected] Roll Skia from a1825062f832 to 886302c7c68e (1 revision) (flutter/flutter#191559)
2026-08-23 [email protected] Roll Skia from ad2106c0bb64 to a1825062f832 (1 revision) (flutter/flutter#191555)
2026-08-22 [email protected] Roll Fuchsia Linux SDK from ic6GjOSn-KN508XyK... to Cqs-NyeELd60hqv1e... (flutter/flutter#191547)
2026-08-22 [email protected] [WebParagraph] Fixing minor problem with getBoxesForRange (flutter/flutter#191419)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

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

CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-ios iOS applications specifically team-ios Owned by iOS platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants