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

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 thi 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,15 @@ 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 thi 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;
 
@@ -342,7 +342,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 +391,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/plugins/util/GeneratedPluginRegister.java

@@ -33,11 +33,15 @@ 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.");
+      Log.e(
+          TAG,
+          // getCause here because the first layer of the exception would be from reflect.
+          "Received exception while registering: " + e.getCause());
     }
   }
 }

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

@@ -21,6 +21,7 @@
 import androidx.annotation.Nullable;
 import androidx.lifecycle.DefaultLifecycleObserver;
 import androidx.lifecycle.LifecycleOwner;
+import io.flutter.FlutterInjector;
 import io.flutter.embedding.android.FlutterActivityLaunchConfigs.BackgroundMode;
 import io.flutter.embedding.engine.FlutterEngine;
 import io.flutter.embedding.engine.FlutterEngineCache;
@@ -48,11 +49,15 @@ public class FlutterActivityTest {
   @Before
   public void setUp() {
     GeneratedPluginRegistrant.clearRegisteredEngines();
+    FlutterJNI mockFlutterJNI = mock(FlutterJNI.class);
+    FlutterInjector.setInstance(
+        new FlutterInjector.Builder().setFlutterLoader(new FlutterLoader(mockFlutterJNI)).build());
   }
 
   @After
   public void tearDown() {
     GeneratedPluginRegistrant.clearRegisteredEngines();
+    FlutterInjector.reset();
   }
 
   @Test
@@ -211,8 +216,10 @@ public void itCreatesCachedEngineIntentThatDestroysTheEngine() {
 
   @Test
   public void itRegistersPluginsAtConfigurationTime() {
-    FlutterActivity activity =
-        Robolectric.buildActivity(FlutterActivityWithProvidedEngine.class).get();
+    Intent intent = FlutterActivity.createDefaultIntent(RuntimeEnvironment.application);
+    ActivityController<FlutterActivity> activityController =
+        Robolectric.buildActivity(FlutterActivity.class, intent);
+    FlutterActivity activity = activityController.get();
     activity.onCreate(null);
 
     assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty());
@@ -282,6 +289,23 @@ public void itRestoresPluginStateBeforePluginOnCreate() {
         "Expected FakeFlutterPlugin onCreateCalled to be true", fakeFlutterPlugin.onCreateCalled);
   }
 
+  @Test
+  public void itDoesNotRegisterPluginsTwiceWhenUsingACachedEngine() {
+    Intent intent =
+        new Intent(RuntimeEnvironment.application, FlutterActivityWithProvidedEngine.class);
+    ActivityController<FlutterActivityWithProvidedEngine> activityController =
+        Robolectric.buildActivity(FlutterActivityWithProvidedEngine.class, intent);
+    activityController.create();
+    FlutterActivityWithProvidedEngine flutterActivity = activityController.get();
+    flutterActivity.configureFlutterEngine(flutterActivity.getFlutterEngine());
+
+    List<FlutterEngine> registeredEngines = GeneratedPluginRegistrant.getRegisteredEngines();
+    // This might cause the plugins to be registered twice, once by the FlutterEngine constructor,
+    // and once by the default FlutterActivity.configureFlutterEngine implementation.
+    // Test that it doesn't happen.
+    assertEquals(1, registeredEngines.size());
+  }
+
   static class FlutterActivityWithProvidedEngine extends FlutterActivity {
     @Override
     protected void onCreate(@Nullable Bundle savedInstanceState) {
@@ -293,10 +317,12 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
     @Override
     public FlutterEngine provideFlutterEngine(@NonNull Context context) {
       FlutterJNI flutterJNI = mock(FlutterJNI.class);
+      FlutterLoader flutterLoader = mock(FlutterLoader.class);
       when(flutterJNI.isAttached()).thenReturn(true);
+      when(flutterLoader.automaticallyRegisterPlugins()).thenReturn(true);
 
       return new FlutterEngine(
-          context, mock(FlutterLoader.class), flutterJNI, new String[] {}, false);
+          context, flutterLoader, flutterJNI, new String[] {}, true);
     }
   }
 

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

@@ -89,6 +89,20 @@ public void itRegistersPluginsAtConfigurationTime() {
     assertEquals(activity.getFlutterEngine(), registeredEngines.get(0));
   }
 
+  @Test
+  public void itDoesNotRegisterPluginsTwiceWhenUsingACachedEngine() {
+    FlutterFragmentActivity activity =
+        Robolectric.buildActivity(FlutterFragmentActivityWithProvidedEngine.class).get();
+    activity.onCreate(null);
+    activity.configureFlutterEngine(activity.getFlutterEngine());
+
+    List<FlutterEngine> registeredEngines = GeneratedPluginRegistrant.getRegisteredEngines();
+    // This might cause the plugins to be registered twice, once by the FlutterEngine constructor,
+    // and once by the default FlutterFragmentActivity.configureFlutterEngine implementation.
+    // Test that it doesn't happen.
+    assertEquals(1, registeredEngines.size());
+  }
+
   @Test
   public void itReturnsValueFromMetaDataWhenCallsShouldHandleDeepLinkingCase1()
       throws PackageManager.NameNotFoundException {
@@ -157,10 +171,12 @@ protected FlutterFragment createFlutterFragment() {
     @Override
     public FlutterEngine provideFlutterEngine(@NonNull Context context) {
       FlutterJNI flutterJNI = mock(FlutterJNI.class);
+      FlutterLoader flutterLoader = mock(FlutterLoader.class);
       when(flutterJNI.isAttached()).thenReturn(true);
+      when(flutterLoader.automaticallyRegisterPlugins()).thenReturn(true);
 
       return new FlutterEngine(
-          context, mock(FlutterLoader.class), flutterJNI, new String[] {}, false);
+          context, flutterLoader, flutterJNI, new String[] {}, true);
     }
   }
 

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

@@ -35,6 +35,7 @@
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
+import org.robolectric.shadows.ShadowLog;
 
 @Config(manifest = Config.NONE)
 @RunWith(RobolectricTestRunner.class)
@@ -63,6 +64,9 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
   @After
   public void tearDown() {
     GeneratedPluginRegistrant.clearRegisteredEngines();
+    // Make sure to not forget to remove the mock exception in the generated plugin registration
+    // mock, or everything subsequent will break.
+    GeneratedPluginRegistrant.pluginRegistrationException = null;
   }
 
   @Test
@@ -78,6 +82,38 @@ public void itAutomaticallyRegistersPluginsByDefault() {
     assertEquals(flutterEngine, registeredEngines.get(0));
   }
 
+  // Helps show the root cause of MissingPluginException type errors like
+  // https://github.com/flutter/flutter/issues/78625.
+  @Test
+  public void itCatchesAndDisplaysRegistrationExceptions() {
+    assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty());
+    GeneratedPluginRegistrant.pluginRegistrationException =
+        new RuntimeException("I'm a bug in the plugin");
+    FlutterLoader mockFlutterLoader = mock(FlutterLoader.class);
+    when(mockFlutterLoader.automaticallyRegisterPlugins()).thenReturn(true);
+    FlutterEngine flutterEngine =
+        new FlutterEngine(RuntimeEnvironment.application, mockFlutterLoader, flutterJNI);
+
+    List<FlutterEngine> registeredEngines = GeneratedPluginRegistrant.getRegisteredEngines();
+    // When it crashes, it doesn't end up registering anything.
+    assertEquals(0, registeredEngines.size());
+
+    // Check the logs actually says registration failed, so a subsequent MissingPluginException
+    // isn't mysterious.
+    assertTrue(
+        ShadowLog.getLogsForTag("GeneratedPluginsRegister")
+            .get(0)
+            .msg
+            .contains("Tried to automatically register plugins"));
+    assertTrue(
+        ShadowLog.getLogsForTag("GeneratedPluginsRegister")
+            .get(1)
+            .msg
+            .contains("I'm a bug in the plugin"));
+
+    GeneratedPluginRegistrant.pluginRegistrationException = null;
+  }
+
   @Test
   public void itDoesNotAutomaticallyRegistersPluginsWhenFlutterLoaderDisablesIt() {
     assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty());
@@ -105,18 +141,6 @@ public void itDoesNotAutomaticallyRegistersPluginsWhenFlutterEngineDisablesIt()
     assertTrue(registeredEngines.isEmpty());
   }
 
-  @Test
-  public void itCanBeConfiguredToNotAutomaticallyRegisterPlugins() {
-    new FlutterEngine(
-        RuntimeEnvironment.application,
-        mock(FlutterLoader.class),
-        flutterJNI,
-        /*dartVmArgs=*/ new String[] {},
-        /*automaticallyRegisterPlugins=*/ false);
-
-    assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty());
-  }
-
   @Test
   public void itNotifiesPlatformViewsControllerWhenDevHotRestart() {
     // Setup test.

shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java

@@ -13,6 +13,7 @@
 @VisibleForTesting
 public class GeneratedPluginRegistrant {
   private static final List<FlutterEngine> registeredEngines = new ArrayList<>();
+  public static RuntimeException pluginRegistrationException;
 
   /**
    * The one and only method currently generated by the tool.
@@ -21,6 +22,9 @@ public class GeneratedPluginRegistrant {
    * all registered engines instead.
    */
   public static void registerWith(FlutterEngine engine) {
+    if (pluginRegistrationException != null) {
+      throw pluginRegistrationException;
+    }
     registeredEngines.add(engine);
   }