reland: Started using FlutterEngineGroups by default on Android (#38367)

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

This reverts commit 13ae6eb.

* fixed the entrypoint test

* updated old test

* coldpalelight's suggestion

* added entrypoint args

Author: gaaclarke

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

@@ -149,7 +149,8 @@
  *
  * <pre>{@code
  * // Create and pre-warm a FlutterEngine.
- * FlutterEngine flutterEngine = new FlutterEngine(context);
+ * FlutterEngineGroup group = new FlutterEngineGroup(context);
+ * FlutterEngine flutterEngine = group.createAndRunDefaultEngine(context);
  * flutterEngine.getDartExecutor().executeDartEntrypoint(DartEntrypoint.createDefault());
  *
  * // Cache the pre-warmed FlutterEngine in the FlutterEngineCache.

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

@@ -90,6 +90,7 @@ public interface DelegateFactory {
   private boolean isFirstFrameRendered;
   private boolean isAttached;
   private Integer previousVisibility;
+  @Nullable private FlutterEngineGroup engineGroup;
 
   @NonNull
   private final FlutterUiDisplayListener flutterUiDisplayListener =
@@ -109,8 +110,13 @@ public void onFlutterUiNoLongerDisplayed() {
       };
 
   FlutterActivityAndFragmentDelegate(@NonNull Host host) {
+    this(host, null);
+  }
+
+  FlutterActivityAndFragmentDelegate(@NonNull Host host, @Nullable FlutterEngineGroup engineGroup) {
     this.host = host;
     this.isFirstFrameRendered = false;
+    this.engineGroup = engineGroup;
   }
 
   /**
@@ -220,6 +226,21 @@ void onAttach(@NonNull Context context) {
     return activity;
   }
 
+  private FlutterEngineGroup.Options addEntrypointOptions(FlutterEngineGroup.Options options) {
+    String appBundlePathOverride = host.getAppBundlePath();
+    if (appBundlePathOverride == null || appBundlePathOverride.isEmpty()) {
+      appBundlePathOverride = FlutterInjector.instance().flutterLoader().findAppBundlePath();
+    }
+
+    DartExecutor.DartEntrypoint dartEntrypoint =
+        new DartExecutor.DartEntrypoint(
+            appBundlePathOverride, host.getDartEntrypointFunctionName());
+    return options
+        .setDartEntrypoint(dartEntrypoint)
+        .setInitialRoute(host.getInitialRoute())
+        .setDartEntrypointArgs(host.getDartEntrypointArgs());
+  }
+
   /**
    * Obtains a reference to a FlutterEngine to back this delegate and its {@code host}.
    *
@@ -277,17 +298,9 @@ void onAttach(@NonNull Context context) {
                 + "'");
       }
 
-      String appBundlePathOverride = host.getAppBundlePath();
-      if (appBundlePathOverride == null || appBundlePathOverride.isEmpty()) {
-        appBundlePathOverride = FlutterInjector.instance().flutterLoader().findAppBundlePath();
-      }
-
-      DartExecutor.DartEntrypoint dartEntrypoint =
-          new DartExecutor.DartEntrypoint(
-              appBundlePathOverride, host.getDartEntrypointFunctionName());
       flutterEngine =
           flutterEngineGroup.createAndRunEngine(
-              host.getContext(), dartEntrypoint, host.getInitialRoute());
+              addEntrypointOptions(new FlutterEngineGroup.Options(host.getContext())));
       isFlutterEngineFromHost = false;
       return;
     }
@@ -298,12 +311,17 @@ void onAttach(@NonNull Context context) {
         TAG,
         "No preferred FlutterEngine was provided. Creating a new FlutterEngine for"
             + " this FlutterFragment.");
+
+    FlutterEngineGroup group =
+        engineGroup == null
+            ? new FlutterEngineGroup(host.getContext(), host.getFlutterShellArgs().toArray())
+            : engineGroup;
     flutterEngine =
-        new FlutterEngine(
-            host.getContext(),
-            host.getFlutterShellArgs().toArray(),
-            /*automaticallyRegisterPlugins=*/ false,
-            /*willProvideRestorationData=*/ host.shouldRestoreAndSaveState());
+        group.createAndRunEngine(
+            addEntrypointOptions(
+                new FlutterEngineGroup.Options(host.getContext())
+                    .setAutomaticallyRegisterPlugins(false)
+                    .setWaitForRestorationData(host.shouldRestoreAndSaveState())));
     isFlutterEngineFromHost = false;
   }
 

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

@@ -79,7 +79,8 @@
  *
  * <pre>{@code
  * // Create and pre-warm a FlutterEngine.
- * FlutterEngine flutterEngine = new FlutterEngine(context);
+ * FlutterEngineGroup group = new FlutterEngineGroup(context);
+ * FlutterEngine flutterEngine = group.createAndRunDefaultEngine(context);
  * flutterEngine
  *   .getDartExecutor()
  *   .executeDartEntrypoint(DartEntrypoint.createDefault());

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

@@ -9,6 +9,7 @@
 import android.content.res.AssetManager;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import io.flutter.FlutterInjector;
 import io.flutter.Log;
 import io.flutter.embedding.engine.dart.DartExecutor;
@@ -279,6 +280,27 @@ public FlutterEngine(
       @Nullable String[] dartVmArgs,
       boolean automaticallyRegisterPlugins,
       boolean waitForRestorationData) {
+    this(
+        context,
+        flutterLoader,
+        flutterJNI,
+        platformViewsController,
+        dartVmArgs,
+        automaticallyRegisterPlugins,
+        waitForRestorationData,
+        null);
+  }
+
+  @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
+  public FlutterEngine(
+      @NonNull Context context,
+      @Nullable FlutterLoader flutterLoader,
+      @NonNull FlutterJNI flutterJNI,
+      @NonNull PlatformViewsController platformViewsController,
+      @Nullable String[] dartVmArgs,
+      boolean automaticallyRegisterPlugins,
+      boolean waitForRestorationData,
+      @Nullable FlutterEngineGroup group) {
     AssetManager assetManager;
     try {
       assetManager = context.createPackageContext(context.getPackageName(), 0).getAssets();
@@ -347,7 +369,8 @@ public FlutterEngine(
     this.platformViewsController.onAttachedToJNI();
 
     this.pluginRegistry =
-        new FlutterEngineConnectionRegistry(context.getApplicationContext(), this, flutterLoader);
+        new FlutterEngineConnectionRegistry(
+            context.getApplicationContext(), this, flutterLoader, group);
 
     localizationPlugin.sendLocalesToFlutter(context.getResources().getConfiguration());
 

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

@@ -97,7 +97,8 @@
   FlutterEngineConnectionRegistry(
       @NonNull Context appContext,
       @NonNull FlutterEngine flutterEngine,
-      @NonNull FlutterLoader flutterLoader) {
+      @NonNull FlutterLoader flutterLoader,
+      @Nullable FlutterEngineGroup group) {
     this.flutterEngine = flutterEngine;
     pluginBinding =
         new FlutterPlugin.FlutterPluginBinding(
@@ -106,7 +107,8 @@
             flutterEngine.getDartExecutor(),
             flutterEngine.getRenderer(),
             flutterEngine.getPlatformViewsController().getRegistry(),
-            new DefaultFlutterAssets(flutterLoader));
+            new DefaultFlutterAssets(flutterLoader),
+            group);
   }
 
   public void destroy() {

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

@@ -210,7 +210,8 @@ public void onEngineWillDestroy() {
         platformViewsController, // PlatformViewsController.
         null, // String[]. The Dart VM has already started, this arguments will have no effect.
         automaticallyRegisterPlugins, // boolean.
-        waitForRestorationData); // boolean.
+        waitForRestorationData, // boolean.
+        this);
   }
 
   /** Options that control how a FlutterEngine should be created. */

shell/platform/android/io/flutter/embedding/engine/plugins/FlutterPlugin.java

@@ -6,8 +6,10 @@
 
 import android.content.Context;
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.lifecycle.Lifecycle;
 import io.flutter.embedding.engine.FlutterEngine;
+import io.flutter.embedding.engine.FlutterEngineGroup;
 import io.flutter.plugin.common.BinaryMessenger;
 import io.flutter.plugin.platform.PlatformViewRegistry;
 import io.flutter.view.TextureRegistry;
@@ -107,20 +109,23 @@ class FlutterPluginBinding {
     private final TextureRegistry textureRegistry;
     private final PlatformViewRegistry platformViewRegistry;
     private final FlutterAssets flutterAssets;
+    private final FlutterEngineGroup group;
 
     public FlutterPluginBinding(
         @NonNull Context applicationContext,
         @NonNull FlutterEngine flutterEngine,
         @NonNull BinaryMessenger binaryMessenger,
         @NonNull TextureRegistry textureRegistry,
         @NonNull PlatformViewRegistry platformViewRegistry,
-        @NonNull FlutterAssets flutterAssets) {
+        @NonNull FlutterAssets flutterAssets,
+        @Nullable FlutterEngineGroup group) {
       this.applicationContext = applicationContext;
       this.flutterEngine = flutterEngine;
       this.binaryMessenger = binaryMessenger;
       this.textureRegistry = textureRegistry;
       this.platformViewRegistry = platformViewRegistry;
       this.flutterAssets = flutterAssets;
+      this.group = group;
     }
 
     @NonNull
@@ -157,6 +162,21 @@ public PlatformViewRegistry getPlatformViewRegistry() {
     public FlutterAssets getFlutterAssets() {
       return flutterAssets;
     }
+
+    /**
+     * Accessor for the {@link FlutterEngineGroup} used to create the {@link FlutterEngine} for the
+     * app.
+     *
+     * <p>This is useful in the rare case that a plugin has to spawn its own engine (for example,
+     * running an engine the background). The result is nullable since old versions of Flutter and
+     * custom setups may not have used a {@link FlutterEngineGroup}. Failing to use this when it is
+     * available will result in suboptimal performance and odd behaviors related to Dart isolate
+     * groups.
+     */
+    @Nullable
+    public FlutterEngineGroup getEngineGroup() {
+      return group;
+    }
   }
 
   /** Provides Flutter plugins with access to Flutter asset information. */

shell/platform/android/io/flutter/embedding/engine/plugins/shim/ShimPluginRegistry.java

@@ -25,7 +25,8 @@
  *
  * <pre>
  * // Create the FlutterEngine that will back the Flutter UI.
- * FlutterEngine flutterEngine = new FlutterEngine(context);
+ * FlutterEngineGroup group = new FlutterEngineGroup(context);
+ * FlutterEngine flutterEngine = group.createAndRunDefaultEngine(context);
  *
  * // Create a ShimPluginRegistry and wrap the FlutterEngine with the shim.
  * ShimPluginRegistry shimPluginRegistry = new ShimPluginRegistry(flutterEngine, platformViewsController);

shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java

@@ -247,10 +247,14 @@ public void itUsesNewEngineInGroupWhenProvided() {
     FlutterEngineGroup flutterEngineGroup = mock(FlutterEngineGroup.class);
     FlutterEngineGroupCache.getInstance().put("my_flutter_engine_group", flutterEngineGroup);
 
+    List<String> entryPointArgs = new ArrayList<>();
+    entryPointArgs.add("entrypoint-arg");
+
     // Adjust fake host to request cached engine group.
     when(mockHost.getCachedEngineGroupId()).thenReturn("my_flutter_engine_group");
     when(mockHost.provideFlutterEngine(any(Context.class))).thenReturn(null);
     when(mockHost.shouldAttachEngineToActivity()).thenReturn(false);
+    when(mockHost.getDartEntrypointArgs()).thenReturn(entryPointArgs);
 
     // Create the real object that we're testing.
     FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(mockHost);
@@ -263,8 +267,15 @@ public void itUsesNewEngineInGroupWhenProvided() {
     // event.
     // Note: "/fake/path" and "main" come from `setUp()`.
     DartExecutor.DartEntrypoint entrypoint = new DartExecutor.DartEntrypoint("/fake/path", "main");
-    verify(flutterEngineGroup, times(1))
-        .createAndRunEngine(mockHost.getContext(), entrypoint, mockHost.getInitialRoute());
+    ArgumentCaptor<FlutterEngineGroup.Options> optionsCaptor =
+        ArgumentCaptor.forClass(FlutterEngineGroup.Options.class);
+    verify(flutterEngineGroup, times(1)).createAndRunEngine(optionsCaptor.capture());
+    assertEquals(mockHost.getContext(), optionsCaptor.getValue().getContext());
+    assertEquals(entrypoint, optionsCaptor.getValue().getDartEntrypoint());
+    assertEquals(mockHost.getInitialRoute(), optionsCaptor.getValue().getInitialRoute());
+    assertNotNull(optionsCaptor.getValue().getDartEntrypointArgs());
+    assertEquals(1, optionsCaptor.getValue().getDartEntrypointArgs().size());
+    assertEquals("entrypoint-arg", optionsCaptor.getValue().getDartEntrypointArgs().get(0));
   }
 
   @Test(expected = IllegalStateException.class)
@@ -1159,6 +1170,22 @@ public void itDoesNotDelayTheFirstDrawWhenRequestedAndWithAProvidedSplashScreen(
     assertNull(delegate.activePreDrawListener);
   }
 
+  @Test
+  public void usesFlutterEngineGroup() {
+    FlutterEngineGroup mockEngineGroup = mock(FlutterEngineGroup.class);
+    when(mockEngineGroup.createAndRunEngine(any(FlutterEngineGroup.Options.class)))
+        .thenReturn(mockFlutterEngine);
+    FlutterActivityAndFragmentDelegate.Host host =
+        mock(FlutterActivityAndFragmentDelegate.Host.class);
+    when(mockHost.getContext()).thenReturn(ctx);
+
+    FlutterActivityAndFragmentDelegate delegate =
+        new FlutterActivityAndFragmentDelegate(mockHost, mockEngineGroup);
+    delegate.onAttach(ctx);
+    FlutterEngine engineUnderTest = delegate.getFlutterEngine();
+    assertEquals(engineUnderTest, mockFlutterEngine);
+  }
+
   /**
    * Creates a mock {@link io.flutter.embedding.engine.FlutterEngine}.
    *

shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineConnectionRegistryTest.java

@@ -40,7 +40,7 @@ public void itDoesNotRegisterTheSamePluginTwice() {
     FakeFlutterPlugin fakePlugin2 = new FakeFlutterPlugin();
 
     FlutterEngineConnectionRegistry registry =
-        new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader);
+        new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader, null);
 
     // Verify that the registry doesn't think it contains our plugin yet.
     assertFalse(registry.has(fakePlugin1.getClass()));
@@ -86,7 +86,7 @@ public void activityResultListenerCanBeRemovedFromListener() {
 
     // Set up the environment to get the required internal data
     FlutterEngineConnectionRegistry registry =
-        new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader);
+        new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader, null);
     FakeActivityAwareFlutterPlugin fakePlugin = new FakeActivityAwareFlutterPlugin();
     registry.add(fakePlugin);
     registry.attachToActivity(appComponent, lifecycle);
@@ -129,7 +129,7 @@ public void softwareRendering() {
 
     // Test attachToActivity with an Activity that has no Intent.
     FlutterEngineConnectionRegistry registry =
-        new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader);
+        new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader, null);
     registry.attachToActivity(appComponent, mock(Lifecycle.class));
     verify(platformViewsController).setSoftwareRendering(false);
 

shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java

@@ -25,8 +25,11 @@
 import io.flutter.FlutterInjector;
 import io.flutter.embedding.engine.FlutterEngine;
 import io.flutter.embedding.engine.FlutterEngine.EngineLifecycleListener;
+import io.flutter.embedding.engine.FlutterEngineGroup;
 import io.flutter.embedding.engine.FlutterJNI;
 import io.flutter.embedding.engine.loader.FlutterLoader;
+import io.flutter.embedding.engine.plugins.FlutterPlugin;
+import io.flutter.embedding.engine.plugins.PluginRegistry;
 import io.flutter.plugin.platform.PlatformViewsController;
 import io.flutter.plugins.GeneratedPluginRegistrant;
 import java.util.List;
@@ -323,4 +326,34 @@ public void itComesWithARunningDartExecutorIfJNIIsAlreadyAttached() throws NameN
 
     assertTrue(engineUnderTest.getDartExecutor().isExecutingDart());
   }
+
+  @Test
+  public void passesEngineGroupToPlugins() throws NameNotFoundException {
+    Context packageContext = mock(Context.class);
+
+    when(mockContext.createPackageContext(any(), anyInt())).thenReturn(packageContext);
+    when(flutterJNI.isAttached()).thenReturn(true);
+
+    FlutterEngineGroup mockGroup = mock(FlutterEngineGroup.class);
+
+    FlutterEngine engineUnderTest =
+        new FlutterEngine(
+            mockContext,
+            mock(FlutterLoader.class),
+            flutterJNI,
+            new PlatformViewsController(),
+            /*dartVmArgs=*/ new String[] {},
+            /*automaticallyRegisterPlugins=*/ false,
+            /*waitForRestorationData=*/ false,
+            mockGroup);
+
+    PluginRegistry registry = engineUnderTest.getPlugins();
+    FlutterPlugin mockPlugin = mock(FlutterPlugin.class);
+    ArgumentCaptor<FlutterPlugin.FlutterPluginBinding> pluginBindingCaptor =
+        ArgumentCaptor.forClass(FlutterPlugin.FlutterPluginBinding.class);
+    registry.add(mockPlugin);
+    verify(mockPlugin).onAttachedToEngine(pluginBindingCaptor.capture());
+    assertNotNull(pluginBindingCaptor.getValue());
+    assertEquals(mockGroup, pluginBindingCaptor.getValue().getEngineGroup());
+  }
 }