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

reland: Started using FlutterEngineGroups by default on Android #38367

Conversation

gaaclarke
Copy link
Member

relands: #37822
revert pr: #38351

The fix is in a separate commit in the PR. I just factored out some logic for creating engines from groups from elsewhere that takes into account the entrypoint. It's a bit tricky since it isn't clear how the engine was getting those things.

There already is a test that covers this.

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

This comment was marked as outdated.

@flutter-dashboard
Copy link

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 gaaclarke force-pushed the revert-13ae6eb75ab97947b2bcd507bf45ee33a1d2033d branch from 8a7bf16 to 3e5e7a4 Compare December 17, 2022 01:19
@gaaclarke gaaclarke marked this pull request as ready for review December 17, 2022 01:24
@gaaclarke gaaclarke changed the title Revert 13ae6eb75ab97947b2bcd507bf45ee33a1d2033d reland: Started using FlutterEngineGroups by default on Android Dec 17, 2022
Copy link
Contributor

@Nayuta403 Nayuta403 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

Copy link
Member

@ColdPaleLight ColdPaleLight left a comment

Choose a reason for hiding this comment

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

LGTM!

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 19, 2022
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 19, 2022
@auto-submit
Copy link
Contributor

auto-submit bot commented Dec 19, 2022

auto label is removed for flutter/engine, pr: 38367, Failed to merge pr#: 38367 with Pull request could not be merged: Resource not accessible by integration.

@chinmaygarde
Copy link
Member

@gaaclarke Did you or someone else re-run the check that tripped on "Resource not accessible by integration". I'm chasing down the flake and just want to make sure it didn't automagically resolve itself.

@gaaclarke
Copy link
Member Author

@gaaclarke Did you or someone else re-run the check that tripped on "Resource not accessible by integration". I'm chasing down the flake and just want to make sure it didn't automagically resolve itself.

@chinmaygarde wasn't me. I didn't even see it. I pushed up a handful of commits back-to-back, so maybe you saw it in between the pushes?

@gaaclarke
Copy link
Member Author

Oh wait, i see the message about autosubmit failing. Weird, I don't know what happened.

@gaaclarke gaaclarke merged commit 725d47d into flutter:main Dec 19, 2022
@chinmaygarde
Copy link
Member

Okie dokie. Pushing the commits was probably what caused the re-runs. Anyway, I have the reference.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 19, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 19, 2022
…117336)

* 290f3efc2 Add more missing includes (flutter/engine#38398)

* 24a6a9183 Remove usage of SkToBool (flutter/engine#38401)

* 725d47da2 reland: Started using FlutterEngineGroups by default on Android (flutter/engine#38367)

* c7cc1b6f7 [Impeller Scene] Add animation/PBR descriptions to ipscene (flutter/engine#38397)

* cba3a3990 Roll Skia from 46af4ad25426 to 8876daf17554 (3 revisions) (flutter/engine#38404)
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Jan 3, 2023
…ter#38367)

* Revert "Revert "Started using FlutterEngineGroups by default on Android  (flutter#37822)" (flutter#38351)"

This reverts commit 13ae6eb.

* fixed the entrypoint test

* updated old test

* coldpalelight's suggestion

* added entrypoint args
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…lutter#117336)

* 290f3efc2 Add more missing includes (flutter/engine#38398)

* 24a6a9183 Remove usage of SkToBool (flutter/engine#38401)

* 725d47da2 reland: Started using FlutterEngineGroups by default on Android (flutter/engine#38367)

* c7cc1b6f7 [Impeller Scene] Add animation/PBR descriptions to ipscene (flutter/engine#38397)

* cba3a3990 Roll Skia from 46af4ad25426 to 8876daf17554 (3 revisions) (flutter/engine#38404)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants