Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@cbracken
Copy link
Member

@cbracken cbracken commented May 5, 2022

Previously, using Platform.executable (from dart:io) returned null (if
non-null-by-default was disabled) or threw an exception (if NNBD was
enabled) since we weren't setting it.

We now pass the executable name to Dart during VM startup based on the
first value in the FlutterProjectArgs::command_line_argv array passed to
FlutterEngineRun (or FlutterEngineInitialize) on startup. argv[0] (if
specified) is explicitly documented as being required to be the
executable name in embedder.h. In the case where no argv[0] is
specified, we instead set Platform.executable to "Flutter" in order to
avoid violating the (non-nullable) type annotation on
Platform.executable.

Note that dart::bin::SetExecutableName() does NOT make a copy of the
input string, so that value needs to be available for the entire lifetime
of the VM.

This also adds EmbedderConfigBuilder::SetExecutableName() to support
setting a fake executable name in unittests. By default, we continue to
set the name "embedder_unittest" unless overridden using this method.

See: https://api.flutter.dev/flutter/dart-io/Platform/executable.html
See: dart-lang/sdk#48427

Issue: flutter/flutter#83921

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.

@flutter-dashboard flutter-dashboard bot added the embedder Related to the embedder API label May 5, 2022
@cbracken cbracken requested a review from loic-sharma May 5, 2022 03:15
@cbracken
Copy link
Member Author

cbracken commented May 5, 2022

Note that while this does fix the crash currently triggered by calling Platform.executable, it doesn't guarantee that each of the embedders is doing the right thing, and in fact macOS is currently passing a placeholder value. I have a separate followup patch that corrects the macOS behaviour.

For the time being, the iOS, Android, and Fuchsia embedders will return "Flutter". This can be improved in one or more followup patches.

@cbracken cbracken changed the title [embedder] Set Platform.executable on startup Set Platform.executable on startup May 5, 2022
Previously, using Platform.executable (from dart:io) returned null (if
non-null-by-default was disabled) or threw an exception (if NNBD was
enabled) since we weren't setting it.

We now pass the executable name to Dart during VM startup based on the
first value in the FlutterProjectArgs::command_line_argv array passed to
FlutterEngineRun (or FlutterEngineInitialize) on startup. argv[0] (if
specified) is explicitly documented as being required to be the
executable name in embedder.h. In the case where no argv[0] is
specified, we instead set Platform.executable to "Flutter" in order to
avoid violating the (non-nullable) type annoation on
Platform.executable.

Note that dart::bin::SetExecutableName() does NOT make a copy of the
input string, so it needs to be available for the entire lifetime of the
VM.

This also adds EmbedderConfigBuilder::SetExecutableName() to support
setting a fake executable name in unittests. By default, we continue to
set the name "embedder_unittest" unless overridden using this method.

See: https://api.flutter.dev/flutter/dart-io/Platform/executable.html

Issue: flutter/flutter#83921
@cbracken cbracken added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 5, 2022
@cbracken cbracken merged commit ea5839b into flutter:main May 5, 2022
@cbracken cbracken deleted the platform-executable-embedder branch May 5, 2022 18:14
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

embedder Related to the embedder API waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants