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

Android embedding refactor pr40 add static engine cache #10481

Merged
matthew-carroll merged 6 commits into from
Aug 6, 2019
Merged

Android embedding refactor pr40 add static engine cache #10481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2fbacc6
WIP
matthew-carroll Jul 31, 2019
a1e82ed
Android Embedding Refactor PR 40: Introduce static cache for FlutterE…
matthew-carroll Aug 2, 2019
829ed51
Remove superfluous getLifecycle() method in FlutterFragment.
matthew-carroll Aug 2, 2019
0e5e3a7
License updates.
matthew-carroll Aug 3, 2019
9f5affc
Formatted BUILD.gn
matthew-carroll Aug 6, 2019
8aa17fb
PR Updates.
matthew-carroll Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ deps = {
'packages': [
{
'package': 'flutter/android/robolectric_bundle',
'version': 'last_updated:2019-07-29T15:27:42-0700'
'version': 'last_updated:2019-08-02T16:01:27-0700'
}
],
'condition': 'download_android_deps',
Expand Down
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/Splas
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/SplashScreenProvider.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEngineAndroidLifecycle.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEngineCache.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java
Expand Down
5 changes: 5 additions & 0 deletions shell/platform/android/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ action("flutter_shell_java") {
"io/flutter/embedding/android/SplashScreenProvider.java",
"io/flutter/embedding/engine/FlutterEngine.java",
"io/flutter/embedding/engine/FlutterEngineAndroidLifecycle.java",
"io/flutter/embedding/engine/FlutterEngineCache.java",
"io/flutter/embedding/engine/FlutterEnginePluginRegistry.java",
"io/flutter/embedding/engine/FlutterJNI.java",
"io/flutter/embedding/engine/FlutterShellArgs.java",
Expand Down Expand Up @@ -330,6 +331,9 @@ action("robolectric_tests") {
"test/io/flutter/FlutterTestSuite.java",
"test/io/flutter/SmokeTest.java",
"test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java",
"test/io/flutter/embedding/android/FlutterActivityTest.java",
"test/io/flutter/embedding/android/FlutterFragmentTest.java",
"test/io/flutter/embedding/engine/FlutterEngineCacheTest.java",
"test/io/flutter/util/PreconditionsTest.java",
]

Expand All @@ -351,6 +355,7 @@ action("robolectric_tests") {
"//third_party/robolectric/lib/common-1.1.1.jar",
"//third_party/robolectric/lib/common-java8-1.1.1.jar",
"//third_party/robolectric/lib/support-annotations-28.0.0.jar",
"//third_party/robolectric/lib/support-fragment-25.2.0.jar",
"//third_party/robolectric/lib/mockito-all-1.10.19.jar",
]

Expand Down
194 changes: 165 additions & 29 deletions shell/platform/android/io/flutter/embedding/android/FlutterActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@
* route may be specified explicitly by passing the name of the route as a {@code String} in
* {@link #EXTRA_INITIAL_ROUTE}, e.g., "my/deep/link".
* <p>
* The Dart entrypoint and initial route can each be controlled using a {@link IntentBuilder}
* The Dart entrypoint and initial route can each be controlled using a {@link NewEngineIntentBuilder}
* via the following methods:
* <ul>
* <li>{@link IntentBuilder#dartEntrypoint}</li>
* <li>{@link IntentBuilder#initialRoute}</li>
* <li>{@link NewEngineIntentBuilder#dartEntrypoint}</li>
* <li>{@link NewEngineIntentBuilder#initialRoute}</li>
* </ul>
* <p>
* The app bundle path, Dart entrypoint, and initial route can also be controlled in a subclass of
Expand All @@ -61,6 +61,37 @@
* <li>{@link #getDartEntrypointFunctionName()}</li>
* <li>{@link #getInitialRoute()}</li>
* </ul>
* <p>
* {@code FlutterActivity} can be used with a cached {@link FlutterEngine} instead of creating a new
* one. Use {@link #withCachedEngine(String)} to build a {@code FlutterActivity} {@code Intent} that
Copy link
Contributor

@mehmetf mehmetf Aug 3, 2019

Choose a reason for hiding this comment

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

Should this be accompanied with some documentation change on when to use a cached engine?

Copy link
Contributor Author

@matthew-carroll matthew-carroll Aug 5, 2019

Choose a reason for hiding this comment

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

Is there some existing documentation that you think should be updated?

Copy link
Contributor

@mehmetf mehmetf Aug 5, 2019

Choose a reason for hiding this comment

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

I am not sure where the upstream user documentation for the new embedding is. Internally, it would be as part of the FAQ.

FlutterActivity can be used with a cached FlutterEngine.

but why and when should I use it with a cached Flutter Engine?

Copy link
Contributor Author

@matthew-carroll matthew-carroll Aug 6, 2019

Choose a reason for hiding this comment

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

@mehmetf @xster the only 2 times that I can think of that you wouldn't want to warm up an engine is:

  1. FlutterActivity is the first Activity that is launched, therefore rendering warm up redundant, or

  2. You have no idea when/if you'll need a Flutter experience and you don't want to waste the resources.

Can either of you think of other cases?

Copy link
Member

@xster xster Aug 6, 2019

Choose a reason for hiding this comment

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

I think that's right. We can probably phrase the doc to say using a cached engine is generally recommended for optimal launch latency except where impossible such as in the cases listed.

Copy link
Contributor Author

@matthew-carroll matthew-carroll Aug 6, 2019

Choose a reason for hiding this comment

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

Updated docs to reflect this.

* is configured to use an existing, cached {@link FlutterEngine}. {@link FlutterEngineCache} is the
* cache that is used to obtain a given cached {@link FlutterEngine}. An
* {@code IllegalStateException} will be thrown if a cached engine is requested but does not exist
* in the cache.
* <p>
* It is generally recommended to use a cached {@link FlutterEngine} to avoid a momentary delay
* when initializing a new {@link FlutterEngine}. The two exceptions to using a cached
* {@link FlutterEngine} are:
* <p>
* <ul>
* <li>When {@code FlutterActivity} is the first {@code Activity} displayed by the app, because
* pre-warming a {@link FlutterEngine} would have no impact in this situation.</li>
* <li>When you are unsure when/if you will need to display a Flutter experience.</li>
* </ul>
* <p>
* The following illustrates how to pre-warm and cache a {@link FlutterEngine}:
* <p>
* {@code
* // Create and pre-warm a FlutterEngine.
* FlutterEngine flutterEngine = new FlutterEngine(context);
* flutterEngine
* .getDartExecutor()
* .executeDartEntrypoint(DartEntrypoint.createDefault());
*
* // Cache the pre-warmed FlutterEngine in the FlutterEngineCache.
* FlutterEngineCache.getInstance().put("my_engine", flutterEngine);
* }
* <p>
* If Flutter is needed in a location that cannot use an {@code Activity}, consider using
* a {@link FlutterFragment}. Using a {@link FlutterFragment} requires forwarding some calls from
* an {@code Activity} to the {@link FlutterFragment}.
Expand Down Expand Up @@ -149,6 +180,8 @@ public class FlutterActivity extends Activity
protected static final String EXTRA_DART_ENTRYPOINT = "dart_entrypoint";
protected static final String EXTRA_INITIAL_ROUTE = "initial_route";
protected static final String EXTRA_BACKGROUND_MODE = "background_mode";
protected static final String EXTRA_CACHED_ENGINE_ID = "cached_engine_id";
protected static final String EXTRA_DESTROY_ENGINE_WITH_ACTIVITY = "destroy_engine_with_activity";

// Default configuration.
protected static final String DEFAULT_DART_ENTRYPOINT = "main";
Expand All @@ -161,50 +194,51 @@ public class FlutterActivity extends Activity
*/
@NonNull
public static Intent createDefaultIntent(@NonNull Context launchContext) {
return createBuilder().build(launchContext);
return withNewEngine().build(launchContext);
}

/**
* Creates an {@link IntentBuilder}, which can be used to configure an {@link Intent} to
* launch a {@code FlutterActivity}.
* Creates an {@link NewEngineIntentBuilder}, which can be used to configure an {@link Intent} to
* launch a {@code FlutterActivity} that internally creates a new {@link FlutterEngine} using
Copy link
Member

@xster xster Aug 6, 2019

Choose a reason for hiding this comment

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

Thanks for revisiting all these docs and clarifying.

* the desired Dart entrypoint, initial route, etc.
*/
@NonNull
public static IntentBuilder createBuilder() {
return new IntentBuilder(FlutterActivity.class);
public static NewEngineIntentBuilder withNewEngine() {
return new NewEngineIntentBuilder(FlutterActivity.class);
}

/**
* Builder to create an {@code Intent} that launches a {@code FlutterActivity} with the
* desired configuration.
* Builder to create an {@code Intent} that launches a {@code FlutterActivity} with a new
* {@link FlutterEngine} and the desired configuration.
*/
public static class IntentBuilder {
public static class NewEngineIntentBuilder {
private final Class<? extends FlutterActivity> activityClass;
private String dartEntrypoint = DEFAULT_DART_ENTRYPOINT;
private String initialRoute = DEFAULT_INITIAL_ROUTE;
private String backgroundMode = DEFAULT_BACKGROUND_MODE;

/**
* Constructor that allows this {@code IntentBuilder} to be used by subclasses of
* Constructor that allows this {@code NewEngineIntentBuilder} to be used by subclasses of
* {@code FlutterActivity}.
* <p>
* Subclasses of {@code FlutterActivity} should provide their own static version of
* {@link #createBuilder()}, which returns an instance of {@code IntentBuilder}
* {@link #withNewEngine()}, which returns an instance of {@code NewEngineIntentBuilder}
* constructed with a {@code Class} reference to the {@code FlutterActivity} subclass,
* e.g.:
* <p>
* {@code
* return new IntentBuilder(MyFlutterActivity.class);
* return new NewEngineIntentBuilder(MyFlutterActivity.class);
* }
*/
protected IntentBuilder(@NonNull Class<? extends FlutterActivity> activityClass) {
protected NewEngineIntentBuilder(@NonNull Class<? extends FlutterActivity> activityClass) {
this.activityClass = activityClass;
}

/**
* The name of the initial Dart method to invoke, defaults to "main".
*/
@NonNull
public IntentBuilder dartEntrypoint(@NonNull String dartEntrypoint) {
public NewEngineIntentBuilder dartEntrypoint(@NonNull String dartEntrypoint) {
this.dartEntrypoint = dartEntrypoint;
return this;
}
Expand All @@ -214,7 +248,7 @@ public IntentBuilder dartEntrypoint(@NonNull String dartEntrypoint) {
* defaults to "/".
*/
@NonNull
public IntentBuilder initialRoute(@NonNull String initialRoute) {
public NewEngineIntentBuilder initialRoute(@NonNull String initialRoute) {
this.initialRoute = initialRoute;
return this;
}
Expand All @@ -236,7 +270,7 @@ public IntentBuilder initialRoute(@NonNull String initialRoute) {
* following property: {@code <item name="android:windowIsTranslucent">true</item>}.
*/
@NonNull
public IntentBuilder backgroundMode(@NonNull BackgroundMode backgroundMode) {
public NewEngineIntentBuilder backgroundMode(@NonNull BackgroundMode backgroundMode) {
this.backgroundMode = backgroundMode.name();
return this;
}
Expand All @@ -250,6 +284,93 @@ public Intent build(@NonNull Context context) {
return new Intent(context, activityClass)
.putExtra(EXTRA_DART_ENTRYPOINT, dartEntrypoint)
.putExtra(EXTRA_INITIAL_ROUTE, initialRoute)
.putExtra(EXTRA_BACKGROUND_MODE, backgroundMode)
.putExtra(EXTRA_DESTROY_ENGINE_WITH_ACTIVITY, true);
}
}

/**
* Creates a {@link CachedEngineIntentBuilder}, which can be used to configure an {@link Intent}
* to launch a {@code FlutterActivity} that internally uses an existing {@link FlutterEngine} that
* is cached in {@link FlutterEngineCache}.
*/
Copy link
Member

@xster xster Aug 6, 2019

Choose a reason for hiding this comment

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

Can we add a snippet showing how one caches a FlutterEngine in a FlutterEngineCache?

Copy link
Contributor Author

@matthew-carroll matthew-carroll Aug 6, 2019

Choose a reason for hiding this comment

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

Sure, done.

public static CachedEngineIntentBuilder withCachedEngine(@NonNull String cachedEngineId) {
return new CachedEngineIntentBuilder(FlutterActivity.class, cachedEngineId);
}

/**
* Builder to create an {@code Intent} that launches a {@code FlutterActivity} with an existing
* {@link FlutterEngine} that is cached in {@link FlutterEngineCache}.
*/
public static class CachedEngineIntentBuilder {
private final Class<? extends FlutterActivity> activityClass;
private final String cachedEngineId;
private boolean destroyEngineWithActivity = false;
private String backgroundMode = DEFAULT_BACKGROUND_MODE;

/**
* Constructor that allows this {@code CachedEngineIntentBuilder} to be used by subclasses of
* {@code FlutterActivity}.
* <p>
* Subclasses of {@code FlutterActivity} should provide their own static version of
* {@link #withNewEngine()}, which returns an instance of {@code CachedEngineIntentBuilder}
* constructed with a {@code Class} reference to the {@code FlutterActivity} subclass,
* e.g.:
* <p>
* {@code
* return new CachedEngineIntentBuilder(MyFlutterActivity.class, engineId);
* }
*/
protected CachedEngineIntentBuilder(
@NonNull Class<? extends FlutterActivity> activityClass,
@NonNull String engineId
) {
this.activityClass = activityClass;
this.cachedEngineId = engineId;
}

/**
* Returns true if the cached {@link FlutterEngine} should be destroyed and removed from the
* cache when this {@code FlutterActivity} is destroyed.
* <p>
* The default value is {@code false}.
*/
public CachedEngineIntentBuilder destroyEngineWithActivity(boolean destroyEngineWithActivity) {
this.destroyEngineWithActivity = destroyEngineWithActivity;
return this;
}

/**
* The mode of {@code FlutterActivity}'s background, either {@link BackgroundMode#opaque} or
* {@link BackgroundMode#transparent}.
* <p>
* The default background mode is {@link BackgroundMode#opaque}.
* <p>
* Choosing a background mode of {@link BackgroundMode#transparent} will configure the inner
* {@link FlutterView} of this {@code FlutterActivity} to be configured with a
* {@link FlutterTextureView} to support transparency. This choice has a non-trivial performance
* impact. A transparent background should only be used if it is necessary for the app design
* being implemented.
* <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>}.
*/
@NonNull
public CachedEngineIntentBuilder backgroundMode(@NonNull BackgroundMode backgroundMode) {
this.backgroundMode = backgroundMode.name();
return this;
}

/**
* Creates and returns an {@link Intent} that will launch a {@code FlutterActivity} with
* the desired configuration.
*/
@NonNull
public Intent build(@NonNull Context context) {
return new Intent(context, activityClass)
.putExtra(EXTRA_CACHED_ENGINE_ID, cachedEngineId)
.putExtra(EXTRA_DESTROY_ENGINE_WITH_ACTIVITY, destroyEngineWithActivity)
.putExtra(EXTRA_BACKGROUND_MODE, backgroundMode);
}
}
Expand Down Expand Up @@ -521,6 +642,31 @@ public FlutterShellArgs getFlutterShellArgs() {
return FlutterShellArgs.fromIntent(getIntent());
}

/**
* Returns the ID of a statically cached {@link FlutterEngine} to use within this
* {@code FlutterActivity}, or {@code null} if this {@code FlutterActivity} does not want to
* use a cached {@link FlutterEngine}.
*/
@Override
@Nullable
public String getCachedEngineId() {
return getIntent().getStringExtra(EXTRA_CACHED_ENGINE_ID);
Copy link
Member

@xster xster Aug 6, 2019

Choose a reason for hiding this comment

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

I forget how this works. If you made an activity with an Intent and rotate the screen, it's the same intent instance?

Copy link
Contributor Author

@matthew-carroll matthew-carroll Aug 6, 2019

Choose a reason for hiding this comment

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

I don't know about the Intent instance, but the Intent should report the same extras.

Copy link
Member

@xster xster Aug 6, 2019

Choose a reason for hiding this comment

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

SG

}

/**
* Returns false if the {@link FlutterEngine} backing this {@code FlutterActivity} should
* outlive this {@code FlutterActivity}, or true to be destroyed when the {@code FlutterActivity}
* is destroyed.
* <p>
* The default value is {@code true} in cases where {@code FlutterActivity} created its own
* {@link FlutterEngine}, and {@code false} in cases where a cached {@link FlutterEngine} was
* provided.
*/
@Override
public boolean shouldDestroyEngineWithHost() {
return getIntent().getBooleanExtra(EXTRA_DESTROY_ENGINE_WITH_ACTIVITY, false);
Copy link
Member

@xster xster Aug 6, 2019

Choose a reason for hiding this comment

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

Describe the default either in this doc or in the builder class or the builder factory.

Copy link
Contributor Author

@matthew-carroll matthew-carroll Aug 6, 2019

Choose a reason for hiding this comment

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

Done. I also updated FlutterActivity and FlutterFragment to set this value to true whenever the FlutterActivity/Fragment create its own FlutterEngine, or one is explicitly provided by subclassing. Do you think that's OK?

}

/**
* The Dart entrypoint that will be executed as soon as the Dart snapshot is loaded.
* <p>
Expand Down Expand Up @@ -617,7 +763,7 @@ public String getAppBundlePath() {

// Return the default app bundle path.
// TODO(mattcarroll): move app bundle resolution into an appropriately named class.
return FlutterMain.findAppBundlePath(getApplicationContext());
return FlutterMain.findAppBundlePath();
}

/**
Expand Down Expand Up @@ -752,16 +898,6 @@ public boolean shouldAttachEngineToActivity() {
return true;
}

/**
* Returns true if the {@link FlutterEngine} backing this {@code FlutterActivity} should
* outlive this {@code FlutterActivity}, or be destroyed when the {@code FlutterActivity}
* is destroyed.
*/
@Override
public boolean retainFlutterEngineAfterHostDestruction() {
return false;
}

@Override
public void onFirstFrameRendered() {}

Expand Down
Loading