Register plugins at the right time, once #15979

mklim · 2020-01-24T23:40:33Z

Description

Currently we're automatically registering plugins both when the
FlutterEngine is constructed and in the flutter create template, when
FlutterActivity#configureFlutterEngine is called. The initial
registration is too early to contain a reference to the activity and the
second registration can cause problems in some plugins.

This alters the flow so automatic registration happens in two discrete
places, and contains the activity in its first and only call for most
apps.

We're no longer automatically registering plugins on FlutterEngine
in any of our activities/fragments at construction time. But since the
FlutterEngine default constructor still automatically registers plugins,
anyone constructing the engine themselves (for example, in a service) is
still going to get automatic registration at FlutterEngine
instantiation time.
We now automatically register plugins in the base FlutterActivity's
configureFlutterEngine hook. Anyone using FlutterActivity (or
FlutterFragment) should be automatically registered once that hook is
called. Right now the flutter create template overrides the base class
method with a subclass that registers everything manually in the same
spot. But with this in place we can safely recommend to remove the
subclass and rely on this hook to automatically register going forward.
Registering at this time means activity is set correctly.

Related Issues

Fixes Automatic plugin registration breaks some cases flutter#49365 in the common case. Prevent duplicate plugin registration in FlutterEnginePluginRegistry. (#49365) #15956 is also needed to prevent duplicate registration in all cases.
Fixes "Upgrading pre 1.12" Android instructions causes Android crashes as written flutter#47921
Fixes [firebase_messaging] onMessage/onResume is called twice on Android/iOS firebase/flutterfire#1669

matthew-carroll

LGTM. Thanks.

FYI, I'm still waiting on LUCI to merge in my related PR.

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

Currently we're automatically registering plugins both when the FlutterEngine is constructed and in the `flutter create` template, when FlutterActivity#configureFlutterEngine is called. The initial registration is too early to contain a reference to the activity and the second registration can cause problems in some plugins. This alters the flow so automatic registration happens in two discrete places, and contains the `activity` in its first and only call for most apps. 1. We're no longer automatically registering plugins on `FlutterEngine` in any of our activities/fragments at construction time. But since the FlutterEngine default constructor still automatically registers plugins, anyone constructing the engine themselves (for example, in a service) is still going to get automatic registration at `FlutterEngine` instantiation time. 2. We now automatically register plugins in the base `FlutterActivity`'s `configureFlutterEngine` hook. Anyone using `FlutterActivity` (or `FlutterFragment`) should be automatically registered once that hook is called. Right now the `flutter create` template overrides the base class method with a subclass that registers everything manually in the same spot. But with this in place we can safely recommend to remove the subclass and rely on this hook to automatically register going forward. Registering at this time means `activity` is set correctly.

1. Previous commit accidentally ate `dartVmArgs`, change the new `FlutterEngine` constructor so that it still is passed through. 2. `GeneratedPluginRegistrant` is in `.gitignore` so the last commit didn't have the fake, force the test fake to be added into version control and commit it.

blasten · 2020-01-25T00:44:06Z

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

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

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


minor nit: plugins that support the android platform

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.

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.

blasten · 2020-01-25T00:44:27Z

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

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


blasten · 2020-01-25T00:48:41Z

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

+    this(context, FlutterLoader.getInstance(), new FlutterJNI(), dartVmArgs, automaticallyRegisterPlugins);
+  }
+
+


minor nit: extra line

blasten

LGTM + nits

scognito · 2020-01-25T10:46:35Z

Is there a workaround to fix this issue on stable channel? I'm using Google maps and Firebase and getting notifications twice

… (#49485) flutter/engine@f30ff4f...6fa1fcd git log f30ff4f..6fa1fcd --first-parent --oneline 2020-01-25 [email protected] Register plugins at the right time, once (flutter/engine#15979) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

mklim · 2020-01-27T18:21:18Z

@scognito there isn't a good workaround on stable yet, sorry. Because of how this is broken there isn't an obvious workaround that doesn't cause a secondary issue. I'm going to see if this can be hotfixed, but for now it's only rolled into the master channel.

scognito · 2020-01-27T20:01:06Z

Switching to master could be a good temporary solution for me.
Once switched to master, and after a flutter clean and build, do I need to change something else?
Thanks for your efforts.

Currently we're automatically registering plugins both when the FlutterEngine is constructed and in the `flutter create` template, when FlutterActivity#configureFlutterEngine is called. The initial registration is too early to contain a reference to the activity and the second registration can cause problems in some plugins. This alters the flow so automatic registration happens in two discrete places, and contains the `activity` in its first and only call for most apps. 1. We're no longer automatically registering plugins on `FlutterEngine` in any of our activities/fragments at construction time. But since the FlutterEngine default constructor still automatically registers plugins, anyone constructing the engine themselves (for example, in a service) is still going to get automatic registration at `FlutterEngine` instantiation time. 2. We now automatically register plugins in the base `FlutterActivity`'s `configureFlutterEngine` hook. Anyone using `FlutterActivity` (or `FlutterFragment`) should be automatically registered once that hook is called. Right now the `flutter create` template overrides the base class method with a subclass that registers everything manually in the same spot. But with this in place we can safely recommend to remove the subclass and rely on this hook to automatically register going forward. Registering at this time means `activity` is set correctly.

This reverts commit 21ea450.

mklim requested a review from matthew-carroll January 24, 2020 23:40

auto-assign bot requested a review from gaaclarke January 24, 2020 23:40

googlebot added the cla: yes label Jan 24, 2020

mklim requested a review from blasten January 24, 2020 23:41

matthew-carroll approved these changes Jan 24, 2020

View reviewed changes

mklim commented Jan 24, 2020

View reviewed changes

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java Outdated Show resolved Hide resolved

mklim added affects: add-to-app affects: engine labels Jan 24, 2020

mklim mentioned this pull request Jan 25, 2020

Automatic plugin registration breaks some cases flutter/flutter#49365

Closed

Michael Klimushyn added 2 commits January 24, 2020 16:18

blasten reviewed Jan 25, 2020

View reviewed changes

blasten approved these changes Jan 25, 2020

View reviewed changes

Review feedback

d518d94

mklim merged commit 6fa1fcd into flutter:master Jan 25, 2020

mklim deleted the fix_registration branch January 25, 2020 08:35

engine-flutter-autoroll mentioned this pull request Jan 25, 2020

Roll engine f30ff4fcbf19..6fa1fcda8f78 (1 commits) flutter/flutter#49485

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 25, 2020

6fa1fcd Register plugins at the right time, once (flutter/engine#15979)

d6297a5

matthew-carroll mentioned this pull request Jan 30, 2020

[google_maps_flutter] Trying to create an already created platform view flutter/flutter#45695

Closed

NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Revert "Register plugins at the right time, once (flutter#15979)"

f48117c

This reverts commit 21ea450.

xster mentioned this pull request Apr 23, 2020

The 'Android embedding V2' can't automatically registers plugins when 'MainActivity extends FlutterFragmentActivity' flutter/flutter#54386

Closed

sarbagyastha mentioned this pull request Jun 22, 2020

Automatically register plugins for FlutterFragmentActivity #19199

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Register plugins at the right time, once #15979

Register plugins at the right time, once #15979

mklim commented Jan 24, 2020 •

edited

Loading

matthew-carroll left a comment

blasten Jan 25, 2020

mklim Jan 25, 2020

blasten Jan 25, 2020

blasten Jan 25, 2020

blasten Jan 25, 2020

blasten left a comment

scognito commented Jan 25, 2020

mklim commented Jan 27, 2020

scognito commented Jan 27, 2020

		this(context, FlutterLoader.getInstance(), new FlutterJNI(), dartVmArgs, automaticallyRegisterPlugins);
		}

Register plugins at the right time, once #15979

Register plugins at the right time, once #15979

Conversation

mklim commented Jan 24, 2020 • edited Loading

Description

Related Issues

matthew-carroll left a comment

Choose a reason for hiding this comment

blasten Jan 25, 2020

Choose a reason for hiding this comment

mklim Jan 25, 2020

Choose a reason for hiding this comment

blasten Jan 25, 2020

Choose a reason for hiding this comment

blasten Jan 25, 2020

Choose a reason for hiding this comment

blasten Jan 25, 2020

Choose a reason for hiding this comment

blasten left a comment

Choose a reason for hiding this comment

scognito commented Jan 25, 2020

mklim commented Jan 27, 2020

scognito commented Jan 27, 2020

mklim commented Jan 24, 2020 •

edited

Loading