New Plugin API PR4: Adds Lifecycle support to the new plugin system.

[matthew-carroll]

New Plugin API PR4: Adds Lifecycle support to the new plugin system.

I still need to send in a real Lifecycle from FlutterFragment, but this PR creates the behavior within the new plugin system to respond to Lifecycle events.

This PR adds 2 Android libs to the engine:

• lifecycle-common-java8, which complements our existing lifecycle-common by adding one class that is needed for Java 8 compliance
• lifecycle-runtime, which was needed to utilize PluginRegistry.

Intended lifecycle behavior
When there is no backing lifecycle, e.g., Activity or Service, the FlutterEngine reports itself in the CREATED state. When either an Activity or a Service is attached to the FlutterEngine, the FlutterEngine defers to whatever the lifecycle of that component reports. The only caveat to this behavior is that even if the backing Activity or Service is destroyed, the FlutterEngine will not send a destruction event. This is because any Lifecycle that emits a destruction event is obligated to never send any future events, which would be inappropriate in our case.

[mklim]

LGTM

[mklim]

It's unexpected to me that onDestroyed would be special cased to the FlutterEngine specifically and that it's impossible to get an onDestroyed callback for the actual underlying lifecycle. Any way we could propagate this information life normal without breaking any kind of Engine contract? Add in an onEngineDestroyed method just for this class to handle the final engine destruction?

[matthew-carroll]

The contract that we're trying not to violate is the Lifecycle contract, not the FlutterEngine contract: https://developer.android.com/reference/android/arch/lifecycle/Lifecycle.State#destroyed

The unique thing about FlutterEngine is that its backing Lifecycle can change over time. It goes from Activity to Service to nothing to Activity again. There is nothing preventing us from forwarding an Activity's ON_DESTROY event, but then any other code that is designed to understand Lifecycle events will have undefined behavior when the FlutterEngine's Lifecycle turns right around and emits an ON_CREATED event. Additionally, it is also conceptually incorrect to send that ON_DESTROY event because even though the Activity has been destroyed, the FlutterEngine remains, and at the end of the day this Lifecycle is truly owned by the FlutterEngine, it just temporarily takes on the Lifecycle of various Android components.

Does that make sense?

[amirh]

I'm trying to use this lifecycle for the GoogleMaps plugin, when the plugin is initialized it needs to know the current state of the activity, drive the backing Google Map to the state, and forward activity lifecycle events to the backing Google Map.

I guess the way to do that with the current API is to make the plugin ActivityAware, and treat the "current" activity as "destroyed" if it's not attached and only if it is attached use the lifecycle from the plugin binding as the source of the activity lifecycle?

What do you think about providing a lifecycle object for the current "attached" activity as part of the ActivityAware interface, and a lifecycle object for the current service as part of ServiceAware instead of providing a single lifecycle object that may be backed by different lifecycles depending on the attachment state?

@mklim @matthew-carroll

[matthew-carroll]

The idea behind providing a single Lifecycle is that it shouldn't matter which component backs it. My guess is that most cases don't care, so splitting out multiple Lifecycle objects for Activity vs Service vs App ends up creating more work for the common case.

[amirh]

The concrete cases in flutter/plugins that need lifecycle (local_auth, and google maps) both care about the activity lifecycle. Do you have some samples about plugins that care about the lifecycle but don't care if it's activity/service?

I think it's worth providing an activity lifecycle object for ActivityAware plugins to keep the plugin code cleaner and less error prone, if it's indeed common to need the lifecycle of an "activity or a service" I guess we can provide both lifecycle objects.

[matthew-carroll]

Unfortunately I don't have concrete alternative examples because we just don't cover enough interesting use-cases in 1P at this point. But I would say, would sensors ever care which lifecycle they're listening to? e.g., BLE? Or how about analytics? Or databases? I could be wrong in my analysis, and I'm happy to reconsider, but I figured that most plugins over time will be used for non-UI purposes, because Flutter brings most UI capabilities and plugins need to fill in the non-UI details. For non-UI cases, it seems unlikely that a plugin would care about whether we're dealing with a Service or an Activity.

If we do want to give a difference lifecycle for ActivityAware, then I think we should bite the bullet and offer it for ActivityAware and ServiceAware and not offer one at all in the standard binding. However confusing it might appear now, it would be doubly confusing to have overlap between the Lifecycle in the standard binding and the Lifecycle in ActivityAware.

So I think at this point the question is, how do we effectively determine if an API change is in our best interest? I understand that it looks good in the 2 or so cases we've seen, but how can we ensure we do our due diligence for the general case? And if it is, how does this change impact the plugin extension/migration effort?

CC @mklim

[mklim]

If we do want to give a difference lifecycle for ActivityAware, then I think we should bite the bullet and offer it for ActivityAware and ServiceAware and not offer one at all in the standard binding.

This makes sense to me too.

So I think at this point the question is, how do we effectively determine if an API change is in our best interest? I understand that it looks good in the 2 or so cases we've seen, but how can we ensure we do our due diligence for the general case?

@matthew-carroll my understanding is that a driving reason behind migrating the 1P plugins now is that we want to test the v2 embedding API as soon as possible and make sure that it's covering all of the ecosystem's needs. Given a hypothetical argument for an API surface and actual concrete plugins needing it to expose a different service like we're seeing now, I think it makes more sense to change it to support the known real use cases.

And if it is, how does this change impact the plugin extension/migration effort?

This comes back to the versioning problem again. I think we probably want to hold off on migrating them for now and wait until the API change rolls into stable.

[matthew-carroll]

@mklim if we wait for it go stable then our "concrete" use-cases remain nearly as hypothetical as the hypothetical use-cases. The most difficult issues tend to be the ones that seem to appear from nowhere when we attempt to actually write the code. Our attempt to migrate these plugins may have shown an API improvement, but I have a difficult time trusting that improvement until we literally see it solve the problem...

[mklim]

@matthew-carroll I don't follow. In this case @amirh actively started writing out a PR now and identified a problem as he was working on a PR today, so I don't think the use case is hypothetical.

I do see how it's frustrating to have such a long tail on when we can use the API from its development. Flutter plugins try to guarantee support for Flutter stable as our minimum baseline. If we write the plugin to depend on a new API surface it'll be a compile time breakage unless we wait that long for it to roll, at least for as long as the v2 embedding is bundled into the Flutter SDK. Theoretically we could reflect on the new surface to avoid it being a compile time breakage, but that wouldn't help verify that the API surface better suits our use case. If we could depend on the v2 embedding outside of the Flutter SDK we wouldn't be limited to the latest stable roll.

[matthew-carroll]

The problem may be very real. But the solution is hypothetical until we apply it. There could be alternative solutions, right? And there could be weird Android race conditions that we won't see until we try it. This Lifecycle API surface seems to be the greatest cause for concern in the entire plugin surface. Just because we make an adjustment doesn't mean that we've shaken out all the issues. In fact, I think the fact that we want to make a significant change to it is a great reason to make sure we exercise the new solution thoroughly.

[amirh]

I'd suggest we first determine, given the information we have right now, what we think the API should look like.

After we've done that we can talk about timelines, migrations, and potential compromises.

[matthew-carroll]

Sure. Happy to talk about the API. It seems like we're all pretty close on that expectation, though. We can solidify the proposal, but then all of my concerns point towards us exercising that API.

[amirh]

To make sure I understand you correctly - you're theoretically support splitting into 2 lifecycle objects if we had no timeline considerations?

[matthew-carroll]

Yes, I'm open to giving a Lifecycle to ActivityAware and ServiceAware, and removing it from FlutterPluginBinding, if the ecosystem team believes that is a better experience for the entire ecosystem.

I am very concerned with making that change and not completely validating it before the next stable. I understand the rationale that we have to believe it will be OK, but this whole exercise is about direct validation. And I do believe in the importance of direct validation.

[mklim]

What if we wrote a PR tentatively using the new interface as soon as it was available and tested it locally with master, but waited on merging it until it was in stable? It's not perfect but that would at least locally validate the API before waiting for an entire roll cycle.

[matthew-carroll]

That's definitely a step in the right direction. There was previously a lot of discouragement about creating staging branches. But I don't have any problem with staging branches.