[WIP]add the feature that can create a new engine with FlutterEngineGroup … #29293

kylinchen · 2021-10-22T06:59:21Z

add the feature that can create a new engine with FlutterEngineGroup for FlutterActivity/FlutterFragment.

FlutterActivity/FlutterFragment adds support for multiple engines. The user sets the FlutterEngineGroup ID, calls the withNewEngineInGroup method, and creates a FlutterEngine in this FlutterEngineGroup.

fix issue:
flutter/flutter#72742

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.

google-cla · 2021-10-22T06:59:25Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

kylinchen · 2021-10-22T07:01:03Z

@googlebot I signed it!

google-cla · 2021-10-22T07:30:22Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

…for FlutterActivity/FlutterFragment

chinmaygarde · 2021-11-11T21:16:46Z

cc @blasten Do you have the cycles to review this soon?

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

blasten · 2021-11-11T22:48:51Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+   * io.flutter.embedding.engine.FlutterEngineGroupCache}. and creates a new {@link
+   * io.flutter.embedding.engine.FlutterEngine} by FlutterEngineGroup#createAndRunEngine
+   *
+   * @param engineGroupId A cached engine group ID.


can we add a code snippet that demonstrates a correct use of this method?

blasten · 2021-11-11T22:52:09Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+     * Constructor that allows this {@code NewEngineInGroupIntentBuilder} to be used by subclasses
+     * of {@code FlutterActivity}.
+     *
+     * <p>Subclasses of {@code FlutterActivity} should provide their own static version of {@link


same comment: an example would be helpful

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

blasten · 2021-11-11T22:55:19Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+     *
+     * <p>A {@code FlutterActivity} that is configured with a background mode of {@link
+     * BackgroundMode#transparent} must have a theme applied to it that includes the following
+     * property: {@code <item name="android:windowIsTranslucent">true</item>}.


in app/src/main/res/values/styles.xml right?

yes, in app/src/main/res/values/styles.xml, we can define custom theme for activity, and set the background of activity.

blasten · 2021-11-11T23:04:30Z

shell/platform/android/io/flutter/embedding/android/FlutterFragment.java

+  /**
+   * Returns a {@link NewEngineInGroupFragmentBuilder} to create a {@code FlutterFragment} with a
+   * cached {@link io.flutter.embedding.engine.FlutterEngineGroup} in {@link
+   * io.flutter.embedding.engine.FlutterEngineGroupCache}. creates a new {@link


I think this sentence got cut off

blasten · 2021-11-11T23:06:36Z

shell/platform/android/io/flutter/embedding/android/FlutterFragment.java

+    private String initialRoute = "/";
+    private boolean handleDeeplinking = false;
+    private RenderMode renderMode = RenderMode.surface;
+    private TransparencyMode transparencyMode = TransparencyMode.transparent;


nit: NonNull annotation for the required fields

blasten · 2021-11-12T02:20:58Z

This is a great PR.

First pass review, I'd like to see maybe a bit more code snippets in the code about how to use these APIs.

blasten · 2021-11-12T02:22:16Z

shell/platform/android/io/flutter/embedding/android/FlutterFragment.java

+     * getInitialRoute} returns null.
+     */
+    @NonNull
+    public NewEngineInGroupFragmentBuilder handleDeeplinking(@NonNull Boolean handleDeeplinking) {


can this be boolean?

chinmaygarde · 2021-12-02T21:14:40Z

Is progress being made on this issue? Looks like @blasten has a few open suggestions.

gaaclarke · 2021-12-02T21:25:31Z

I closed out the iOS implementation saying that convenience functions can be implemented by users and incorporating them into the API doesn't scale.

edit: I said this after having read through the iOS PR. This Android one is a bit more involved so it might make sense having this if it's not just trivial convenience but removing boilerplate that will exist for most users. I haven't had a chance to read it closely to see if that's the case.

ColdPaleLight · 2021-12-03T03:57:48Z

Is progress being made on this issue? Looks like @blasten has a few open suggestions.

Yes, it will update soon.

xster · 2021-12-03T08:42:13Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+  /**
+   * Creates a {@link NewEngineInGroupIntentBuilder}, which can be used to configure an {@link
+   * Intent} to launch a {@code FlutterActivity} that internally uses an existing {@link
+   * io.flutter.embedding.engine.FlutterEngineGroup} that is cached in {@link


that is cached in a FlutterEngineGroup

yes, when we use this method. First, we should create a FlutterEngineGroup and cached in FlutterEngineGroupCache by flutterEngineGroupId. Then we can use FlutterActivity.withNewEngineInGroup(flutterEngineGroupId) to create intent and start FlutterActivity

xster · 2021-12-03T08:45:28Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+   * Creates a {@link NewEngineInGroupIntentBuilder}, which can be used to configure an {@link
+   * Intent} to launch a {@code FlutterActivity} that internally uses an existing {@link
+   * io.flutter.embedding.engine.FlutterEngineGroup} that is cached in {@link
+   * io.flutter.embedding.engine.FlutterEngineGroupCache}. and creates a new {@link


to launch a FlutterActivity by internally creating a FlutterEngine from an existing FlutterEngineGroup cached in a specified FlutterEngineGroupCache

xster · 2021-12-03T08:46:20Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+    }
+
+    /**
+     * The Dart entrypoint that will be executed as soon as the Dart snapshot is loaded, default to


The Dart entrypoint that will be executed in the newly created FlutterEngine as soon as the Dart snapshot is loaded

xster · 2021-12-03T08:48:32Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+     * @return The engine group intent builder.
+     */
+    @NonNull
+    public NewEngineInGroupIntentBuilder backgroundMode(@NonNull BackgroundMode backgroundMode) {


rather than copying all this stuff again, can't we have them both be in the same builder class? Just have 2 constructors. One with an engine id and one with a engine group id.

yes , we can have them both be in the same builder class. But this break Single Responsibility Principle.

xster · 2021-12-03T08:50:24Z

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

@@ -214,6 +216,10 @@ void onAttach(@NonNull Context context) {
   * <p>Second, the {@code host} is given an opportunity to provide a {@link
   * io.flutter.embedding.engine.FlutterEngine} via {@link Host#provideFlutterEngine(Context)}.
   *
+   * <p>Third, the {@code host} is given an FluttenEngineGroup Id, then used a cached {@link


this statement doesn't match the code below? The host is asked whether it would like to provide a cached FlutterEngineGroup id rather than being given one

yes, you are right. I will update

xster · 2021-12-03T08:51:49Z

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

+                + "'");
+      }
+
+      String appBundlePathOverride = host.getAppBundlePath();


this block seems unrelated to the proposed change and is a scope creep right?

because the Constructor of DartEntrypoint need 2 params. One is AppBundlePath, this block provide it from Host or FlutterLoader

xster · 2021-12-03T08:52:57Z

shell/platform/android/io/flutter/embedding/engine/FlutterEngineGroupCache.java

+ * <p>The ID of a given {@link io.flutter.embedding.engine.FlutterEngineGroup} can be whatever
+ * {@code String} is desired.
+ *
+ * <p>{@code FlutterEngineGroupCache} is useful for storing pre-warmed {@link


this sentence makes sense for FlutterEngine, less for FlutterEngineGroups. e.g. a FlutterEngineGroup can't exactly be "pre-warmed"

yes, you are right. I will update

…terEngineGroup add a code snippet for the feature that create a new engine with FlutterEngineGroup

ColdPaleLight · 2021-12-13T06:33:35Z

Friendly ping @blasten @xster

xster

sorry for the wait. I was away last month. Almost there.

xster · 2022-01-11T20:46:08Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+   * Intent} to launch a {@code FlutterActivity} by internally creating a FlutterEngine from an
+   * existing {@link io.flutter.embedding.engine.FlutterEngineGroup} cached in a specified {@link
+   * io.flutter.embedding.engine.FlutterEngineGroupCache}, and creates a new {@link
+   * io.flutter.embedding.engine.FlutterEngine} by FlutterEngineGroup#createAndRunEngine


we can remove the last sentence

xster · 2022-01-11T20:46:30Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+   * io.flutter.embedding.engine.FlutterEngine} by FlutterEngineGroup#createAndRunEngine
+   *
+   * <pre>{@code
+   * // Create a FlutterEngineGroup, usually we could create it in onCreate method of Application.


Create a FlutterEngineGroup, such as in the onCreate method of the Application

xster · 2022-01-11T20:47:22Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+   * FlutterEngineGroup engineGroup = new FlutterEngineGroup(this);
+   * FlutterEngineGroupCache.getInstance().put("my_cached_engine_group_id", engineGroup);
+   *
+   * // use the intent that build by withNewEngineInGroup to start FlutterActivity


Start a FlutterActivity with the FlutterEngineGroup by creating an intent with withNewEngineInGroup

xster · 2022-01-11T20:48:11Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+
+  /**
+   * Builder to create an {@code Intent} that launches a {@code FlutterActivity} with a new {@link
+   * FlutterEngine} by FlutterEngineGroup#createAndRunEngine.


with a new {@link FlutterEngine} created by FlutterEngineGroup#createAndRunEngine.

xster · 2022-01-11T20:48:40Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+     * cacheedEngineGroupId); }
+     *
+     * <pre>{@code
+     * // Create a FlutterEngineGroup, usually we could create it in onCreate method of Application.


Create a FlutterEngineGroup, such as in the onCreate method of the Application

xster · 2022-01-11T20:55:08Z

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java

+     * FlutterEngineGroup engineGroup = new FlutterEngineGroup(this);
+     * FlutterEngineGroupCache.getInstance().put("my_cached_engine_group_id", engineGroup);
+     *
+     * // create NewEngineInGroupIntentBuilder, and start my custom FlutterActivity with the intent


Create a NewEngineInGroupIntentBuilder that would build an intent to start my custom FlutterActivity subclass.

xster · 2022-01-11T20:59:18Z

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

@@ -213,6 +215,10 @@ void onAttach(@NonNull Context context) {
   * <p>Second, the {@code host} is given an opportunity to provide a {@link
   * io.flutter.embedding.engine.FlutterEngine} via {@link Host#provideFlutterEngine(Context)}.
   *
+   * <p>Third, the {@code host} is given an FluttenEngineGroup Id, then used a cached {@link


Suggested change

* <p>Third, the {@code host} is given an FluttenEngineGroup Id, then used a cached {@link

* <p>Third, the {@code host} is asked if it would like to use a cached

* {@link io.flutter.embedding.engine.FlutterEngineGroup}. If so, the cached

* {@link io.flutter.embedding.engine.FlutterEngineGroup} is used to create a new

* {@link io.flutter.embedding.engine.FlutterEngine}.

xster · 2022-01-11T20:59:42Z

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

@@ -241,6 +247,34 @@ void onAttach(@NonNull Context context) {
      return;
    }

+    // Third, check if the host wants to use a cached FlutterEngineGroup
+    // and create new FlutterEngine by FlutterEngineGroup#createAndRunEngine


Suggested change

// and create new FlutterEngine by FlutterEngineGroup#createAndRunEngine

// and create new FlutterEngine using FlutterEngineGroup#createAndRunEngine

xster · 2022-01-11T21:00:41Z

shell/platform/android/io/flutter/embedding/android/FlutterFragment.java

@@ -702,6 +705,263 @@ protected Bundle createArgs() {
    }
  }

+  /**


same comments on the doc for from FlutterActivity on the next 2 classes

xster · 2022-01-11T21:01:24Z

shell/platform/android/io/flutter/embedding/engine/FlutterEngineGroup.java

@@ -178,7 +178,7 @@ public void onEngineWillDestroy() {
  }

  @VisibleForTesting
-  /* package */ FlutterEngine createEngine(Context context) {
+  public FlutterEngine createEngine(Context context) {


you don't need to do this right?

chinmaygarde · 2022-01-27T21:08:52Z

@kylinchen Any updates after @xster s last review?

chinmaygarde · 2022-02-17T21:06:15Z

Closing as this seems stale. Please re-open once progress can be made. Thanks.

ColdPaleLight · 2022-05-16T02:30:30Z

Talked to @kylinchen offline and he will restart work on this PR recently. So I reopened the PR and marked it as "WIP"

chinmaygarde · 2022-07-21T20:24:55Z

Are any updates on this likely?

ColdPaleLight · 2022-07-27T12:54:46Z

I'll take over this PR and close it for now. I will reopen it in the future.

flutter-dashboard bot added the platform-android label Oct 22, 2021

google-cla bot added the cla: no label Oct 22, 2021

google-cla bot added cla: yes and removed cla: no labels Oct 22, 2021

google-cla bot added cla: no and removed cla: yes labels Oct 22, 2021

add the feature that can create a new engine with FlutterEngineGroup …

ea6fe7e

…for FlutterActivity/FlutterFragment

kylinchen force-pushed the activity_with_engine_group branch from 76af5ed to ea6fe7e Compare October 22, 2021 07:35

google-cla bot added cla: yes and removed cla: no labels Oct 22, 2021

luckysmg mentioned this pull request Oct 22, 2021

Add engine group constructor in FlutterViewConrtroller #29181

Closed

8 tasks

xster requested a review from blasten October 26, 2021 22:01

blasten reviewed Nov 11, 2021

View reviewed changes

shell/platform/android/io/flutter/embedding/android/FlutterActivity.java Outdated Show resolved Hide resolved

blasten reviewed Nov 12, 2021

View reviewed changes

CaseyHillers changed the base branch from master to main November 15, 2021 18:13

fluttergithubbot force-pushed the master branch from b1a8771 to 2962099 Compare November 15, 2021 18:30

luckysmg mentioned this pull request Nov 19, 2021

Add engine group constructor in FlutterViewConrtroller #29836

Closed

8 tasks

godofredoc changed the base branch from master to main November 24, 2021 07:13

xster reviewed Dec 3, 2021

View reviewed changes

add a code snippet for the feature that create a new engine with Flut…

eb2d4d8

…terEngineGroup add a code snippet for the feature that create a new engine with FlutterEngineGroup

kylinchen force-pushed the activity_with_engine_group branch from b9b3ace to eb2d4d8 Compare December 6, 2021 12:44

kylinchen added 5 commits December 7, 2021 15:42

update comments

b61668d

Merge branch 'flutter:main' into activity_with_engine_group

c067104

update comments

af1418f

format code

87dcfce

Merge branch 'flutter:main' into activity_with_engine_group

eab1e7c

kylinchen requested review from xster and blasten December 8, 2021 05:54

ColdPaleLight added the customer: alibaba label Jan 6, 2022

xster reviewed Jan 11, 2022

View reviewed changes

chinmaygarde closed this Feb 17, 2022

ColdPaleLight added the Work in progress (WIP) Not ready (yet) for review! label May 16, 2022

ColdPaleLight changed the title ~~add the feature that can create a new engine with FlutterEngineGroup …~~ [WIP]add the feature that can create a new engine with FlutterEngineGroup … May 16, 2022

ColdPaleLight reopened this May 16, 2022

ColdPaleLight closed this Jul 27, 2022

ColdPaleLight removed the customer: alibaba label Jul 28, 2022

Nayuta403 mentioned this pull request Oct 24, 2022

Create FlutterActivity/FlutterFragment using light weight engine with FlutterEngineGroup #36963

Merged

8 tasks

-   * <p>Third, the {@code host} is given an FluttenEngineGroup Id, then used a cached {@link
+   * <p>Third, the {@code host} is asked if it would like to use a cached
+   * {@link io.flutter.embedding.engine.FlutterEngineGroup}. If so, the cached
+   * {@link io.flutter.embedding.engine.FlutterEngineGroup} is used to create a new
+   * {@link io.flutter.embedding.engine.FlutterEngine}.

	// and create new FlutterEngine by FlutterEngineGroup#createAndRunEngine
	// and create new FlutterEngine using FlutterEngineGroup#createAndRunEngine

@@ @@ -702,6 +705,263 @@ protected Bundle createArgs() { @@
                   }
                 }
+                /**

[WIP]add the feature that can create a new engine with FlutterEngineGroup … #29293

[WIP]add the feature that can create a new engine with FlutterEngineGroup … #29293

Conversation

kylinchen commented Oct 22, 2021 • edited by ColdPaleLight Loading

Pre-launch Checklist

google-cla bot commented Oct 22, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

kylinchen commented Oct 22, 2021

google-cla bot commented Oct 22, 2021

chinmaygarde commented Nov 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blasten commented Nov 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinmaygarde commented Dec 2, 2021

gaaclarke commented Dec 2, 2021 • edited Loading

ColdPaleLight commented Dec 3, 2021

Choose a reason for hiding this comment

kylinchen Dec 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ColdPaleLight commented Dec 13, 2021

xster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinmaygarde commented Jan 27, 2022

chinmaygarde commented Feb 17, 2022

ColdPaleLight commented May 16, 2022

chinmaygarde commented Jul 21, 2022

ColdPaleLight commented Jul 27, 2022

kylinchen commented Oct 22, 2021 •

edited by ColdPaleLight

Loading

gaaclarke commented Dec 2, 2021 •

edited

Loading

kylinchen Dec 6, 2021 •

edited

Loading