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

Register plugins at the right time, once #15979

Merged
merged 3 commits into from
Jan 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions shell/platform/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ action("robolectric_tests") {
"test/io/flutter/plugin/common/StandardMessageCodecTest.java",
"test/io/flutter/plugin/editing/TextInputPluginTest.java",
"test/io/flutter/plugin/platform/SingleViewPresentationTest.java",
"test/io/flutter/plugins/GeneratedPluginRegistrant.java",
"test/io/flutter/util/PreconditionsTest.java",
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
import io.flutter.plugin.platform.PlatformPlugin;
import io.flutter.view.FlutterMain;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;

import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.DART_ENTRYPOINT_META_DATA_KEY;
import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.DEFAULT_BACKGROUND_MODE;
import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.DEFAULT_DART_ENTRYPOINT;
Expand Down Expand Up @@ -873,14 +876,18 @@ public PlatformPlugin providePlatformPlugin(@Nullable Activity activity, @NonNul
}

/**
* Hook for subclasses to easily configure a {@code FlutterEngine}, e.g., register
* plugins.
* <p>
* This method is called after {@link #provideFlutterEngine(Context)}.
* Hook for subclasses to easily configure a {@code FlutterEngine}.
*
* <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.
*/
@Override
public void configureFlutterEngine(@NonNull FlutterEngine flutterEngine) {
// No-op. Hook for subclasses.
registerPlugins(flutterEngine);
}

/**
Expand Down Expand Up @@ -950,4 +957,28 @@ public void onFlutterUiNoLongerDisplayed() {
// no-op
}

/**
* Registers all plugins that an app lists in its pubspec.yaml.
Copy link

Choose a reason for hiding this comment

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

minor nit: plugins that support the android platform

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that would be redundant to point out here. This is already a Javadoc comment in the Android embedding specifically and not a generic part of the engine's C++ source code, so I don't think it's worth pointing out that this logic is Android-only. If you feel strongly about it I can update it though.

Copy link

Choose a reason for hiding this comment

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

Not at all. 🙂Although, if someone wants to understand why a plugin isn't added to the GeneratedPluginRegistrant, the comment might be useful.

The current definition of supporting the Android platform is a plugin that contains android/build.gradle.

https://api.flutter.dev/javadoc/io/flutter/app/FlutterActivity.html.

* <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}.
Copy link

Choose a reason for hiding this comment

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

ditto

* 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 static void registerPlugins(@NonNull FlutterEngine flutterEngine) {
try {
Class<?> generatedPluginRegistrant = Class.forName("io.flutter.plugins.GeneratedPluginRegistrant");
Method registrationMethod = generatedPluginRegistrant.getDeclaredMethod("registerWith", FlutterEngine.class);
registrationMethod.invoke(null, flutterEngine);
} catch (Exception e) {
Log.w(TAG, "Tried to automatically register plugins with FlutterEngine ("
+ flutterEngine + ") but could not find and invoke the GeneratedPluginRegistrant.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import io.flutter.embedding.engine.FlutterShellArgs;
import io.flutter.embedding.engine.dart.DartExecutor;
import io.flutter.embedding.engine.renderer.FlutterUiDisplayListener;
import io.flutter.embedding.engine.FlutterJNI;
import io.flutter.embedding.engine.loader.FlutterLoader;
import io.flutter.plugin.platform.PlatformPlugin;

import static android.content.ComponentCallbacks2.TRIM_MEMORY_RUNNING_LOW;
Expand Down Expand Up @@ -223,7 +225,7 @@ void onAttach(@NonNull Context context) {
// FlutterView.
Log.v(TAG, "No preferred FlutterEngine was provided. Creating a new FlutterEngine for"
+ " this FlutterFragment.");
flutterEngine = new FlutterEngine(host.getContext(), host.getFlutterShellArgs().toArray());
flutterEngine = new FlutterEngine(host.getContext(), host.getFlutterShellArgs().toArray(), /*automaticallyRegisterPlugins=*/false);
isFlutterEngineFromHost = false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ public void onPreEngineRestart() {
* {@link RenderSurface} is registered. See
* {@link #getRenderer()} and {@link FlutterRenderer#startRenderingToSurface(RenderSurface)}.
* <p>
* A new {@code FlutterEngine} does not come with any Flutter plugins attached. To attach plugins,
* see {@link #getPlugins()}.
* A new {@code FlutterEngine} automatically attaches all plugins. See {@link #getPlugins()}.
* <p>
* A new {@code FlutterEngine} does come with all default system channels attached.
* <p>
Expand All @@ -152,6 +151,19 @@ public FlutterEngine(@NonNull Context context, @Nullable String[] dartVmArgs) {
this(context, FlutterLoader.getInstance(), new FlutterJNI(), dartVmArgs, true);
}

/**
* Same as {@link #FlutterEngine(Context)} with added support for passing Dart
* VM arguments and avoiding automatic plugin registration.
* <p>
* If the Dart VM has already started, the given arguments will have no effect.
*/
public FlutterEngine(
@NonNull Context context,
@Nullable String[] dartVmArgs,
boolean automaticallyRegisterPlugins) {
this(context, FlutterLoader.getInstance(), new FlutterJNI(), dartVmArgs, automaticallyRegisterPlugins);
}

/**
* Same as {@link #FlutterEngine(Context, FlutterLoader, FlutterJNI, String[])} but with no Dart
* VM flags.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;

import io.flutter.plugins.GeneratedPluginRegistrant;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.Robolectric;
Expand All @@ -30,6 +34,16 @@
@Config(manifest=Config.NONE)
@RunWith(RobolectricTestRunner.class)
public class FlutterActivityTest {
@Before
public void setUp() {
GeneratedPluginRegistrant.clearRegisteredEngines();
}

@After
public void tearDown() {
GeneratedPluginRegistrant.clearRegisteredEngines();
}

@Test
public void itCreatesDefaultIntentWithExpectedDefaults() {
Intent intent = FlutterActivity.createDefaultIntent(RuntimeEnvironment.application);
Expand Down Expand Up @@ -122,6 +136,19 @@ public void itCreatesCachedEngineIntentThatDestroysTheEngine() {
assertTrue(flutterActivity.shouldDestroyEngineWithHost());
}

@Test
public void itRegistersPluginsAtConfigurationTime() {
FlutterActivity activity = Robolectric.buildActivity(FlutterActivityWithProvidedEngine.class).get();
activity.onCreate(null);

assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty());
activity.configureFlutterEngine(activity.getFlutterEngine());

List<FlutterEngine> registeredEngines = GeneratedPluginRegistrant.getRegisteredEngines();
assertEquals(1, registeredEngines.size());
assertEquals(activity.getFlutterEngine(), registeredEngines.get(0));
}

static class FlutterActivityWithProvidedEngine extends FlutterActivity {
@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
Expand All @@ -140,7 +167,7 @@ public FlutterEngine provideFlutterEngine(@NonNull Context context) {
mock(FlutterLoader.class),
flutterJNI,
new String[]{},
true
false
);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package test.io.flutter.embedding.engine;

import io.flutter.plugins.GeneratedPluginRegistrant;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;
Expand All @@ -10,25 +16,52 @@
import io.flutter.embedding.engine.FlutterJNI;
import io.flutter.embedding.engine.loader.FlutterLoader;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

@Config(manifest=Config.NONE)
@RunWith(RobolectricTestRunner.class)
public class FlutterEngineTest {
@Test
public void itDoesNotCrashIfGeneratedPluginRegistrantIsUnavailable() {
FlutterJNI flutterJNI = mock(FlutterJNI.class);
@Mock FlutterJNI flutterJNI;

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
when(flutterJNI.isAttached()).thenReturn(true);
GeneratedPluginRegistrant.clearRegisteredEngines();
}

@After
public void tearDown() {
GeneratedPluginRegistrant.clearRegisteredEngines();
}

@Test
public void itAutomaticallyRegistersPluginsByDefault() {
assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty());
FlutterEngine flutterEngine = new FlutterEngine(
RuntimeEnvironment.application,
mock(FlutterLoader.class),
flutterJNI
);

List<FlutterEngine> registeredEngines = GeneratedPluginRegistrant.getRegisteredEngines();
assertEquals(1, registeredEngines.size());
assertEquals(flutterEngine, registeredEngines.get(0));
}

@Test
public void itCanBeConfiguredToNotAutomaticallyRegisterPlugins() {
new FlutterEngine(
RuntimeEnvironment.application,
mock(FlutterLoader.class),
flutterJNI,
new String[] {},
true
/*dartVmArgs=*/new String[] {},
/*automaticallyRegisterPlugins=*/false
);
// The fact that the above constructor executed without error means that
// it dealt with a non-existent GeneratedPluginRegistrant.

assertTrue(GeneratedPluginRegistrant.getRegisteredEngines().isEmpty());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package io.flutter.plugins;

import android.support.annotation.VisibleForTesting;
import io.flutter.embedding.engine.FlutterEngine;
import java.util.List;
import java.util.ArrayList;

/**
* A fake of the {@code GeneratedPluginRegistrant} normally built by the tool into Flutter apps.
*
* <p>Used to test engine logic which interacts with the generated class.
*/
@VisibleForTesting
public class GeneratedPluginRegistrant {
private static final List<FlutterEngine> registeredEngines = new ArrayList<>();

/**
* The one and only method currently generated by the tool.
*
* <p>Normally it registers all plugins in an app with the given {@code engine}. This fake tracks
* all registered engines instead.
*/
public static void registerWith(FlutterEngine engine) {
registeredEngines.add(engine);
}

/**
* Clears the mutable static state regrettably stored in this class.
*
* <p>{@link #registerWith} is a static call with no visible side effects. In order to verify when
* it's been called we also unfortunately need to store the state statically. This should be
* called before and after each test run accessing this class to make sure the state is clear both
* before and after the run.
*/
public static void clearRegisteredEngines() {
registeredEngines.clear();
}

/**
* Returns a list of all the engines registered so far.
*
* <p>CAUTION: This list is static and must be manually wiped in between test runs. See
* {@link #clearRegisteredEngines()}.
*/
public static List<FlutterEngine> getRegisteredEngines() {
return new ArrayList<>(registeredEngines);
}
}