Android embedding API updates for plugin ecosystem #13280

matthew-carroll · 2019-10-22T05:43:02Z

FlutterPluginBinding is a facade, getFlutterEngine() is deprecated
Lifecycle is split between ActivityAware and ServiceAware, FlutterEngine#getLifecycle() is deprecated
Activity instance state saving callbacks are forwarded to plugins

…gine, split Lifecycle across ActivityAware and ServiceAware and deprected getLifecycle().

matthew-carroll · 2019-10-23T02:13:51Z

@amirh @mklim @cyanglaz @bparrishMines @collinjackson @kroikie Can you all please take a look at FlutterPluginBinding and let me know if you have any thoughts about additions/removals of accessors as a facade for plugins? I went with the few things that seemed obvious and necessary. We should try to reach consensus on that API surface ASAP. Thanks.

mklim · 2019-10-24T00:10:31Z

The facade API LG to me, I didn't notice anything missing. I could very likely be forgetting an edge case though.

tvolkert · 2019-10-24T01:32:19Z

API LGTM. We can always add more to the surface if needed later.

collinjackson

lgtm from a testing and Flutterfire perspective.

…saving callbacks are forwarded to plugins.

matthew-carroll · 2019-10-24T04:41:00Z

I added JVM tests to ensure that the new plugin facade references are not null when the binding is given to plugins. I also verified that state saving callbacks are forwarded to plugins that register for them.

matthew-carroll · 2019-10-24T21:47:20Z

@mklim would you mind taking a look at this PR? I'd like to merge this in before beginning work on creating a custom Lifecycle interface.

mklim

LGTM

mklim · 2019-10-24T21:50:49Z

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

    Log.d(TAG, "No preferred FlutterEngine was provided. Creating a new FlutterEngine for"
        + " this FlutterFragment.");
-    flutterEngine = new FlutterEngine(host.getContext());
+    flutterEngine = new FlutterEngine(host.getContext(), host.getFlutterShellArgs().toArray());


getFlutterShellArgs() is non-nullable, right?

mklim · 2019-10-24T22:01:23Z

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

+    cachedEngine.getPlugins().add(mockPlugin);
+
+    // Create a fake Host, which is required by the delegate.
+    FlutterActivityAndFragmentDelegate.Host mockHost = mock(FlutterActivityAndFragmentDelegate.Host.class);


readability nit: It would probably be better to create a stub than a mock here. Subjective but stubs are generally easier to reason about than a series of when...thenReturn() statements.

Sure. I switched out those mocks for fake implementations.

kroikie · 2019-10-24T22:09:02Z

LGTM from FlutterFire end.

amirh · 2019-10-25T19:33:39Z

shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityPluginBinding.java

+   * Returns the {@link Lifecycle} associated with the attached {@code Activity}.
+   */
+  @NonNull
+  Lifecycle getLifecycle();


Lifecycle should be fetched through the flutter_android_lifecycle plugin, and not available directly here

…acade, split Lifecycle, save state callbacks to plugins (flutter#43241, flutter#43242, flutter#43295) (flutter/engine#13280)

[email protected]:flutter/engine.git/compare/7a9c86b8d5e9...89731ae git log 7a9c86b..89731ae --no-merges --oneline 2019-10-25 [email protected] Intercept SystemSound.play platform message before it's sent. (flutter/engine#13342) 2019-10-25 [email protected] [fuchsia] [packaging] Layout debug symbols for Fuchsia (flutter/engine#13338) 2019-10-25 [email protected] Roll fuchsia/sdk/core/linux-amd64 from Dr9GE... to fHxWy... (flutter/engine#13358) 2019-10-25 [email protected] Roll fuchsia/sdk/core/mac-amd64 from KC8wX... to Tbg2V... (flutter/engine#13357) 2019-10-25 [email protected] Bump dart/language_model to 9fJQZ0TrnAGQKrEtuL3-AXbUfPzYxqpN_OBHr9P4hE4C (flutter/engine#13337) 2019-10-25 [email protected] Roll src/third_party/skia a7f3157ac012..d0a404e84d47 (6 commits) (flutter/engine#13356) 2019-10-25 [email protected] Roll src/third_party/dart d576ce69e1..5ba6fb73ec (3 commits) 2019-10-25 [email protected] Create a separate directory for the intermediate outputs of each Fuchsia archive build action (flutter/engine#13341) 2019-10-25 [email protected] Fix the output filename of the Fuchsia archive build template (flutter/engine#13339) 2019-10-25 [email protected] Roll src/third_party/dart b42c2af535..d576ce69e1 (10 commits) 2019-10-25 [email protected] Roll src/third_party/skia 24a409611f24..a7f3157ac012 (1 commits) (flutter/engine#13353) 2019-10-25 [email protected] Roll src/third_party/skia 28a8f28b3eaf..24a409611f24 (2 commits) (flutter/engine#13352) 2019-10-25 [email protected] Roll fuchsia/sdk/core/linux-amd64 from 9m5ec... to Dr9GE... (flutter/engine#13351) 2019-10-25 [email protected] Roll fuchsia/sdk/core/mac-amd64 from yuA3r... to KC8wX... (flutter/engine#13350) 2019-10-25 [email protected] Roll src/third_party/skia 6f1c20f01fa9..28a8f28b3eaf (2 commits) (flutter/engine#13348) 2019-10-25 [email protected] Expose platform view ID on embedder semantics node (flutter/engine#13345) 2019-10-25 [email protected] Remove TODO on embedder a11y unit tests (flutter/engine#13346) 2019-10-25 [email protected] Roll src/third_party/dart 1bc9fba660..b42c2af535 (12 commits) 2019-10-25 [email protected] Android embedding API updates for plugin ecosystem - plugin facade, split Lifecycle, save state callbacks to plugins (#43241, #43242, #43295) (flutter/engine#13280) 2019-10-24 [email protected] Roll src/third_party/skia 740f85949db2..6f1c20f01fa9 (16 commits) (flutter/engine#13343) 2019-10-24 [email protected] Roll src/third_party/dart 1bd6e20d76..1bc9fba660 (14 commits) 2019-10-24 [email protected] Roll src/third_party/skia 4ab4e641f151..740f85949db2 (12 commits) (flutter/engine#13336) 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

[email protected]:flutter/engine.git/compare/7a9c86b8d5e9...89731ae git log 7a9c86b..89731ae --no-merges --oneline 2019-10-25 [email protected] Intercept SystemSound.play platform message before it's sent. (flutter/engine#13342) 2019-10-25 [email protected] [fuchsia] [packaging] Layout debug symbols for Fuchsia (flutter/engine#13338) 2019-10-25 [email protected] Roll fuchsia/sdk/core/linux-amd64 from Dr9GE... to fHxWy... (flutter/engine#13358) 2019-10-25 [email protected] Roll fuchsia/sdk/core/mac-amd64 from KC8wX... to Tbg2V... (flutter/engine#13357) 2019-10-25 [email protected] Bump dart/language_model to 9fJQZ0TrnAGQKrEtuL3-AXbUfPzYxqpN_OBHr9P4hE4C (flutter/engine#13337) 2019-10-25 [email protected] Roll src/third_party/skia a7f3157ac012..d0a404e84d47 (6 commits) (flutter/engine#13356) 2019-10-25 [email protected] Roll src/third_party/dart d576ce69e1..5ba6fb73ec (3 commits) 2019-10-25 [email protected] Create a separate directory for the intermediate outputs of each Fuchsia archive build action (flutter/engine#13341) 2019-10-25 [email protected] Fix the output filename of the Fuchsia archive build template (flutter/engine#13339) 2019-10-25 [email protected] Roll src/third_party/dart b42c2af535..d576ce69e1 (10 commits) 2019-10-25 [email protected] Roll src/third_party/skia 24a409611f24..a7f3157ac012 (1 commits) (flutter/engine#13353) 2019-10-25 [email protected] Roll src/third_party/skia 28a8f28b3eaf..24a409611f24 (2 commits) (flutter/engine#13352) 2019-10-25 [email protected] Roll fuchsia/sdk/core/linux-amd64 from 9m5ec... to Dr9GE... (flutter/engine#13351) 2019-10-25 [email protected] Roll fuchsia/sdk/core/mac-amd64 from yuA3r... to KC8wX... (flutter/engine#13350) 2019-10-25 [email protected] Roll src/third_party/skia 6f1c20f01fa9..28a8f28b3eaf (2 commits) (flutter/engine#13348) 2019-10-25 [email protected] Expose platform view ID on embedder semantics node (flutter/engine#13345) 2019-10-25 [email protected] Remove TODO on embedder a11y unit tests (flutter/engine#13346) 2019-10-25 [email protected] Roll src/third_party/dart 1bc9fba660..b42c2af535 (12 commits) 2019-10-25 [email protected] Android embedding API updates for plugin ecosystem - plugin facade, split Lifecycle, save state callbacks to plugins (flutter#43241, flutter#43242, flutter#43295) (flutter/engine#13280) 2019-10-24 [email protected] Roll src/third_party/skia 740f85949db2..6f1c20f01fa9 (16 commits) (flutter/engine#13343) 2019-10-24 [email protected] Roll src/third_party/dart 1bd6e20d76..1bc9fba660 (14 commits) 2019-10-24 [email protected] Roll src/third_party/skia 4ab4e641f151..740f85949db2 (12 commits) (flutter/engine#13336) 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

Turned FlutterPluginBinding into a facade and deprecated getFlutterEn…

2f90351

…gine, split Lifecycle across ActivityAware and ServiceAware and deprected getLifecycle().

matthew-carroll requested review from amirh, blasten, mklim, tvolkert and xster October 22, 2019 05:43

googlebot added cla: yes labels Oct 22, 2019

Added state saving and restoring for plugins.

2ec56bf

collinjackson self-requested a review October 24, 2019 02:16

collinjackson approved these changes Oct 24, 2019

View reviewed changes

Added tests to ensure new plugin facade provides resources and state …

385b930

…saving callbacks are forwarded to plugins.

matthew-carroll marked this pull request as ready for review October 24, 2019 04:41

mklim approved these changes Oct 24, 2019

View reviewed changes

Changed Host from mock to fake.

5082f52

matthew-carroll merged commit 74d18bc into flutter:master Oct 25, 2019

This was referenced Oct 25, 2019

Move Lifecycle to ActivityAware and ServiceAware flutter/flutter#43295

Closed

Deprecate getFlutterEngine() and turn FlutterPluginBinding into a facade flutter/flutter#43242

Closed

Add state saving callbacks for plugins flutter/flutter#43241

Closed

amirh reviewed Oct 25, 2019

View reviewed changes

engine-flutter-autoroll mentioned this pull request Oct 25, 2019

Roll engine 7a9c86b8d5e9..89731aefca5b (22 commits) flutter/flutter#43541

Merged

amirh mentioned this pull request Oct 29, 2019

[flutter_plugin_android_lifecycle] Adapt the FlutterLifecycleAdapter to the new embedding API flutter/plugins#2243

Merged

13 tasks

gaaclarke mentioned this pull request Mar 4, 2023

Why was FlutterPluginBinding.getFlutterEngine() deprecated? flutter/flutter#121920

Open

Android embedding API updates for plugin ecosystem #13280

Android embedding API updates for plugin ecosystem #13280

Uh oh!

Conversation

matthew-carroll commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthew-carroll commented Oct 23, 2019

Uh oh!

mklim commented Oct 24, 2019

Uh oh!

tvolkert commented Oct 24, 2019

Uh oh!

collinjackson left a comment

Choose a reason for hiding this comment

Uh oh!

matthew-carroll commented Oct 24, 2019

Uh oh!

matthew-carroll commented Oct 24, 2019

Uh oh!

mklim left a comment

Choose a reason for hiding this comment

Uh oh!

mklim Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

matthew-carroll Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

mklim Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

matthew-carroll Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

kroikie commented Oct 24, 2019

Uh oh!

amirh Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

matthew-carroll commented Oct 22, 2019 •

edited

Loading