Deduplicate plugin registration logic and make error logs visible - take 2

shell/platform/android/io/flutter/FlutterInjector.java

@@ -7,6 +7,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
+import io.flutter.embedding.engine.FlutterJNI;
 import io.flutter.embedding.engine.deferredcomponents.DeferredComponentManager;
 import io.flutter.embedding.engine.loader.FlutterLoader;
 
@@ -65,13 +66,17 @@ public static void reset() {
   }
 
   private FlutterInjector(
-      @NonNull FlutterLoader flutterLoader, DeferredComponentManager deferredComponentManager) {
+      @NonNull FlutterLoader flutterLoader,
+      @Nullable DeferredComponentManager deferredComponentManager,
+      @NonNull FlutterJNI.Factory flutterJniFactory) {
     this.flutterLoader = flutterLoader;
     this.deferredComponentManager = deferredComponentManager;
+    this.flutterJniFactory = flutterJniFactory;
   }
 
   private FlutterLoader flutterLoader;
   private DeferredComponentManager deferredComponentManager;
+  private FlutterJNI.Factory flutterJniFactory;
 
   /** Returns the {@link FlutterLoader} instance to use for the Flutter Android engine embedding. */
   @NonNull
@@ -88,6 +93,11 @@ public DeferredComponentManager deferredComponentManager() {
     return deferredComponentManager;
   }
 
+  @NonNull
+  public FlutterJNI.Factory getFlutterJNIFactory() {
+    return flutterJniFactory;
+  }
+
   /**
    * Builder used to supply a custom FlutterInjector instance to {@link
    * FlutterInjector#setInstance(FlutterInjector)}.
@@ -97,6 +107,7 @@ public DeferredComponentManager deferredComponentManager() {
   public static final class Builder {
     private FlutterLoader flutterLoader;
     private DeferredComponentManager deferredComponentManager;
+    private FlutterJNI.Factory flutterJniFactory;
     /**
      * Sets a {@link FlutterLoader} override.
      *
@@ -113,9 +124,18 @@ public Builder setDeferredComponentManager(
       return this;
     }
 
+    public Builder setFlutterJNIFactory(@NonNull FlutterJNI.Factory factory) {
+      this.flutterJniFactory = factory;
+      return this;
+    }
+
     private void fillDefaults() {
+      if (flutterJniFactory == null) {
+        flutterJniFactory = new FlutterJNI.Factory();
+      }
+
       if (flutterLoader == null) {
-        flutterLoader = new FlutterLoader();
+        flutterLoader = new FlutterLoader(flutterJniFactory.provideFlutterJNI());
       }
       // DeferredComponentManager's intended default is null.
     }
@@ -127,7 +147,7 @@ private void fillDefaults() {
     public FlutterInjector build() {
       fillDefaults();
 
-      return new FlutterInjector(flutterLoader, deferredComponentManager);
+      return new FlutterInjector(flutterLoader, deferredComponentManager, flutterJniFactory);
     }
   }
 }

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

@@ -919,12 +919,21 @@ public PlatformPlugin providePlatformPlugin(
    * <p>This method is called after {@link #provideFlutterEngine(Context)}.
    *
    * <p>All plugins listed in the app's pubspec are registered in the base implementation of this
-   * method. To avoid automatic plugin registration, override this method without invoking super().
-   * To keep automatic plugin registration and further configure the flutterEngine, override this
-   * method, invoke super(), and then configure the flutterEngine as desired.
+   * method unless the FlutterEngine for this activity was externally created. To avoid the
+   * automatic plugin registration for implicitly created FlutterEngines, override this method
+   * without invoking super(). To keep automatic plugin registration and further configure the
+   * FlutterEngine, override this method, invoke super(), and then configure the FlutterEngine as
+   * desired.
    */
   @Override
   public void configureFlutterEngine(@NonNull FlutterEngine flutterEngine) {
+    if (delegate.isFlutterEngineFromHost()) {
+      // If the FlutterEngine was explicitly built and injected into this FlutterActivity, the
+      // builder should explicitly decide whether to automatically register plugins via the
+      // FlutterEngine's construction parameter or via the AndroidManifest metadata.
+      return;
+    }
+
     GeneratedPluginRegister.registerGeneratedPlugins(flutterEngine);
   }
 

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

@@ -835,6 +835,14 @@ public String getCachedEngineId() {
     return getArguments().getString(ARG_CACHED_ENGINE_ID, null);
   }
 
+  /**
+   * Returns true a {@code FlutterEngine} was explicitly created and injected into the {@code
+   * FlutterFragment} rather than one that was created implicitly in the {@code FlutterFragment}.
+   */
+  /* package */ boolean isFlutterEngineInjected() {
+    return delegate.isFlutterEngineFromHost();
+  }
+
   /**
    * Returns false if the {@link FlutterEngine} within this {@code FlutterFragment} should outlive
    * the {@code FlutterFragment}, itself.

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

@@ -580,12 +580,21 @@ public FlutterEngine provideFlutterEngine(@NonNull Context context) {
    * <p>This method is called after {@link #provideFlutterEngine(Context)}.
    *
    * <p>All plugins listed in the app's pubspec are registered in the base implementation of this
-   * method. To avoid automatic plugin registration, override this method without invoking super().
-   * To keep automatic plugin registration and further configure the flutterEngine, override this
-   * method, invoke super(), and then configure the flutterEngine as desired.
+   * method unless the FlutterEngine for this activity was externally created. To avoid the
+   * automatic plugin registration for implicitly created FlutterEngines, override this method
+   * without invoking super(). To keep automatic plugin registration and further configure the
+   * FlutterEngine, override this method, invoke super(), and then configure the FlutterEngine as
+   * desired.
    */
   @Override
   public void configureFlutterEngine(@NonNull FlutterEngine flutterEngine) {
+    if (flutterFragment.isFlutterEngineInjected()) {
+      // If the FlutterEngine was explicitly built and injected into this FlutterActivity, the
+      // builder should explicitly decide whether to automatically register plugins via the
+      // FlutterEngine's construction parameter or via the AndroidManifest metadata.
+      return;
+    }
+
     GeneratedPluginRegister.registerGeneratedPlugins(flutterEngine);
   }
 

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

@@ -20,6 +20,7 @@
 import io.flutter.embedding.engine.plugins.broadcastreceiver.BroadcastReceiverControlSurface;
 import io.flutter.embedding.engine.plugins.contentprovider.ContentProviderControlSurface;
 import io.flutter.embedding.engine.plugins.service.ServiceControlSurface;
+import io.flutter.embedding.engine.plugins.util.GeneratedPluginRegister;
 import io.flutter.embedding.engine.renderer.FlutterRenderer;
 import io.flutter.embedding.engine.renderer.RenderSurface;
 import io.flutter.embedding.engine.systemchannels.AccessibilityChannel;
@@ -36,7 +37,6 @@
 import io.flutter.embedding.engine.systemchannels.TextInputChannel;
 import io.flutter.plugin.localization.LocalizationPlugin;
 import io.flutter.plugin.platform.PlatformViewsController;
-import java.lang.reflect.Method;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -157,7 +157,7 @@ public FlutterEngine(@NonNull Context context) {
    * <p>If the Dart VM has already started, the given arguments will have no effect.
    */
   public FlutterEngine(@NonNull Context context, @Nullable String[] dartVmArgs) {
-    this(context, /* flutterLoader */ null, new FlutterJNI(), dartVmArgs, true);
+    this(context, /* flutterLoader */ null, /* flutterJNI */ null, dartVmArgs, true);
   }
 
   /**
@@ -173,7 +173,7 @@ public FlutterEngine(
     this(
         context,
         /* flutterLoader */ null,
-        new FlutterJNI(),
+        /* flutterJNI */ null,
         dartVmArgs,
         automaticallyRegisterPlugins);
   }
@@ -204,7 +204,7 @@ public FlutterEngine(
     this(
         context,
         /* flutterLoader */ null,
-        new FlutterJNI(),
+        /* flutterJNI */ null,
         new PlatformViewsController(),
         dartVmArgs,
         automaticallyRegisterPlugins,
@@ -282,6 +282,14 @@ public FlutterEngine(
     } catch (NameNotFoundException e) {
       assetManager = context.getAssets();
     }
+
+    FlutterInjector injector = FlutterInjector.instance();
+
+    if (flutterJNI == null) {
+      flutterJNI = injector.getFlutterJNIFactory().provideFlutterJNI();
+    }
+    this.flutterJNI = flutterJNI;
+
     this.dartExecutor = new DartExecutor(flutterJNI, assetManager);
     this.dartExecutor.onAttachedToJNI();
 
@@ -307,9 +315,8 @@ public FlutterEngine(
 
     this.localizationPlugin = new LocalizationPlugin(context, localizationChannel);
 
-    this.flutterJNI = flutterJNI;
     if (flutterLoader == null) {
-      flutterLoader = FlutterInjector.instance().flutterLoader();
+      flutterLoader = injector.flutterLoader();
     }
 
     if (!flutterJNI.isAttached()) {
@@ -320,7 +327,7 @@ public FlutterEngine(
     flutterJNI.addEngineLifecycleListener(engineLifecycleListener);
     flutterJNI.setPlatformViewsController(platformViewsController);
     flutterJNI.setLocalizationPlugin(localizationPlugin);
-    flutterJNI.setDeferredComponentManager(FlutterInjector.instance().deferredComponentManager());
+    flutterJNI.setDeferredComponentManager(injector.deferredComponentManager());
 
     // It should typically be a fresh, unattached JNI. But on a spawned engine, the JNI instance
     // is already attached to a native shell. In that case, the Java FlutterEngine is created around
@@ -342,7 +349,7 @@ public FlutterEngine(
     // Only automatically register plugins if both constructor parameter and
     // loaded AndroidManifest config turn this feature on.
     if (automaticallyRegisterPlugins && flutterLoader.automaticallyRegisterPlugins()) {
-      registerPlugins();
+      GeneratedPluginRegister.registerGeneratedPlugins(this);
     }
   }
 
@@ -391,36 +398,6 @@ private boolean isAttachedToJni() {
         newFlutterJNI); // FlutterJNI.
   }
 
-  /**
-   * Registers all plugins that an app lists in its pubspec.yaml.
-   *
-   * <p>The Flutter tool generates a class called GeneratedPluginRegistrant, which includes the code
-   * necessary to register every plugin in the pubspec.yaml with a given {@code FlutterEngine}. The
-   * GeneratedPluginRegistrant must be generated per app, because each app uses different sets of
-   * plugins. Therefore, the Android embedding cannot place a compile-time dependency on this
-   * generated class. This method uses reflection to attempt to locate the generated file and then
-   * use it at runtime.
-   *
-   * <p>This method fizzles if the GeneratedPluginRegistrant cannot be found or invoked. This
-   * situation should never occur, but if any eventuality comes up that prevents an app from using
-   * this behavior, that app can still write code that explicitly registers plugins.
-   */
-  private void registerPlugins() {
-    try {
-      Class<?> generatedPluginRegistrant =
-          Class.forName("io.flutter.plugins.GeneratedPluginRegistrant");
-      Method registrationMethod =
-          generatedPluginRegistrant.getDeclaredMethod("registerWith", FlutterEngine.class);
-      registrationMethod.invoke(null, this);
-    } catch (Exception e) {
-      Log.w(
-          TAG,
-          "Tried to automatically register plugins with FlutterEngine ("
-              + this
-              + ") but could not find and invoke the GeneratedPluginRegistrant.");
-    }
-  }
-
   /**
    * Cleans up all components within this {@code FlutterEngine} and destroys the associated Dart
    * Isolate. All state held by the Dart Isolate, such as the Flutter Elements tree, is lost.

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

@@ -282,6 +282,7 @@ public static native void nativeOnVsync(
 
   @NonNull private final Looper mainLooper; // cached to avoid synchronization on repeat access.
 
+  // Prefer using the FlutterJNI.Factory so it's easier to test.
   public FlutterJNI() {
     // We cache the main looper so that we can ensure calls are made on the main thread
     // without consistently paying the synchronization cost of getMainLooper().
@@ -1258,4 +1259,15 @@ public interface AccessibilityDelegate {
   public interface AsyncWaitForVsyncDelegate {
     void asyncWaitForVsync(final long cookie);
   }
+
+  /**
+   * A factory for creating {@code FlutterJNI} instances. Useful for FlutterJNI injections during
+   * tests.
+   */
+  public static class Factory {
+    /** @return a {@link FlutterJNI} instance. */
+    public FlutterJNI provideFlutterJNI() {
+      return new FlutterJNI();
+    }
+  }
 }

shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java

@@ -17,6 +17,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import io.flutter.BuildConfig;
+import io.flutter.FlutterInjector;
 import io.flutter.Log;
 import io.flutter.embedding.engine.FlutterJNI;
 import io.flutter.util.PathUtils;
@@ -70,7 +71,7 @@ public static FlutterLoader getInstance() {
 
   /** Creates a {@code FlutterLoader} that uses a default constructed {@link FlutterJNI}. */
   public FlutterLoader() {
-    this(new FlutterJNI());
+    this(FlutterInjector.instance().getFlutterJNIFactory().provideFlutterJNI());
   }
 
   /**

shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java

@@ -33,11 +33,12 @@ public static void registerGeneratedPlugins(@NonNull FlutterEngine flutterEngine
           generatedPluginRegistrant.getDeclaredMethod("registerWith", FlutterEngine.class);
       registrationMethod.invoke(null, flutterEngine);
     } catch (Exception e) {
-      Log.w(
+      Log.e(
           TAG,
           "Tried to automatically register plugins with FlutterEngine ("
               + flutterEngine
-              + ") but could not find and invoke the GeneratedPluginRegistrant.");
+              + ") but could not find or invoke the GeneratedPluginRegistrant.");
+      Log.e(TAG, "Received exception while registering", e);
     }
   }
 }