From 1265af369b27725b006dfe943c855b9e590da3bc Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Mon, 29 Mar 2021 15:11:01 -0700 Subject: [PATCH 1/4] Deduplicate plugin registration logic --- .../embedding/engine/FlutterEngine.java | 33 ++----------------- .../plugins/util/GeneratedPluginRegister.java | 5 ++- 2 files changed, 6 insertions(+), 32 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java index 3d7f197dcfeeb..3b3124c80971c 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java +++ b/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; @@ -342,7 +343,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 +392,6 @@ private boolean isAttachedToJni() { newFlutterJNI); // FlutterJNI. } - /** - * Registers all plugins that an app lists in its pubspec.yaml. - * - *

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. - * - *

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. diff --git a/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java b/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java index 5bb0ff5a6f37b..8759bde2316ae 100644 --- a/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java +++ b/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java @@ -33,11 +33,14 @@ 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, + "Received exception " + e.getCause()); } } } From 6bfdde09989f3d0b45c4a43d8d1f7a8966167089 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Mon, 29 Mar 2021 21:08:34 -0700 Subject: [PATCH 2/4] part test --- .../embedding/engine/FlutterEngineTest.java | 30 ++++++++++++------- .../plugins/GeneratedPluginRegistrant.java | 4 +++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java index bcfabf863dafe..2b8f5a03b1c3c 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java +++ b/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) @@ -78,6 +79,25 @@ 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 registeredEngines = GeneratedPluginRegistrant.getRegisteredEngines(); + // When it crashes, it doesn't end up registering anything. + assertEquals(0, registeredEngines.size()); + assertTrue(ShadowLog.getLogsForTag("GeneratedPluginsRegister")[0].msg.contains("Tried to automatically register plugins")); + + GeneratedPluginRegistrant.pluginRegistrationException = null; + } + @Test public void itDoesNotAutomaticallyRegistersPluginsWhenFlutterLoaderDisablesIt() { assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty()); @@ -105,17 +125,7 @@ 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() { diff --git a/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java b/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java index 9b93bd6399464..afef14ffe80f9 100644 --- a/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java +++ b/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java @@ -13,6 +13,7 @@ @VisibleForTesting public class GeneratedPluginRegistrant { private static final List registeredEngines = new ArrayList<>(); + public static Exception 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); } From 44692dc1229439414e9437d22ac3bf91ac77208a Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Mon, 29 Mar 2021 22:01:59 -0700 Subject: [PATCH 3/4] add test --- .../engine/plugins/util/GeneratedPluginRegister.java | 3 ++- .../io/flutter/embedding/engine/FlutterEngineTest.java | 9 ++++++++- .../io/flutter/plugins/GeneratedPluginRegistrant.java | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java b/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java index 8759bde2316ae..62580cd0008c2 100644 --- a/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java +++ b/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java @@ -40,7 +40,8 @@ public static void registerGeneratedPlugins(@NonNull FlutterEngine flutterEngine + ") but could not find and invoke the GeneratedPluginRegistrant."); Log.e( TAG, - "Received exception " + e.getCause()); + // getCause here because the first layer of the exception would be from reflect. + "Received exception while registering: " + e.getCause()); } } } diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java index 2b8f5a03b1c3c..578d64d890d62 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java @@ -64,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 @@ -93,7 +96,11 @@ public void itCatchesAndDisplaysRegistrationExceptions() { List registeredEngines = GeneratedPluginRegistrant.getRegisteredEngines(); // When it crashes, it doesn't end up registering anything. assertEquals(0, registeredEngines.size()); - assertTrue(ShadowLog.getLogsForTag("GeneratedPluginsRegister")[0].msg.contains("Tried to automatically register plugins")); + + // 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; } diff --git a/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java b/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java index afef14ffe80f9..c2528725b89b3 100644 --- a/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java +++ b/shell/platform/android/test/io/flutter/plugins/GeneratedPluginRegistrant.java @@ -13,7 +13,7 @@ @VisibleForTesting public class GeneratedPluginRegistrant { private static final List registeredEngines = new ArrayList<>(); - public static Exception pluginRegistrationException; + public static RuntimeException pluginRegistrationException; /** * The one and only method currently generated by the tool. From a1ebd0b9bce8793a1618c43ed162e9c825283088 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Tue, 30 Mar 2021 10:14:42 -0700 Subject: [PATCH 4/4] autoformat --- .../flutter/embedding/engine/FlutterEngine.java | 1 - .../plugins/util/GeneratedPluginRegister.java | 6 +++--- .../embedding/engine/FlutterEngineTest.java | 17 ++++++++++++----- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java index 3b3124c80971c..389d3af06afdb 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java @@ -37,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; diff --git a/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java b/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java index 62580cd0008c2..fab7eb8c7ee65 100644 --- a/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java +++ b/shell/platform/android/io/flutter/embedding/engine/plugins/util/GeneratedPluginRegister.java @@ -39,9 +39,9 @@ public static void registerGeneratedPlugins(@NonNull FlutterEngine 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()); + TAG, + // getCause here because the first layer of the exception would be from reflect. + "Received exception while registering: " + e.getCause()); } } } diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java index 578d64d890d62..1af79de943259 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineTest.java @@ -82,7 +82,8 @@ 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. + // 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()); @@ -99,8 +100,16 @@ public void itCatchesAndDisplaysRegistrationExceptions() { // 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")); + 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; } @@ -132,8 +141,6 @@ public void itDoesNotAutomaticallyRegistersPluginsWhenFlutterEngineDisablesIt() assertTrue(registeredEngines.isEmpty()); } - - @Test public void itNotifiesPlatformViewsControllerWhenDevHotRestart() { // Setup test.