Add PlatformDispatcher.engineId #163476

knopp · 2025-02-17T12:53:02Z

This PR adds engineId field to PlatformDispatcher. When provided by the engine, this can be used to retrieve the engine instance from native code.

Dart code:

final identifier = PlatformDispatcher.instance.engineId!;

macOS, iOS:

FlutterEngine *engine = [FlutterEngine engineForIdentifier: identifier];

Android:

FlutterEngine engine = FlutterEngine.engineForId(identifier);

Linux

FlEngine *engine = fl_engine_for_id(identifier);

Windows

FlutterDesktopEngineRef engine = FlutterDesktopEngineForId(identifier);

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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.

matanlurey

For dev/integration_tests/android_engine_test, mostly LG, please address comment.

matanlurey · 2025-02-18T17:22:38Z

dev/integration_tests/android_engine_test/test_driver/engine_handle_test.dart

This is ... OK.

A better solution would be to use package:integration_test, and instead just write a (functional) unit test:

void main() { test('verify that...', () { expect(PlatformDispatcher.instance.engineHandle, equals(1), reason: 'Explain what "1" means.'); }); }

However, this is probably not hooked up right now.

If you end up going with this solution, can you file an issue, and leave a TODO For matanlurey, and I'll address it?

I looked into that, but it seems like that'd require some setup and also changes to test runner, I don't think I want to do this as part of this PR. Will happily leave the TODO though :)

dev/integration_tests/android_engine_test/test_driver/engine_handle_test.dart

robert-ancell

The term handle seems inappropriate. In the case of views we have an ID, and this seems to be the correct term to use here.

robert-ancell · 2025-02-18T22:09:46Z

engine/src/flutter/shell/platform/linux/fl_engine.cc

 static constexpr int32_t kPointerPanZoomDeviceId = 1;

+static int64_t next_handle = 1;
+static GHashTable* engine_map;


Having these global values is a red flag - instead the handle should be set with fl_engine_set_handle and retrieved with fl_engine_get_handle. If we need to look up an engine it would probably live in the place that owns the engines, which is likely to be FlApplication (though we only support one engine at this point).

The engine must be obtainable from handle without any other context. Requiring FlApplication instance in order to lookup the engine seems to go against the purpose of this PR?

Reading the linked issue, shouldn't the handle just be the memory address of the engine, and then this can used by the FFI directly? Then we don't need any IDs or maps.

I want to avoid casting it to pointer directly and also detect invalid / outdated handles, so that we can ensure that fl_engine_for_handle will return null if the engine no longer exists.

Alternatively, if we relax the requirements, fl_engine_for_handle implementation can be

g_return_val_if_fail(FL_IS_ENGINE(engine_handle), nullptr); return FL_ENGINE(engine_handle)

I don't feel too strongly about this, if the global map and counter is too much of an issue.

On other platforms we don't have similar checks though, so we'll probably still be using maps. Not sure if inconsistent behavior between platforms is a good trade-off.

The handle should remain valid until FlutterEngineShutdown is called. If the platform and UI thread is the same then we should always have safe interactions.

Personally, I also like Robert's suggestion of using the engine's memory address.

... so that we can ensure that fl_engine_for_handle will return null if the engine no longer exists.

While this would be a nice check, it doesn't completely solve the problem of a plugin holding to an engine pointer after it is destroyed.

Windows already has APIs like ViewControllerFromHandle that will gladly return garbage if you pass a handle that no longer exists.

Not sure if inconsistent behavior between platforms is a good trade-off.

Personally, I think it's reasonable for the API to require the handle is valid, and if not, have undefined behavior like assert or return null.

ViewControllerFromHandle simply casts FlutterDesktopViewControllerRef (C API) to flutter:FlutterWindowsViewController (c++ class). I don't think that's the same. In this case the "handle" is clearly a pointer. I think better comparison would be that once we have the ViewForId method - that will never return garbage for invalid Id.

I think it all depends on the threading model. Passing raw engine pointer as handle can be dangerous if the plugin needs to do the hop from UI to platform thread, because while the callback waits to be executed on platform thread the engine might be destroyed and the plugin has have no way of detecting that handle is invalid.

This would be much less of an issue if we get the threading change in.

@robert-ancell, this is the implementation for fl_engine_for_id now:

FlEngine* fl_engine_for_id(int64_t id) { void* engine = reinterpret_cast<void*>(id); g_return_val_if_fail(FL_IS_ENGINE(engine), nullptr); return FL_ENGINE(engine); }

knopp · 2025-02-18T22:26:36Z

The term handle seems inappropriate. In the case of views we have an ID, and this seems to be the correct term to use here.

Personally I like the term handle better in this context, but I can see how it is not consistent with view identifier. I'll look into it.

Roll Flutter from 2e570ca to 842db35 (59 revisions) flutter/flutter@2e570ca...842db35 2025-03-02 [email protected] Roll Skia from ad64415050aa to 101eee8fce59 (1 revision) (flutter/flutter#164449) 2025-03-02 [email protected] Roll Fuchsia Linux SDK from ln3joxJfRN2XGhvCv... to AO1KirSDI7-MVYNPN... (flutter/flutter#164440) 2025-03-02 [email protected] android: Clean up gen_snapshot artifact build (flutter/flutter#164418) 2025-03-02 [email protected] Start using `bin/cache/engine.{stamp|realm}` instead of `bin/internal/engine.{realm|version}`. (flutter/flutter#164352) 2025-03-01 [email protected] Add macos/android_debug_unopt to local_engine.json (flutter/flutter#164410) 2025-03-01 [email protected] Delete unused build archive targets (flutter/flutter#164414) 2025-03-01 [email protected] Roll Fuchsia Linux SDK from QMun2itYrV_zUYrvW... to ln3joxJfRN2XGhvCv... (flutter/flutter#164423) 2025-03-01 [email protected] Roll Skia from ac14158663ea to ad64415050aa (1 revision) (flutter/flutter#164413) 2025-03-01 [email protected] Update linux_host_engine.json ci/host_release description (flutter/flutter#164402) 2025-03-01 [email protected] In update_engine_version_test.dart, do not populate the test environment with the host platform environment (flutter/flutter#164395) 2025-03-01 [email protected] Roll-forward #164317: Use `bin/cache/engine.stamp` (flutter/flutter#164401) 2025-02-28 [email protected] Make pressing and moving on CupertinoButton closer to native behavior. (flutter/flutter#161731) 2025-02-28 [email protected] Roll Skia from 4005ba3ca7b6 to ac14158663ea (7 revisions) (flutter/flutter#164404) 2025-02-28 [email protected] [macOS] Prepare FlutterKeyboardManager for multi-view (flutter/flutter#163962) 2025-02-28 [email protected] Add PlatformDispatcher.engineId (flutter/flutter#163476) 2025-02-28 [email protected] Move `integration_test.FlutterDeviceScreenshotTest` to the framework slow shard (flutter/flutter#164398) 2025-02-28 [email protected] Fix: Update DelegatedTransition animation parameter correctly (flutter/flutter#163853) 2025-02-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Write an identical value to `bin/cache/engine.stamp` to prepare for migration (#164317)" (flutter/flutter#164396) 2025-02-28 [email protected] Add action for configuring default action of EditableText.onTapUpOutside (flutter/flutter#162575) 2025-02-28 [email protected] Align jvmTarget usages across codebase, while editing build.gradle files align them with android version documentation (flutter/flutter#164200) 2025-02-28 [email protected] Roll Packages from 01d3d5c to 70b41e1 (5 revisions) (flutter/flutter#164380) 2025-02-28 [email protected] [tool] Allow using archiveName in android bundle build (flutter/flutter#162390) 2025-02-28 [email protected] Fix incorrectly checking for invalid environment variables in the tool (flutter/flutter#164101) 2025-02-28 [email protected] Support forward and back buttons (flutter/flutter#164356) 2025-02-28 [email protected] Fix window creation callback for multi-window (flutter/flutter#164353) 2025-02-28 [email protected] Fix flutter doctor usage of eglinfo in failure cases. (flutter/flutter#164334) 2025-02-28 [email protected] Refactor writing of package config in tests (flutter/flutter#163734) 2025-02-28 [email protected] Fixed the issue that Slider's secondaryTrackValue is not updated. (flutter/flutter#163996) 2025-02-28 [email protected] Roll pub packages (flutter/flutter#164357) 2025-02-28 [email protected] Remove `Mac mac_unopt` presubmit retry count (flutter/flutter#164350) 2025-02-28 [email protected] Roll Fuchsia Linux SDK from 1elkOxihZuTEiTXzY... to QMun2itYrV_zUYrvW... (flutter/flutter#164351) 2025-02-28 [email protected] Drag handles only need to be tested on mobile platforms. (flutter/flutter#163723) 2025-02-28 [email protected] [Android] Use java for looking up Android API level. (flutter/flutter#163558) 2025-02-28 [email protected] Write an identical value to `bin/cache/engine.stamp` to prepare for migration (flutter/flutter#164317) 2025-02-27 [email protected] Wires up expanded state in web engine (flutter/flutter#164048) 2025-02-27 [email protected] Remove Cheserton's File (flutter/flutter#164340) 2025-02-27 [email protected] Replace update semantics handler with signal. (flutter/flutter#163583) 2025-02-27 [email protected] Split up the conical gradient fragment shader (flutter/flutter#164058) 2025-02-27 [email protected] Roll Skia from c16b145749d4 to 4005ba3ca7b6 (3 revisions) (flutter/flutter#164339) 2025-02-27 [email protected] Delete and update stale documentation regarding engine/engine hash. (flutter/flutter#164324) 2025-02-27 [email protected] Document how `engine.version` (is/will be) computed (flutter/flutter#164335) 2025-02-27 [email protected] Update conductor to write engine.version file (flutter/flutter#163350) 2025-02-27 [email protected] remove last usages of min/compile/target SdkVersion, align sourceCompatibility across repo and update android version documentation (flutter/flutter#164198) 2025-02-27 [email protected] Update links to the `flutter/engine` repository for the monorepo. (flutter/flutter#164328) 2025-02-27 [email protected] Add empty `io.flutter.app.FlutterApplication` to give deprecation notice, and un-break projects that have not migrated (flutter/flutter#164233) 2025-02-27 [email protected] Revert dart sdks that were causing dartaotruntime issues in g3 (flutter/flutter#164307) ...

github-actions bot added engine flutter/engine related. See also e: labels. a: desktop Running on desktop platform-macos labels Feb 17, 2025

knopp force-pushed the engine_handle branch 3 times, most recently from b11a598 to 9c0aeec Compare February 17, 2025 14:54

knopp requested a review from matanlurey as a code owner February 17, 2025 21:00

github-actions bot added the platform-android Android applications specifically label Feb 17, 2025

knopp force-pushed the engine_handle branch from f823cb7 to 1217feb Compare February 17, 2025 21:07

knopp changed the title ~~WIP: Add PlatformDispatcher.engineHandle to allow accessing engine from native code~~ WIP: Add PlatformDispatcher.engineHandle Feb 17, 2025

knopp force-pushed the engine_handle branch from 1217feb to 1432103 Compare February 18, 2025 09:11

github-actions bot added platform-ios iOS applications specifically team-ios Owned by iOS platform team platform-linux Building on or for Linux specifically platform-windows Building on or for Windows specifically labels Feb 18, 2025

knopp changed the title ~~WIP: Add PlatformDispatcher.engineHandle~~ Add PlatformDispatcher.engineHandle Feb 18, 2025

knopp force-pushed the engine_handle branch from dd56542 to 5703dac Compare February 18, 2025 13:32

knopp requested review from cbracken, chinmaygarde, loic-sharma and robert-ancell February 18, 2025 13:33

knopp force-pushed the engine_handle branch from 5703dac to 6168aef Compare February 18, 2025 14:25

knopp requested a review from jonahwilliams February 18, 2025 14:30

knopp force-pushed the engine_handle branch from 6168aef to db567cb Compare February 18, 2025 15:21

knopp mentioned this pull request Feb 18, 2025

Expose Flutter Engine address to be used with FFI #163430

Closed

knopp force-pushed the engine_handle branch from db567cb to c15a082 Compare February 18, 2025 16:48

matanlurey reviewed Feb 18, 2025

View reviewed changes

knopp force-pushed the engine_handle branch from 4f7f6df to 7d86a9e Compare February 18, 2025 21:15

robert-ancell requested changes Feb 18, 2025

View reviewed changes

knopp added 9 commits February 28, 2025 21:54

Rename engineForId: to engineForIdentifier:

6d87fba

Make accessing instance from Id private for the time being

f61fce3

Rename

def4bbf

Add missing @OverRide annotation

6600298

Use SAFE_ACCESS when getting engineId. Move before check.

5f7906f

Reformat because of trailing comma.

c8fba15

Use bridged cast to raise eyebrows

f330d9c

Add shell test for Null engineId

976d513

safe access

8dc6f1a

knopp force-pushed the engine_handle branch from 643a603 to 8dc6f1a Compare February 28, 2025 20:54

knopp added this pull request to the merge queue Feb 28, 2025

Merged via the queue into flutter:master with commit b831b26 Feb 28, 2025
176 checks passed

knopp deleted the engine_handle branch February 28, 2025 22:48

engine-flutter-autoroll mentioned this pull request Mar 1, 2025

Roll Flutter from 2e570ca393c8 to 0c055f2c508a (55 revisions) flutter/packages#8767

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 1, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

2dc5eac

engine-flutter-autoroll mentioned this pull request Mar 1, 2025

Roll Flutter from 2e570ca393c8 to 0c055f2c508a (55 revisions) flutter/packages#8768

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 1, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

146c950

engine-flutter-autoroll mentioned this pull request Mar 2, 2025

Roll Flutter from 2e570ca393c8 to 619d4248114d (58 revisions) flutter/packages#8769

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 2, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

71abbc9

engine-flutter-autoroll mentioned this pull request Mar 2, 2025

Roll Flutter from 2e570ca393c8 to 842db35d27b8 (59 revisions) flutter/packages#8770

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 2, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

9c5b4a3

engine-flutter-autoroll mentioned this pull request Mar 2, 2025

Roll Flutter from 2e570ca393c8 to 842db35d27b8 (59 revisions) flutter/packages#8771

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 2, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

d82f04f

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

7580335

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

dc09497

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

b831b26 Add PlatformDispatcher.engineId (flutter/flutter#163476)

5a8996b

Add PlatformDispatcher.engineId #163476

Add PlatformDispatcher.engineId #163476

Uh oh!

Conversation

knopp commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

matanlurey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robert-ancell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knopp Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knopp Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knopp commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

knopp commented Feb 17, 2025 •

edited

Loading

knopp Feb 18, 2025 •

edited

Loading

loic-sharma Feb 19, 2025 •

edited

Loading

knopp Feb 19, 2025 •

edited

Loading

knopp commented Feb 18, 2025 •

edited

Loading