From 6924873cafc585809580f5a1441bd6ed37ce5fea Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Fri, 18 Sep 2020 13:04:10 -0700 Subject: [PATCH 01/12] Let FlutterActivity subclasses detach from engine --- .../embedding/android/FlutterActivity.java | 30 +++++++++++++++-- .../android/FlutterActivityTest.java | 33 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java index 22993f08fe58e..33d8ec5bfdffa 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java @@ -560,7 +560,11 @@ protected void onPause() { @Override protected void onStop() { super.onStop(); - delegate.onStop(); + if (delegate != null) { + delegate.onStop(); + } else { + Log.v(TAG, "FlutterActivity " + hashCode() + " onStop called after release."); + } lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_STOP); } @@ -570,11 +574,31 @@ protected void onSaveInstanceState(Bundle outState) { delegate.onSaveInstanceState(outState); } + /** + * Irreversibly release this activity's control of the {@link FlutterEngine} and its + * subcomponents. + * + * Calling will disconnect this activity's view from the Flutter renderer, disconnect this + * activity from plugins' {@link ActivityControlSurface}, and stop system channel messages from + * this activity. + * + * After calling, this activity should be disposed immediately and not be re-used. + */ + protected void release() { + delegate.onDestroyView(); + delegate.onDetach(); + delegate.release(); + delegate = null; + } + @Override protected void onDestroy() { super.onDestroy(); - delegate.onDestroyView(); - delegate.onDetach(); + if (delegate != null) { + release(); + } else { + Log.v(TAG, "FlutterActivity " + hashCode() + " onDestroy called after release."); + } lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY); } diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java index 67e5984fad0ec..0a5327a5d9600 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java @@ -6,6 +6,9 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.Context; @@ -158,6 +161,36 @@ public void itRegistersPluginsAtConfigurationTime() { assertEquals(activity.getFlutterEngine(), registeredEngines.get(0)); } + @Test + public void itCanBeManuallyReleasedBeforeOnDestroy() { + FlutterActivityAndFragmentDelegate mockDelegate = mock(FlutterActivityAndFragmentDelegate.class); + + Intent intent = FlutterActivity.withCachedEngine("my_cached_engine") + .build(RuntimeEnvironment.application); + ActivityController activityController = + Robolectric.buildActivity(FlutterActivity.class, intent); + FlutterActivity flutterActivity = activityController.get(); + flutterActivity.setDelegate(mockDelegate); + flutterActivity.onCreate(null); + flutterActivity.onStart(); + flutterActivity.onResume(); + + verify(mockDelegate, times(1)).onAttach(flutterActivity); + verify(mockDelegate, times(1)).onStart(); + verify(mockDelegate, times(1)).onResume(); + + flutterActivity.onPause(); + flutterActivity.release(); // Package visibility. + verify(mockDelegate, times(1)).onPause(); + + flutterActivity.onStop(); + flutterActivity.onDestroy(); + + verify(mockDelegate, never()).onStop(); + verify(mockDelegate, never()).onDestroyView(); + verify(mockDelegate, never()).onDetach(); + } + static class FlutterActivityWithProvidedEngine extends FlutterActivity { @Override protected void onCreate(@Nullable Bundle savedInstanceState) { From c9eaae6d6741e1ba5e09a9bee97c53ca50551d74 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Sat, 19 Sep 2020 23:45:38 -0700 Subject: [PATCH 02/12] Enforce exclusivity for activity and fragments attached to FlutterEngine --- shell/platform/android/BUILD.gn | 1 + .../android/ExclusiveAppComponent.java | 17 +++ .../embedding/android/FlutterActivity.java | 83 +++++++++--- .../FlutterActivityAndFragmentDelegate.java | 50 +++++-- .../embedding/android/FlutterFragment.java | 16 +++ .../embedding/android/FlutterView.java | 2 +- .../engine/FlutterEnginePluginRegistry.java | 126 ++++++++++++------ .../activity/ActivityControlSurface.java | 4 + .../platform/PlatformViewsController.java | 4 +- .../android/FlutterActivityTest.java | 7 +- 10 files changed, 234 insertions(+), 76 deletions(-) create mode 100644 shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 3443dba51d3ae..b6cd832a83e6f 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -128,6 +128,7 @@ android_java_sources = [ "io/flutter/embedding/android/AndroidKeyProcessor.java", "io/flutter/embedding/android/AndroidTouchProcessor.java", "io/flutter/embedding/android/DrawableSplashScreen.java", + "io/flutter/embedding/android/ExclusiveAppComponent.java", "io/flutter/embedding/android/FlutterActivity.java", "io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java", "io/flutter/embedding/android/FlutterActivityLaunchConfigs.java", diff --git a/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java b/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java new file mode 100644 index 0000000000000..e7e7c351e7645 --- /dev/null +++ b/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java @@ -0,0 +1,17 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package io.flutter.embedding.android; + +import androidx.annotation.NonNull; + +/** + * + * @param + */ +public interface ExclusiveAppComponent { + void detachFromFlutterEngine(); + + @NonNull T getAppComponent(); +} \ No newline at end of file diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java index 33d8ec5bfdffa..51b8392410957 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java @@ -126,14 +126,17 @@ * *

The following illustrates how to pre-warm and cache a {@link FlutterEngine}: * - *

{@code
- * // Create and pre-warm a FlutterEngine.
- * FlutterEngine flutterEngine = new FlutterEngine(context);
- * flutterEngine.getDartExecutor().executeDartEntrypoint(DartEntrypoint.createDefault());
+ * 
+ * {
+ *   @code
+ *   // Create and pre-warm a FlutterEngine.
+ *   FlutterEngine flutterEngine = new FlutterEngine(context);
+ *   flutterEngine.getDartExecutor().executeDartEntrypoint(DartEntrypoint.createDefault());
  *
- * // Cache the pre-warmed FlutterEngine in the FlutterEngineCache.
- * FlutterEngineCache.getInstance().put("my_engine", flutterEngine);
- * }
+ * // Cache the pre-warmed FlutterEngine in the FlutterEngineCache. + * FlutterEngineCache.getInstance().put("my_engine", flutterEngine); + * } + *
* *

Alternatives to FlutterActivity * @@ -185,7 +188,7 @@ * windowBackground} as the launch theme discussed previously. To use that splash screen, include * the following metadata in AndroidManifest.xml for this {@code FlutterActivity}: * - *

{@code {@code } * *

Alternative Activity {@link FlutterFragmentActivity} is also available, which @@ -193,8 +196,10 @@ * {@code FlutterActivity}, if possible, but if you need a {@code FragmentActivity} then you should * use {@link FlutterFragmentActivity}. */ -// A number of methods in this class have the same implementation as FlutterFragmentActivity. These -// methods are duplicated for readability purposes. Be sure to replicate any change in this class in +// A number of methods in this class have the same implementation as +// FlutterFragmentActivity. These +// methods are duplicated for readability purposes. Be sure to replicate any +// change in this class in // FlutterFragmentActivity, too. public class FlutterActivity extends Activity implements FlutterActivityAndFragmentDelegate.Host, LifecycleOwner { @@ -373,7 +378,8 @@ public Intent build(@NonNull Context context) { } // Delegate that runs all lifecycle and OS hook logic that is common between - // FlutterActivity and FlutterFragment. See the FlutterActivityAndFragmentDelegate + // FlutterActivity and FlutterFragment. See the + // FlutterActivityAndFragmentDelegate // implementation for details about why it exists. @VisibleForTesting protected FlutterActivityAndFragmentDelegate delegate; @@ -402,11 +408,15 @@ public FlutterActivity() { protected void onCreate(@Nullable Bundle savedInstanceState) { switchLaunchThemeForNormalTheme(); + Log.e("meh", "Activity " + this + " onCreate"); + super.onCreate(savedInstanceState); + Log.e("meh", "Activity " + this + " super oncreate finished"); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_CREATE); delegate = new FlutterActivityAndFragmentDelegate(this); + Log.e("meh", "Activity " + this + " created delegate"); delegate.onAttach(this); delegate.onActivityCreated(savedInstanceState); @@ -415,6 +425,15 @@ protected void onCreate(@Nullable Bundle savedInstanceState) { configureStatusBarForFullscreenFlutterExperience(); } + @Override + protected void onRestart() { + super.onRestart(); + Log.e("meh", "Activity " + this + " onRestart"); + delegate = new FlutterActivityAndFragmentDelegate(this); + delegate.onAttach(this); + delegate.onActivityCreated(null); + } + /** * Switches themes for this {@code Activity} from the theme used to launch this {@code Activity} * to a "normal theme" that is intended for regular {@code Activity} operation. @@ -533,6 +552,7 @@ private void configureStatusBarForFullscreenFlutterExperience() { @Override protected void onStart() { super.onStart(); + Log.e("meh", "Activity " + this + " onStart"); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_START); delegate.onStart(); } @@ -540,6 +560,7 @@ protected void onStart() { @Override protected void onResume() { super.onResume(); + Log.e("meh", "Activity " + this + " onResume"); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_RESUME); delegate.onResume(); } @@ -553,6 +574,7 @@ public void onPostResume() { @Override protected void onPause() { super.onPause(); + Log.e("meh", "Activity " + this + " onPause"); delegate.onPause(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_PAUSE); } @@ -560,10 +582,11 @@ protected void onPause() { @Override protected void onStop() { super.onStop(); + Log.e("meh", "Activity " + this + " onStop"); if (delegate != null) { delegate.onStop(); } else { - Log.v(TAG, "FlutterActivity " + hashCode() + " onStop called after release."); + Log.v(TAG, "FlutterActivity " + this + " onStop called after release."); } lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_STOP); } @@ -571,33 +594,50 @@ protected void onStop() { @Override protected void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); - delegate.onSaveInstanceState(outState); + if (delegate != null) { + delegate.onSaveInstanceState(outState); + } else { + Log.v(TAG, "FlutterActivity " + this + " onSaveInstanceState called after release."); + } } /** * Irreversibly release this activity's control of the {@link FlutterEngine} and its * subcomponents. * - * Calling will disconnect this activity's view from the Flutter renderer, disconnect this + *

Calling will disconnect this activity's view from the Flutter renderer, disconnect this * activity from plugins' {@link ActivityControlSurface}, and stop system channel messages from * this activity. * - * After calling, this activity should be disposed immediately and not be re-used. + *

After calling, this activity should be disposed immediately and not be re-used. */ - protected void release() { + private void release() { delegate.onDestroyView(); delegate.onDetach(); delegate.release(); delegate = null; } + @Override + public void detachFromFlutterEngine() { + Log.v( + TAG, + "FlutterActivity " + + this + + " connection to the engine " + + getFlutterEngine() + + " evicted by another attaching activity"); + release(); + } + @Override protected void onDestroy() { super.onDestroy(); + Log.e("meh", "Activity " + this + " onDestroy"); if (delegate != null) { release(); } else { - Log.v(TAG, "FlutterActivity " + hashCode() + " onDestroy called after release."); + Log.v(TAG, "FlutterActivity " + this + " onDestroy called after release."); } lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY); } @@ -792,7 +832,8 @@ public String getInitialRoute() { public String getAppBundlePath() { // If this Activity was launched from tooling, and the incoming Intent contains // a custom app bundle path, return that path. - // TODO(mattcarroll): determine if we should have an explicit FlutterTestActivity instead of + // TODO(mattcarroll): determine if we should have an explicit + // FlutterTestActivity instead of // conflating. if (isDebuggable() && Intent.ACTION_RUN.equals(getIntent().getAction())) { String appBundlePath = getIntent().getDataString(); @@ -960,7 +1001,8 @@ public void onFlutterTextureViewCreated(@NonNull FlutterTextureView flutterTextu @Override public void onFlutterUiDisplayed() { - // Notifies Android that we're fully drawn so that performance metrics can be collected by + // Notifies Android that we're fully drawn so that performance metrics can be + // collected by // Flutter performance tests. // This was supported in KitKat (API 19), but has a bug around requiring // permissions. See https://github.com/flutter/flutter/issues/46172 @@ -980,7 +1022,8 @@ public boolean shouldRestoreAndSaveState() { return getIntent().getBooleanExtra(EXTRA_ENABLE_STATE_RESTORATION, false); } if (getCachedEngineId() != null) { - // Prevent overwriting the existing state in a cached engine with restoration state. + // Prevent overwriting the existing state in a cached engine with restoration + // state. return false; } return true; diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java index e4cad301c21a0..e16360147c5fc 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java @@ -62,7 +62,8 @@ * the same form. Do not use this class as a convenient shortcut for any other * behavior. */ -/* package */ final class FlutterActivityAndFragmentDelegate { +/* package */ final class FlutterActivityAndFragmentDelegate + implements ExclusiveAppComponent { private static final String TAG = "FlutterActivityAndFragmentDelegate"; private static final String FRAMEWORK_RESTORATION_BUNDLE_KEY = "framework"; private static final String PLUGINS_RESTORATION_BUNDLE_KEY = "plugins"; @@ -154,14 +155,6 @@ void onAttach(@NonNull Context context) { setupFlutterEngine(); } - // Regardless of whether or not a FlutterEngine already existed, the PlatformPlugin - // is bound to a specific Activity. Therefore, it needs to be created and configured - // every time this Fragment attaches to a new Activity. - // TODO(mattcarroll): the PlatformPlugin needs to be reimagined because it implicitly takes - // control of the entire window. This is unacceptable for non-fullscreen - // use-cases. - platformPlugin = host.providePlatformPlugin(host.getActivity(), flutterEngine); - if (host.shouldAttachEngineToActivity()) { // Notify any plugins that are currently attached to our FlutterEngine that they // are now attached to an Activity. @@ -173,14 +166,31 @@ void onAttach(@NonNull Context context) { // sync with the Activity. We use the Fragment's Lifecycle because it is possible that the // attached Activity is not a LifecycleOwner. Log.v(TAG, "Attaching FlutterEngine to the Activity that owns this Fragment."); - flutterEngine - .getActivityControlSurface() - .attachToActivity(host.getActivity(), host.getLifecycle()); + flutterEngine.getActivityControlSurface().attachToActivity(this, host.getLifecycle()); } + // Regardless of whether or not a FlutterEngine already existed, the PlatformPlugin + // is bound to a specific Activity. Therefore, it needs to be created and configured + // every time this Fragment attaches to a new Activity. + // TODO(mattcarroll): the PlatformPlugin needs to be reimagined because it implicitly takes + // control of the entire window. This is unacceptable for non-fullscreen + // use-cases. + platformPlugin = host.providePlatformPlugin(host.getActivity(), flutterEngine); + host.configureFlutterEngine(flutterEngine); } + @Override + public @NonNull Activity getAppComponent() { + final Activity activity = host.getActivity(); + if (activity == null) { + throw new AssertionError( + "FlutterActivityAndFragmentDelegate's getAppComponent should only " + + "be queried after onAttach, when the host's activity should always be non-null"); + } + return host.getActivity(); + } + /** * Obtains a reference to a FlutterEngine to back this delegate and its {@code host}. * @@ -480,6 +490,20 @@ void onSaveInstanceState(@Nullable Bundle bundle) { } } + @Override + public void detachFromFlutterEngine() { + if (host.shouldDestroyEngineWithHost()) { + // The host owns the engine and should never have its engine taken by another exclusive + // activity. + throw new AssertionError(""); + } + + // Default, but customizable, behavior is for the host to call {@link #onDetach} + // deterministically + // as to not mix more events during the lifecycle of the next exclusive activity. + host.detachFromFlutterEngine(); + } + /** * Invoke this from {@code Activity#onDestroy()} or {@code Fragment#onDetach()}. * @@ -741,6 +765,8 @@ private void ensureAlive() { */ boolean shouldDestroyEngineWithHost(); + void detachFromFlutterEngine(); + /** Returns the Dart entrypoint that should run when a new {@link FlutterEngine} is created. */ @NonNull String getDartEntrypointFunctionName(); diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java index d64b6788e71a6..a473f4ad5e0c9 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java @@ -638,6 +638,22 @@ public void onSaveInstanceState(Bundle outState) { delegate.onSaveInstanceState(outState); } + @Override + public void detachFromFlutterEngine() { + Log.v( + TAG, + "FlutterFragment " + + this + + " connection to the engine " + + getFlutterEngine() + + " evicted by another attaching activity"); + // Redundant calls are ok. + delegate.onDestroyView(); + delegate.onDetach(); + delegate.release(); + delegate = null; + } + @Override public void onDetach() { super.onDetach(); diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 2f1942034a7e5..dde3dbaecdd00 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -967,7 +967,7 @@ public void attachToFlutterEngine(@NonNull FlutterEngine flutterEngine) { public void detachFromFlutterEngine() { Log.v(TAG, "Detaching from a FlutterEngine: " + flutterEngine); if (!isAttachedToFlutterEngine()) { - Log.v(TAG, "Not attached to an engine. Doing nothing."); + Log.v(TAG, "FlutterView not attached to an engine. Not detaching."); return; } diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java index 7e9d39b30752d..eadc706c32747 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java @@ -15,6 +15,7 @@ import androidx.annotation.Nullable; import androidx.lifecycle.Lifecycle; import io.flutter.Log; +import io.flutter.embedding.android.ExclusiveAppComponent; import io.flutter.embedding.engine.loader.FlutterLoader; import io.flutter.embedding.engine.plugins.FlutterPlugin; import io.flutter.embedding.engine.plugins.PluginRegistry; @@ -36,7 +37,7 @@ import java.util.Map; import java.util.Set; -class FlutterEnginePluginRegistry +/* package */ class FlutterEnginePluginRegistry implements PluginRegistry, ActivityControlSurface, ServiceControlSurface, @@ -58,6 +59,7 @@ class FlutterEnginePluginRegistry new HashMap<>(); @Nullable private Activity activity; + @Nullable private ExclusiveAppComponent exclusiveActivity; @Nullable private FlutterEngineActivityPluginBinding activityPluginBinding; private boolean isWaitingForActivityReattachment = false; @@ -102,11 +104,14 @@ class FlutterEnginePluginRegistry public void destroy() { Log.v(TAG, "Destroying."); - // Detach from any Android component that we may currently be attached to, e.g., Activity, + // Detach from any Android component that we may currently be attached to, e.g., + // Activity, // Service, - // BroadcastReceiver, ContentProvider. This must happen before removing all plugins so that the - // plugins have an opportunity to clean up references as a result of component detachment. - detachFromAndroidComponent(); + // BroadcastReceiver, ContentProvider. This must happen before removing all + // plugins so that the + // plugins have an opportunity to clean up references as a result of component + // detachment. + detachFromAppComponent(); // Remove all registered plugins. removeAll(); @@ -156,9 +161,11 @@ public void add(@NonNull FlutterPlugin plugin) { } } - // For BroadcastReceiverAware plugins, add the plugin to our set of BroadcastReceiverAware + // For BroadcastReceiverAware plugins, add the plugin to our set of + // BroadcastReceiverAware // plugins, and if this engine is currently attached to a BroadcastReceiver, - // notify the BroadcastReceiverAware plugin that it is now attached to a BroadcastReceiver. + // notify the BroadcastReceiverAware plugin that it is now attached to a + // BroadcastReceiver. if (plugin instanceof BroadcastReceiverAware) { BroadcastReceiverAware broadcastReceiverAware = (BroadcastReceiverAware) plugin; broadcastReceiverAwarePlugins.put(plugin.getClass(), broadcastReceiverAware); @@ -168,9 +175,11 @@ public void add(@NonNull FlutterPlugin plugin) { } } - // For ContentProviderAware plugins, add the plugin to our set of ContentProviderAware + // For ContentProviderAware plugins, add the plugin to our set of + // ContentProviderAware // plugins, and if this engine is currently attached to a ContentProvider, - // notify the ContentProviderAware plugin that it is now attached to a ContentProvider. + // notify the ContentProviderAware plugin that it is now attached to a + // ContentProvider. if (plugin instanceof ContentProviderAware) { ContentProviderAware contentProviderAware = (ContentProviderAware) plugin; contentProviderAwarePlugins.put(plugin.getClass(), contentProviderAware); @@ -225,8 +234,10 @@ public void remove(@NonNull Class pluginClass) { serviceAwarePlugins.remove(pluginClass); } - // For BroadcastReceiverAware plugins, notify the plugin that it is detached from - // a BroadcastReceiver if a BroadcastReceiver is currently attached to this engine. Then + // For BroadcastReceiverAware plugins, notify the plugin that it is detached + // from + // a BroadcastReceiver if a BroadcastReceiver is currently attached to this + // engine. Then // remove the plugin from our set of BroadcastReceiverAware plugins. if (plugin instanceof BroadcastReceiverAware) { if (isAttachedToBroadcastReceiver()) { @@ -237,7 +248,8 @@ public void remove(@NonNull Class pluginClass) { } // For ContentProviderAware plugins, notify the plugin that it is detached from - // a ContentProvider if a ContentProvider is currently attached to this engine. Then + // a ContentProvider if a ContentProvider is currently attached to this engine. + // Then // remove the plugin from our set of ContentProviderAware plugins. if (plugin instanceof ContentProviderAware) { if (isAttachedToContentProvider()) { @@ -269,7 +281,7 @@ public void removeAll() { plugins.clear(); } - private void detachFromAndroidComponent() { + private void detachFromAppComponent() { if (isAttachedToActivity()) { detachFromActivity(); } else if (isAttachedToService()) { @@ -283,7 +295,11 @@ private void detachFromAndroidComponent() { // -------- Start ActivityControlSurface ------- private boolean isAttachedToActivity() { - return activity != null; + return activity != null || exclusiveActivity != null; + } + + private Activity attachedActivity() { + return exclusiveActivity != null ? exclusiveActivity.getAppComponent() : activity; } @Override @@ -294,20 +310,42 @@ public void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle life + activity + "." + (isWaitingForActivityReattachment ? " This is after a config change." : "")); - // If we were already attached to an Android component, detach from it. - detachFromAndroidComponent(); + // If we were already attached to an app component, detach from it. + detachFromAppComponent(); this.activity = activity; + attachToActivityInternal(activity, lifecycle); + } + + @Override + public void attachToActivity( + @NonNull ExclusiveAppComponent exclusiveActivity, @NonNull Lifecycle lifecycle) { + Log.v( + TAG, + "Attaching to an exclusive Activity: " + + exclusiveActivity.getAppComponent() + + (isAttachedToActivity() ? " evicting previous activity " + attachedActivity() : "") + + "." + + (isWaitingForActivityReattachment ? " This is after a config change." : "")); + // If we were already attached to an app component, detach from it. + detachFromAppComponent(); + + this.exclusiveActivity = exclusiveActivity; + attachToActivityInternal(exclusiveActivity.getAppComponent(), lifecycle); + } + + private void attachToActivityInternal(@NonNull Activity activity, @NonNull Lifecycle lifecycle) { this.activityPluginBinding = new FlutterEngineActivityPluginBinding(activity, lifecycle); - // Activate the PlatformViewsController. This must happen before any plugins attempt - // to use it, otherwise an error stack trace will appear that says there is no + // Activate the PlatformViewsController. This must happen before any plugins + // attempt to use it, otherwise an error stack trace will appear that says there is no // flutter/platform_views channel. flutterEngine .getPlatformViewsController() .attach(activity, flutterEngine.getRenderer(), flutterEngine.getDartExecutor()); - // Notify all ActivityAware plugins that they are now attached to a new Activity. + // Notify all ActivityAware plugins that they are now attached to a new + // Activity. for (ActivityAware activityAware : activityAwarePlugins.values()) { if (isWaitingForActivityReattachment) { activityAware.onReattachedToActivityForConfigChanges(activityPluginBinding); @@ -321,18 +359,14 @@ public void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle life @Override public void detachFromActivityForConfigChanges() { if (isAttachedToActivity()) { - Log.v(TAG, "Detaching from an Activity for config changes: " + activity); + Log.v(TAG, "Detaching from an Activity for config changes: " + attachedActivity()); isWaitingForActivityReattachment = true; for (ActivityAware activityAware : activityAwarePlugins.values()) { activityAware.onDetachedFromActivityForConfigChanges(); } - // Deactivate PlatformViewsController. - flutterEngine.getPlatformViewsController().detach(); - - activity = null; - activityPluginBinding = null; + detachFromActivityInternal(); } else { Log.e(TAG, "Attempted to detach plugins from an Activity when no Activity was attached."); } @@ -341,21 +375,30 @@ public void detachFromActivityForConfigChanges() { @Override public void detachFromActivity() { if (isAttachedToActivity()) { - Log.v(TAG, "Detaching from an Activity: " + activity); + Log.v(TAG, "Detaching from an Activity: " + attachedActivity()); for (ActivityAware activityAware : activityAwarePlugins.values()) { activityAware.onDetachedFromActivity(); } - // Deactivate PlatformViewsController. - flutterEngine.getPlatformViewsController().detach(); - - activity = null; - activityPluginBinding = null; + detachFromActivityInternal(); } else { Log.e(TAG, "Attempted to detach plugins from an Activity when no Activity was attached."); } } + private void detachFromActivityInternal() { + // Deactivate PlatformViewsController. + flutterEngine.getPlatformViewsController().detach(); + + if (exclusiveActivity != null) { + final ExclusiveAppComponent detachingActivity = exclusiveActivity; + exclusiveActivity = null; + detachingActivity.detachFromFlutterEngine(); + } + activity = null; + activityPluginBinding = null; + } + @Override public boolean onRequestPermissionsResult( int requestCode, @NonNull String[] permissions, @NonNull int[] grantResult) { @@ -443,7 +486,7 @@ public void attachToService( @NonNull Service service, @Nullable Lifecycle lifecycle, boolean isForeground) { Log.v(TAG, "Attaching to a Service: " + service); // If we were already attached to an Android component, detach from it. - detachFromAndroidComponent(); + detachFromAppComponent(); this.service = service; this.servicePluginBinding = new FlutterEngineServicePluginBinding(service, lifecycle); @@ -458,7 +501,8 @@ public void attachToService( public void detachFromService() { if (isAttachedToService()) { Log.v(TAG, "Detaching from a Service: " + service); - // Notify all ServiceAware plugins that they are no longer attached to a Service. + // Notify all ServiceAware plugins that they are no longer attached to a + // Service. for (ServiceAware serviceAware : serviceAwarePlugins.values()) { serviceAware.onDetachedFromService(); } @@ -497,12 +541,13 @@ public void attachToBroadcastReceiver( @NonNull BroadcastReceiver broadcastReceiver, @NonNull Lifecycle lifecycle) { Log.v(TAG, "Attaching to BroadcastReceiver: " + broadcastReceiver); // If we were already attached to an Android component, detach from it. - detachFromAndroidComponent(); + detachFromAppComponent(); this.broadcastReceiver = broadcastReceiver; this.broadcastReceiverPluginBinding = new FlutterEngineBroadcastReceiverPluginBinding(broadcastReceiver); - // TODO(mattcarroll): resolve possibility of different lifecycles between this and engine + // TODO(mattcarroll): resolve possibility of different lifecycles between this + // and engine // attachment // Notify all BroadcastReceiverAware plugins that they are now attached to a new @@ -516,7 +561,8 @@ public void attachToBroadcastReceiver( public void detachFromBroadcastReceiver() { if (isAttachedToBroadcastReceiver()) { Log.v(TAG, "Detaching from BroadcastReceiver: " + broadcastReceiver); - // Notify all BroadcastReceiverAware plugins that they are no longer attached to a + // Notify all BroadcastReceiverAware plugins that they are no longer attached to + // a // BroadcastReceiver. for (BroadcastReceiverAware broadcastReceiverAware : broadcastReceiverAwarePlugins.values()) { broadcastReceiverAware.onDetachedFromBroadcastReceiver(); @@ -539,15 +585,17 @@ public void attachToContentProvider( @NonNull ContentProvider contentProvider, @NonNull Lifecycle lifecycle) { Log.v(TAG, "Attaching to ContentProvider: " + contentProvider); // If we were already attached to an Android component, detach from it. - detachFromAndroidComponent(); + detachFromAppComponent(); this.contentProvider = contentProvider; this.contentProviderPluginBinding = new FlutterEngineContentProviderPluginBinding(contentProvider); - // TODO(mattcarroll): resolve possibility of different lifecycles between this and engine + // TODO(mattcarroll): resolve possibility of different lifecycles between this + // and engine // attachment - // Notify all ContentProviderAware plugins that they are now attached to a new ContentProvider. + // Notify all ContentProviderAware plugins that they are now attached to a new + // ContentProvider. for (ContentProviderAware contentProviderAware : contentProviderAwarePlugins.values()) { contentProviderAware.onAttachedToContentProvider(contentProviderPluginBinding); } diff --git a/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java b/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java index 671f8311f4827..2b7a57a090a6b 100644 --- a/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java +++ b/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java @@ -10,6 +10,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.lifecycle.Lifecycle; +import io.flutter.embedding.android.ExclusiveAppComponent; /** * Control surface through which an {@link Activity} attaches to a {@link FlutterEngine}. @@ -51,6 +52,9 @@ public interface ActivityControlSurface { */ void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle lifecycle); + void attachToActivity( + @NonNull ExclusiveAppComponent exclusiveActivity, @NonNull Lifecycle lifecycle); + /** * Call this method from the {@link Activity} that is attached to this {@code * ActivityControlSurfaces}'s {@link FlutterEngine} when the {@link Activity} is about to be diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 75d724605c69d..c7de3c2c0ec6c 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -460,7 +460,9 @@ public void attach( */ @UiThread public void detach() { - platformViewsChannel.setPlatformViewsHandler(null); + if (platformViewsChannel != null) { + platformViewsChannel.setPlatformViewsHandler(null); + } platformViewsChannel = null; context = null; textureRegistry = null; diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java index 0a5327a5d9600..dc28202229f58 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java @@ -163,10 +163,11 @@ public void itRegistersPluginsAtConfigurationTime() { @Test public void itCanBeManuallyReleasedBeforeOnDestroy() { - FlutterActivityAndFragmentDelegate mockDelegate = mock(FlutterActivityAndFragmentDelegate.class); + FlutterActivityAndFragmentDelegate mockDelegate = + mock(FlutterActivityAndFragmentDelegate.class); - Intent intent = FlutterActivity.withCachedEngine("my_cached_engine") - .build(RuntimeEnvironment.application); + Intent intent = + FlutterActivity.withCachedEngine("my_cached_engine").build(RuntimeEnvironment.application); ActivityController activityController = Robolectric.buildActivity(FlutterActivity.class, intent); FlutterActivity flutterActivity = activityController.get(); From 501f8860b6198f260b4658498530d2996c82c488 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Sun, 20 Sep 2020 00:44:13 -0700 Subject: [PATCH 03/12] docs --- .../android/ExclusiveAppComponent.java | 20 ++++++- .../embedding/android/FlutterActivity.java | 56 +++++-------------- .../FlutterActivityAndFragmentDelegate.java | 17 ++++-- .../android/FlutterEngineConfigurator.java | 5 +- .../embedding/android/FlutterFragment.java | 32 ++++++++--- .../engine/FlutterEnginePluginRegistry.java | 52 ++++++----------- .../activity/ActivityControlSurface.java | 30 +++++++++- 7 files changed, 118 insertions(+), 94 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java b/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java index e7e7c351e7645..77500d283ceff 100644 --- a/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java +++ b/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java @@ -7,11 +7,29 @@ import androidx.annotation.NonNull; /** + * An Android App Component exclusively attached to a {@link io.flutter.embedding.engine.FlutterEngine}. * - * @param + * An exclusive App Component's {@link #detachFromFlutterEngine} is invoked when another + * App Component is becoming attached to the {@link io.flutter.embedding.engine.FlutterEngine}. + * + * The term "App Component" refer to the 4 component types: Activity, Service, Broadcast Receiver, + * and Content Provider, as defined in https://developer.android.com/guide/components/fundamentals. + * + * @param The App Component behind this exclusive App Component. */ public interface ExclusiveAppComponent { + /** + * Called when another App Component is about to become attached to the + * {@link io.flutter.embedding.engine.FlutterEngine} this App Component is currently attached + * to. + * + * This App Component's connections to the {@link io.flutter.embedding.engine.FlutterEngine} are + * still valid at the moment of this call. + */ void detachFromFlutterEngine(); + /** + * Retrieve the App Component behind this exclusive App Component. + */ @NonNull T getAppComponent(); } \ No newline at end of file diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java index 51b8392410957..0ad28010898f8 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java @@ -126,17 +126,14 @@ * *

The following illustrates how to pre-warm and cache a {@link FlutterEngine}: * - *

- * {
- *   @code
- *   // Create and pre-warm a FlutterEngine.
- *   FlutterEngine flutterEngine = new FlutterEngine(context);
- *   flutterEngine.getDartExecutor().executeDartEntrypoint(DartEntrypoint.createDefault());
+ * 
{@code
+ * // Create and pre-warm a FlutterEngine.
+ * FlutterEngine flutterEngine = new FlutterEngine(context);
+ * flutterEngine.getDartExecutor().executeDartEntrypoint(DartEntrypoint.createDefault());
  *
- *   // Cache the pre-warmed FlutterEngine in the FlutterEngineCache.
- *   FlutterEngineCache.getInstance().put("my_engine", flutterEngine);
- * }
- * 
+ * // Cache the pre-warmed FlutterEngine in the FlutterEngineCache. + * FlutterEngineCache.getInstance().put("my_engine", flutterEngine); + * }
* *

Alternatives to FlutterActivity * @@ -188,7 +185,7 @@ * windowBackground} as the launch theme discussed previously. To use that splash screen, include * the following metadata in AndroidManifest.xml for this {@code FlutterActivity}: * - *

{@code {@code } * *

Alternative Activity {@link FlutterFragmentActivity} is also available, which @@ -196,10 +193,8 @@ * {@code FlutterActivity}, if possible, but if you need a {@code FragmentActivity} then you should * use {@link FlutterFragmentActivity}. */ -// A number of methods in this class have the same implementation as -// FlutterFragmentActivity. These -// methods are duplicated for readability purposes. Be sure to replicate any -// change in this class in +// A number of methods in this class have the same implementation as FlutterFragmentActivity. These +// methods are duplicated for readability purposes. Be sure to replicate any change in this class in // FlutterFragmentActivity, too. public class FlutterActivity extends Activity implements FlutterActivityAndFragmentDelegate.Host, LifecycleOwner { @@ -378,8 +373,7 @@ public Intent build(@NonNull Context context) { } // Delegate that runs all lifecycle and OS hook logic that is common between - // FlutterActivity and FlutterFragment. See the - // FlutterActivityAndFragmentDelegate + // FlutterActivity and FlutterFragment. See the FlutterActivityAndFragmentDelegate // implementation for details about why it exists. @VisibleForTesting protected FlutterActivityAndFragmentDelegate delegate; @@ -408,15 +402,11 @@ public FlutterActivity() { protected void onCreate(@Nullable Bundle savedInstanceState) { switchLaunchThemeForNormalTheme(); - Log.e("meh", "Activity " + this + " onCreate"); - super.onCreate(savedInstanceState); - Log.e("meh", "Activity " + this + " super oncreate finished"); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_CREATE); delegate = new FlutterActivityAndFragmentDelegate(this); - Log.e("meh", "Activity " + this + " created delegate"); delegate.onAttach(this); delegate.onActivityCreated(savedInstanceState); @@ -425,15 +415,6 @@ protected void onCreate(@Nullable Bundle savedInstanceState) { configureStatusBarForFullscreenFlutterExperience(); } - @Override - protected void onRestart() { - super.onRestart(); - Log.e("meh", "Activity " + this + " onRestart"); - delegate = new FlutterActivityAndFragmentDelegate(this); - delegate.onAttach(this); - delegate.onActivityCreated(null); - } - /** * Switches themes for this {@code Activity} from the theme used to launch this {@code Activity} * to a "normal theme" that is intended for regular {@code Activity} operation. @@ -552,7 +533,6 @@ private void configureStatusBarForFullscreenFlutterExperience() { @Override protected void onStart() { super.onStart(); - Log.e("meh", "Activity " + this + " onStart"); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_START); delegate.onStart(); } @@ -560,7 +540,6 @@ protected void onStart() { @Override protected void onResume() { super.onResume(); - Log.e("meh", "Activity " + this + " onResume"); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_RESUME); delegate.onResume(); } @@ -574,7 +553,6 @@ public void onPostResume() { @Override protected void onPause() { super.onPause(); - Log.e("meh", "Activity " + this + " onPause"); delegate.onPause(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_PAUSE); } @@ -582,7 +560,6 @@ protected void onPause() { @Override protected void onStop() { super.onStop(); - Log.e("meh", "Activity " + this + " onStop"); if (delegate != null) { delegate.onStop(); } else { @@ -832,8 +809,7 @@ public String getInitialRoute() { public String getAppBundlePath() { // If this Activity was launched from tooling, and the incoming Intent contains // a custom app bundle path, return that path. - // TODO(mattcarroll): determine if we should have an explicit - // FlutterTestActivity instead of + // TODO(mattcarroll): determine if we should have an explicit FlutterTestActivity instead of // conflating. if (isDebuggable() && Intent.ACTION_RUN.equals(getIntent().getAction())) { String appBundlePath = getIntent().getDataString(); @@ -973,7 +949,7 @@ public void cleanUpFlutterEngine(@NonNull FlutterEngine flutterEngine) { *

Returning false from this method does not preclude a {@link FlutterEngine} from being * attaching to a {@code FlutterActivity} - it just prevents the attachment from happening * automatically. A developer can choose to subclass {@code FlutterActivity} and then invoke - * {@link ActivityControlSurface#attachToActivity(Activity, Lifecycle)} and {@link + * {@link ActivityControlSurface#attachToActivity(ExclusiveAppComponent, Lifecycle)} and {@link * ActivityControlSurface#detachFromActivity()} at the desired times. * *

One reason that a developer might choose to manually manage the relationship between the @@ -1001,8 +977,7 @@ public void onFlutterTextureViewCreated(@NonNull FlutterTextureView flutterTextu @Override public void onFlutterUiDisplayed() { - // Notifies Android that we're fully drawn so that performance metrics can be - // collected by + // Notifies Android that we're fully drawn so that performance metrics can be collected by // Flutter performance tests. // This was supported in KitKat (API 19), but has a bug around requiring // permissions. See https://github.com/flutter/flutter/issues/46172 @@ -1022,8 +997,7 @@ public boolean shouldRestoreAndSaveState() { return getIntent().getBooleanExtra(EXTRA_ENABLE_STATE_RESTORATION, false); } if (getCachedEngineId() != null) { - // Prevent overwriting the existing state in a cached engine with restoration - // state. + // Prevent overwriting the existing state in a cached engine with restoration state. return false; } return true; diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java index e16360147c5fc..99685f4ed519a 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java @@ -165,7 +165,7 @@ void onAttach(@NonNull Context context) { // which means there shouldn't be any possibility for the Fragment Lifecycle to get out of // sync with the Activity. We use the Fragment's Lifecycle because it is possible that the // attached Activity is not a LifecycleOwner. - Log.v(TAG, "Attaching FlutterEngine to the Activity that owns this Fragment."); + Log.v(TAG, "Attaching FlutterEngine to the Activity that owns this delegate."); flutterEngine.getActivityControlSurface().attachToActivity(this, host.getLifecycle()); } @@ -495,12 +495,14 @@ public void detachFromFlutterEngine() { if (host.shouldDestroyEngineWithHost()) { // The host owns the engine and should never have its engine taken by another exclusive // activity. - throw new AssertionError(""); + throw new AssertionError("The internal FlutterEngine created by " + host + + " has been attached to by another activity. To persist a FlutterEngine beyond the " + + "ownership of this activity, explicitly create a FlutterEngine"; } // Default, but customizable, behavior is for the host to call {@link #onDetach} - // deterministically - // as to not mix more events during the lifecycle of the next exclusive activity. + // deterministically as to not mix more events during the lifecycle of the next exclusive + // activity. host.detachFromFlutterEngine(); } @@ -765,6 +767,13 @@ private void ensureAlive() { */ boolean shouldDestroyEngineWithHost(); + /** + * Callback called when the {@link FlutterEngine} has been attached to by another activity + * before this activity was destroyed. + * + * The expected behavior is for this activity to synchronously stop using the + * {@link FlutterEngine} to avoid lifecycle crosstalk with the new activity. + */ void detachFromFlutterEngine(); /** Returns the Dart entrypoint that should run when a new {@link FlutterEngine} is created. */ diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterEngineConfigurator.java b/shell/platform/android/io/flutter/embedding/android/FlutterEngineConfigurator.java index 227770d4b0bd6..94dc318173e79 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterEngineConfigurator.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterEngineConfigurator.java @@ -4,7 +4,6 @@ package io.flutter.embedding.android; -import android.app.Activity; import androidx.annotation.NonNull; import androidx.lifecycle.Lifecycle; import io.flutter.embedding.engine.FlutterEngine; @@ -21,8 +20,8 @@ public interface FlutterEngineConfigurator { * *

This method is called after the given {@link FlutterEngine} has been attached to the owning * {@code FragmentActivity}. See {@link - * io.flutter.embedding.engine.plugins.activity.ActivityControlSurface#attachToActivity(Activity, - * Lifecycle)}. + * io.flutter.embedding.engine.plugins.activity.ActivityControlSurface#attachToActivity( + * ExclusiveAppComponent, Lifecycle)}. * *

It is possible that the owning {@code FragmentActivity} opted not to connect itself as an * {@link io.flutter.embedding.engine.plugins.activity.ActivityControlSurface}. In that case, any diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java index a473f4ad5e0c9..3101cbfd1eee1 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java @@ -623,19 +623,31 @@ public void onPause() { @Override public void onStop() { super.onStop(); - delegate.onStop(); + if (delegate != null) { + delegate.onStop(); + } else { + Log.v(TAG, "FlutterFragment " + this + " onStop called after release."); + } } @Override public void onDestroyView() { super.onDestroyView(); - delegate.onDestroyView(); + if (delegate != null) { + delegate.onDestroyView(); + } else { + Log.v(TAG, "FlutterFragment " + this + " onDestroyView called after release."); + } } @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); - delegate.onSaveInstanceState(outState); + if (delegate != null) { + delegate.onSaveInstanceState(outState); + } else { + Log.v(TAG, "FlutterFragment " + this + " onSaveInstanceState called after release."); + } } @Override @@ -657,9 +669,13 @@ public void detachFromFlutterEngine() { @Override public void onDetach() { super.onDetach(); - delegate.onDetach(); - delegate.release(); - delegate = null; + if (delegate != null) { + delegate.onDetach(); + delegate.release(); + delegate = null; + } else { + Log.v(TAG, "FlutterFragment " + this + " onDetach called after release."); + } } /** @@ -945,8 +961,8 @@ public PlatformPlugin providePlatformPlugin( * *

This method is called after {@link #provideFlutterEngine(Context)}, and after the given * {@link FlutterEngine} has been attached to the owning {@code FragmentActivity}. See {@link - * io.flutter.embedding.engine.plugins.activity.ActivityControlSurface#attachToActivity(Activity, - * Lifecycle)}. + * io.flutter.embedding.engine.plugins.activity.ActivityControlSurface#attachToActivity( + * ExclusiveAppComponent, Lifecycle)}. * *

It is possible that the owning {@code FragmentActivity} opted not to connect itself as an * {@link io.flutter.embedding.engine.plugins.activity.ActivityControlSurface}. In that case, any diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java index eadc706c32747..1c8575bc1d95f 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java @@ -104,12 +104,9 @@ public void destroy() { Log.v(TAG, "Destroying."); - // Detach from any Android component that we may currently be attached to, e.g., - // Activity, - // Service, - // BroadcastReceiver, ContentProvider. This must happen before removing all - // plugins so that the - // plugins have an opportunity to clean up references as a result of component + // Detach from any Android component that we may currently be attached to, e.g., Activity, + // Service, BroadcastReceiver, ContentProvider. This must happen before removing all plugins so + // that the plugins have an opportunity to clean up references as a result of component // detachment. detachFromAppComponent(); @@ -161,11 +158,9 @@ public void add(@NonNull FlutterPlugin plugin) { } } - // For BroadcastReceiverAware plugins, add the plugin to our set of - // BroadcastReceiverAware + // For BroadcastReceiverAware plugins, add the plugin to our set of BroadcastReceiverAware // plugins, and if this engine is currently attached to a BroadcastReceiver, - // notify the BroadcastReceiverAware plugin that it is now attached to a - // BroadcastReceiver. + // notify the BroadcastReceiverAware plugin that it is now attached to a BroadcastReceiver. if (plugin instanceof BroadcastReceiverAware) { BroadcastReceiverAware broadcastReceiverAware = (BroadcastReceiverAware) plugin; broadcastReceiverAwarePlugins.put(plugin.getClass(), broadcastReceiverAware); @@ -175,11 +170,9 @@ public void add(@NonNull FlutterPlugin plugin) { } } - // For ContentProviderAware plugins, add the plugin to our set of - // ContentProviderAware + // For ContentProviderAware plugins, add the plugin to our set of ContentProviderAware // plugins, and if this engine is currently attached to a ContentProvider, - // notify the ContentProviderAware plugin that it is now attached to a - // ContentProvider. + // notify the ContentProviderAware plugin that it is now attached to a ContentProvider. if (plugin instanceof ContentProviderAware) { ContentProviderAware contentProviderAware = (ContentProviderAware) plugin; contentProviderAwarePlugins.put(plugin.getClass(), contentProviderAware); @@ -234,10 +227,8 @@ public void remove(@NonNull Class pluginClass) { serviceAwarePlugins.remove(pluginClass); } - // For BroadcastReceiverAware plugins, notify the plugin that it is detached - // from - // a BroadcastReceiver if a BroadcastReceiver is currently attached to this - // engine. Then + // For BroadcastReceiverAware plugins, notify the plugin that it is detached from + // a BroadcastReceiver if a BroadcastReceiver is currently attached to this engine. Then // remove the plugin from our set of BroadcastReceiverAware plugins. if (plugin instanceof BroadcastReceiverAware) { if (isAttachedToBroadcastReceiver()) { @@ -248,8 +239,7 @@ public void remove(@NonNull Class pluginClass) { } // For ContentProviderAware plugins, notify the plugin that it is detached from - // a ContentProvider if a ContentProvider is currently attached to this engine. - // Then + // a ContentProvider if a ContentProvider is currently attached to this engine. Then // remove the plugin from our set of ContentProviderAware plugins. if (plugin instanceof ContentProviderAware) { if (isAttachedToContentProvider()) { @@ -337,15 +327,14 @@ public void attachToActivity( private void attachToActivityInternal(@NonNull Activity activity, @NonNull Lifecycle lifecycle) { this.activityPluginBinding = new FlutterEngineActivityPluginBinding(activity, lifecycle); - // Activate the PlatformViewsController. This must happen before any plugins - // attempt to use it, otherwise an error stack trace will appear that says there is no + // Activate the PlatformViewsController. This must happen before any plugins attempt + // to use it, otherwise an error stack trace will appear that says there is no // flutter/platform_views channel. flutterEngine .getPlatformViewsController() .attach(activity, flutterEngine.getRenderer(), flutterEngine.getDartExecutor()); - // Notify all ActivityAware plugins that they are now attached to a new - // Activity. + // Notify all ActivityAware plugins that they are now attached to a new Activity. for (ActivityAware activityAware : activityAwarePlugins.values()) { if (isWaitingForActivityReattachment) { activityAware.onReattachedToActivityForConfigChanges(activityPluginBinding); @@ -501,8 +490,7 @@ public void attachToService( public void detachFromService() { if (isAttachedToService()) { Log.v(TAG, "Detaching from a Service: " + service); - // Notify all ServiceAware plugins that they are no longer attached to a - // Service. + // Notify all ServiceAware plugins that they are no longer attached to a Service. for (ServiceAware serviceAware : serviceAwarePlugins.values()) { serviceAware.onDetachedFromService(); } @@ -546,8 +534,7 @@ public void attachToBroadcastReceiver( this.broadcastReceiver = broadcastReceiver; this.broadcastReceiverPluginBinding = new FlutterEngineBroadcastReceiverPluginBinding(broadcastReceiver); - // TODO(mattcarroll): resolve possibility of different lifecycles between this - // and engine + // TODO(mattcarroll): resolve possibility of different lifecycles between this and engine // attachment // Notify all BroadcastReceiverAware plugins that they are now attached to a new @@ -561,8 +548,7 @@ public void attachToBroadcastReceiver( public void detachFromBroadcastReceiver() { if (isAttachedToBroadcastReceiver()) { Log.v(TAG, "Detaching from BroadcastReceiver: " + broadcastReceiver); - // Notify all BroadcastReceiverAware plugins that they are no longer attached to - // a + // Notify all BroadcastReceiverAware plugins that they are no longer attached to a // BroadcastReceiver. for (BroadcastReceiverAware broadcastReceiverAware : broadcastReceiverAwarePlugins.values()) { broadcastReceiverAware.onDetachedFromBroadcastReceiver(); @@ -590,12 +576,10 @@ public void attachToContentProvider( this.contentProvider = contentProvider; this.contentProviderPluginBinding = new FlutterEngineContentProviderPluginBinding(contentProvider); - // TODO(mattcarroll): resolve possibility of different lifecycles between this - // and engine + // TODO(mattcarroll): resolve possibility of different lifecycles between this and engine // attachment - // Notify all ContentProviderAware plugins that they are now attached to a new - // ContentProvider. + // Notify all ContentProviderAware plugins that they are now attached to a new ContentProvider. for (ContentProviderAware contentProviderAware : contentProviderAwarePlugins.values()) { contentProviderAware.onAttachedToContentProvider(contentProviderPluginBinding); } diff --git a/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java b/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java index 2b7a57a090a6b..88a2ad1c82711 100644 --- a/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java +++ b/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java @@ -20,9 +20,10 @@ * *

    *
  1. Once an {@link Activity} is created, and its associated {@link FlutterEngine} is executing - * Dart code, the {@link Activity} should invoke {@link #attachToActivity(Activity, - * Lifecycle)}. At this point the {@link FlutterEngine} is considered "attached" to the {@link - * Activity} and all {@link ActivityAware} plugins are given access to the {@link Activity}. + * Dart code, the {@link Activity} should invoke {@link #attachToActivity( + * ExclusiveAppComponent, Lifecycle)}. At this point the {@link FlutterEngine} is considered + * "attached" to the {@link Activity} and all {@link ActivityAware} plugins are given access + * to the {@link Activity}. *
  2. Just before an attached {@link Activity} is destroyed for configuration change purposes, * that {@link Activity} should invoke {@link #detachFromActivityForConfigChanges()}, giving * each {@link ActivityAware} plugin an opportunity to clean up its references before the @@ -33,6 +34,10 @@ *
  3. When an {@link Activity} is destroyed for non-configuration-change purposes, or when the * {@link Activity} is no longer interested in displaying a {@link FlutterEngine}'s content, * the {@link Activity} should invoke {@link #detachFromActivity()}. + *
  4. When a {@link Activity} is being attached while an existing {@link ExclusiveAppComponent} + * is already attached, the existing {@link ExclusiveAppComponent} is given a chance to + * detach first via {@link ExclusiveAppComponent#detachFromFlutterEngine()} before the new + * activity attaches. *
* * The attached {@link Activity} should also forward all {@link Activity} calls that this {@code @@ -49,9 +54,28 @@ public interface ActivityControlSurface { * Dart code, the {@link Activity} should invoke this method. At that point the {@link * FlutterEngine} is considered "attached" to the {@link Activity} and all {@link ActivityAware} * plugins are given access to the {@link Activity}. + * + * @deprecated Prefer using the {@link #attachToActivity(ExclusiveAppComponent, Lifecycle)} API + * to avoid situations where multiple activities are driving the FlutterEngine simultaneously. + * See https://github.com/flutter/flutter/issues/66192. */ + @Deprecated void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle lifecycle); + /** + * Call this method from the {@link ExclusiveAppComponent} that is displaying the visual content + * of the {@link FlutterEngine} that is associated with this {@code ActivityControlSurface}. + * + *

Once an {@link ExclusiveAppComponent} is created, and its associated {@link FlutterEngine} + * is executing Dart code, the {@link ExclusiveAppComponent} should invoke this method. At that + * point the {@link FlutterEngine} is considered "attached" to the {@link ExclusiveAppComponent} + * and all {@link ActivityAware} plugins are given access to the {@link ExclusiveAppComponent}'s + * {@link Activity}. + * + *

This method differs from {@link #attachToActivity(Activity, Lifecycle)} in that it calls + * back the existing {@link ExclusiveAppComponent} to give it a chance to cleanly detach before a + * new {@link ExclusiveAppComponent} is attached. + */ void attachToActivity( @NonNull ExclusiveAppComponent exclusiveActivity, @NonNull Lifecycle lifecycle); From 2c2185779538850c20b582a9b6df02c594e5df38 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Sun, 20 Sep 2020 02:22:03 -0700 Subject: [PATCH 04/12] add test --- .../FlutterActivityAndFragmentDelegate.java | 4 +- .../engine/FlutterEnginePluginRegistry.java | 12 ++- ...lutterActivityAndFragmentDelegateTest.java | 3 +- .../android/FlutterActivityTest.java | 19 ++-- .../android/FlutterAndroidComponentTest.java | 94 ++++++++++++++++++- .../android/FlutterFragmentTest.java | 35 +++++++ 6 files changed, 151 insertions(+), 16 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java index 99685f4ed519a..84d0bbea764e8 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java @@ -62,7 +62,7 @@ * the same form. Do not use this class as a convenient shortcut for any other * behavior. */ -/* package */ final class FlutterActivityAndFragmentDelegate +/* package */ class FlutterActivityAndFragmentDelegate implements ExclusiveAppComponent { private static final String TAG = "FlutterActivityAndFragmentDelegate"; private static final String FRAMEWORK_RESTORATION_BUNDLE_KEY = "framework"; @@ -497,7 +497,7 @@ public void detachFromFlutterEngine() { // activity. throw new AssertionError("The internal FlutterEngine created by " + host + " has been attached to by another activity. To persist a FlutterEngine beyond the " + - "ownership of this activity, explicitly create a FlutterEngine"; + "ownership of this activity, explicitly create a FlutterEngine"); } // Default, but customizable, behavior is for the host to call {@link #onDetach} diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java index 1c8575bc1d95f..6b51b0307ba98 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java @@ -300,6 +300,9 @@ public void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle life + activity + "." + (isWaitingForActivityReattachment ? " This is after a config change." : "")); + if (this.exclusiveActivity != null) { + this.exclusiveActivity.detachFromFlutterEngine(); + } // If we were already attached to an app component, detach from it. detachFromAppComponent(); @@ -317,6 +320,9 @@ public void attachToActivity( + (isAttachedToActivity() ? " evicting previous activity " + attachedActivity() : "") + "." + (isWaitingForActivityReattachment ? " This is after a config change." : "")); + if (this.exclusiveActivity != null) { + this.exclusiveActivity.detachFromFlutterEngine(); + } // If we were already attached to an app component, detach from it. detachFromAppComponent(); @@ -379,11 +385,7 @@ private void detachFromActivityInternal() { // Deactivate PlatformViewsController. flutterEngine.getPlatformViewsController().detach(); - if (exclusiveActivity != null) { - final ExclusiveAppComponent detachingActivity = exclusiveActivity; - exclusiveActivity = null; - detachingActivity.detachFromFlutterEngine(); - } + exclusiveActivity = null; activity = null; activityPluginBinding = null; } diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java index 7ef0b048e7e8f..06bdc15af37ee 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java @@ -18,6 +18,7 @@ import androidx.annotation.NonNull; import androidx.lifecycle.Lifecycle; import io.flutter.FlutterInjector; +import io.flutter.embedding.android.ExclusiveAppComponent; import io.flutter.embedding.android.FlutterActivityAndFragmentDelegate.Host; import io.flutter.embedding.engine.FlutterEngine; import io.flutter.embedding.engine.FlutterEngineCache; @@ -357,7 +358,7 @@ public void itAttachesFlutterToTheActivityIfDesired() { // Verify that the ActivityControlSurface was told to attach to an Activity. verify(mockFlutterEngine.getActivityControlSurface(), times(1)) - .attachToActivity(any(Activity.class), any(Lifecycle.class)); + .attachToActivity(any(ExclusiveAppComponent.class), any(Lifecycle.class)); // Flutter is detached from the surrounding Activity in onDetach. delegate.onDetach(); diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java index dc28202229f58..c753686c21550 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java @@ -9,6 +9,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import android.content.Context; @@ -16,10 +17,13 @@ import android.os.Bundle; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import io.flutter.embedding.android.FlutterActivity; import io.flutter.embedding.android.FlutterActivityLaunchConfigs.BackgroundMode; import io.flutter.embedding.engine.FlutterEngine; +import io.flutter.embedding.engine.FlutterEngineCache; import io.flutter.embedding.engine.FlutterJNI; import io.flutter.embedding.engine.loader.FlutterLoader; +import io.flutter.embedding.engine.plugins.activity.ActivityControlSurface; import io.flutter.plugins.GeneratedPluginRegistrant; import java.util.List; import org.junit.After; @@ -162,9 +166,11 @@ public void itRegistersPluginsAtConfigurationTime() { } @Test - public void itCanBeManuallyReleasedBeforeOnDestroy() { + public void itCanBeDetachedFromTheEngineAndStopSendingFurtherEvents() { FlutterActivityAndFragmentDelegate mockDelegate = mock(FlutterActivityAndFragmentDelegate.class); + FlutterEngine mockEngine = mock(FlutterEngine.class); + FlutterEngineCache.getInstance().put("my_cached_engine", mockEngine); Intent intent = FlutterActivity.withCachedEngine("my_cached_engine").build(RuntimeEnvironment.application); @@ -172,24 +178,25 @@ public void itCanBeManuallyReleasedBeforeOnDestroy() { Robolectric.buildActivity(FlutterActivity.class, intent); FlutterActivity flutterActivity = activityController.get(); flutterActivity.setDelegate(mockDelegate); - flutterActivity.onCreate(null); flutterActivity.onStart(); flutterActivity.onResume(); - verify(mockDelegate, times(1)).onAttach(flutterActivity); verify(mockDelegate, times(1)).onStart(); verify(mockDelegate, times(1)).onResume(); flutterActivity.onPause(); - flutterActivity.release(); // Package visibility. + flutterActivity.detachFromFlutterEngine(); verify(mockDelegate, times(1)).onPause(); + verify(mockDelegate, times(1)).onDestroyView(); + verify(mockDelegate, times(1)).onDetach(); flutterActivity.onStop(); flutterActivity.onDestroy(); verify(mockDelegate, never()).onStop(); - verify(mockDelegate, never()).onDestroyView(); - verify(mockDelegate, never()).onDetach(); + // 1 time same as before. + verify(mockDelegate, times(1)).onDestroyView(); + verify(mockDelegate, times(1)).onDetach(); } static class FlutterActivityWithProvidedEngine extends FlutterActivity { diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java index 9255cb4077b67..3e702f0463a48 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java @@ -3,19 +3,25 @@ import static org.junit.Assert.assertNotNull; import static org.mockito.Matchers.any; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; import android.app.Activity; import android.content.Context; +import android.content.Intent; import android.os.Bundle; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.lifecycle.Lifecycle; +import io.flutter.FlutterInjectorTest; +import io.flutter.embedding.android.FlutterActivity; import io.flutter.embedding.engine.FlutterEngine; import io.flutter.embedding.engine.FlutterEngineCache; import io.flutter.embedding.engine.FlutterJNI; @@ -24,15 +30,18 @@ import io.flutter.embedding.engine.plugins.FlutterPlugin; import io.flutter.embedding.engine.plugins.activity.ActivityAware; import io.flutter.embedding.engine.plugins.activity.ActivityPluginBinding; +import io.flutter.embedding.engine.systemchannels.LifecycleChannel; import io.flutter.plugin.platform.PlatformPlugin; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.android.controller.ActivityController; import org.robolectric.annotation.Config; @Config(manifest = Config.NONE) @@ -54,7 +63,8 @@ public void pluginsReceiveFlutterPluginBinding() { cachedEngine.getPlugins().add(mockPlugin); // Create a fake Host, which is required by the delegate. - FlutterActivityAndFragmentDelegate.Host fakeHost = new FakeHost(cachedEngine); + FakeHost fakeHost = new FakeHost(cachedEngine); + fakeHost.shouldDestroyEngineWithHost = true; // Create the real object that we're testing. FlutterActivityAndFragmentDelegate delegate = new FlutterActivityAndFragmentDelegate(fakeHost); @@ -165,9 +175,86 @@ public Object answer(InvocationOnMock invocation) throws Throwable { verify(activityAwarePlugin, times(1)).onDetachedFromActivity(); } + @Test + public void normalLifecycleStepsDoNotTriggerADetachFromFlutterEngine() { + // ---- Test setup ---- + // Place a FlutterEngine in the static cache. + FlutterLoader mockFlutterLoader = mock(FlutterLoader.class); + FlutterJNI mockFlutterJni = mock(FlutterJNI.class); + when(mockFlutterJni.isAttached()).thenReturn(true); + FlutterEngine cachedEngine = + spy(new FlutterEngine(RuntimeEnvironment.application, mockFlutterLoader, mockFlutterJni)); + FlutterEngineCache.getInstance().put("my_flutter_engine", cachedEngine); + + // Create a fake Host, which is required by the delegate. + FakeHost fakeHost = new FakeHost(cachedEngine); + + // Create the real object that we're testing. + FlutterActivityAndFragmentDelegate delegate = spy(new FlutterActivityAndFragmentDelegate(fakeHost)); + + // --- Execute the behavior under test --- + // Push the delegate through all lifecycle methods all the way to destruction. + delegate.onAttach(RuntimeEnvironment.application); + delegate.onActivityCreated(null); + delegate.onCreateView(null, null, null); + delegate.onStart(); + delegate.onResume(); + delegate.onPause(); + delegate.onStop(); + delegate.onDestroyView(); + delegate.onDetach(); + + verify(delegate, never()).detachFromFlutterEngine(); + } + + @Test + public void twoOverlappingFlutterActivitiesDoNotCrosstalk() { + // ---- Test setup ---- + // Place a FlutterEngine in the static cache. + FlutterLoader mockFlutterLoader = mock(FlutterLoader.class); + FlutterJNI mockFlutterJni = mock(FlutterJNI.class); + when(mockFlutterJni.isAttached()).thenReturn(true); + FlutterEngine cachedEngine = + spy(new FlutterEngine(RuntimeEnvironment.application, mockFlutterLoader, mockFlutterJni)); + FlutterEngineCache.getInstance().put("my_flutter_engine", cachedEngine); + LifecycleChannel mockLifecycleChannel = mock(LifecycleChannel.class); + when(cachedEngine.getLifecycleChannel()).thenReturn(mockLifecycleChannel); + + Intent intent = + FlutterActivity.withCachedEngine("my_flutter_engine").build(RuntimeEnvironment.application); + ActivityController activityController1 = + Robolectric.buildActivity(FlutterActivity.class, intent); + activityController1.create().start().resume(); + + InOrder inOrder = inOrder(mockLifecycleChannel); + inOrder.verify(mockLifecycleChannel, times(1)).appIsResumed(); + verifyNoMoreInteractions(mockLifecycleChannel); + + activityController1.pause(); + // Create a second instance on the same engine and start running it as well. + ActivityController activityController2 = + Robolectric.buildActivity(FlutterActivity.class, intent); + activityController2.create().start().resume(); + + // From the onPause of the first activity. + inOrder.verify(mockLifecycleChannel, times(1)).appIsInactive(); + // By creating the second activity, we should automatically detach the first activity. + inOrder.verify(mockLifecycleChannel, times(1)).appIsDetached(); + // In order, the second activity then is resumed. + inOrder.verify(mockLifecycleChannel, times(1)).appIsResumed(); + verifyNoMoreInteractions(mockLifecycleChannel); + + // The first activity goes through the normal lifecycles of destruction, but since we + // detached the first activity during the second activity's creation, we should ignore the + // first activity's destruction events to avoid crosstalk. + activityController1.stop().destroy(); + verifyNoMoreInteractions(mockLifecycleChannel); + } + private static class FakeHost implements FlutterActivityAndFragmentDelegate.Host { final FlutterEngine cachedEngine; Activity activity; + boolean shouldDestroyEngineWithHost = false; Lifecycle lifecycle = mock(Lifecycle.class); private FakeHost(@NonNull FlutterEngine flutterEngine) { @@ -209,7 +296,7 @@ public String getCachedEngineId() { @Override public boolean shouldDestroyEngineWithHost() { - return true; + return shouldDestroyEngineWithHost; } @NonNull @@ -288,5 +375,8 @@ public void onFlutterUiDisplayed() {} @Override public void onFlutterUiNoLongerDisplayed() {} + + @Override + public void detachFromFlutterEngine() {} } } diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java index 6dea8ec6b946d..72c66047e581a 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java @@ -6,6 +6,11 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -71,4 +76,34 @@ public void itCreatesCachedEngineFragmentThatDestroysTheEngine() { assertEquals("my_cached_engine", fragment.getCachedEngineId()); assertTrue(fragment.shouldDestroyEngineWithHost()); } + + @Test + public void itCanBeDetachedFromTheEngineAndStopSendingFurtherEvents() { + FlutterActivityAndFragmentDelegate mockDelegate = + mock(FlutterActivityAndFragmentDelegate.class); + FlutterFragment fragment = + FlutterFragment.withCachedEngine("my_cached_engine") + .destroyEngineWithFragment(true) + .build(); + fragment.setDelegate(mockDelegate); + fragment.onStart(); + fragment.onResume(); + + verify(mockDelegate, times(1)).onStart(); + verify(mockDelegate, times(1)).onResume(); + + fragment.onPause(); + fragment.detachFromFlutterEngine(); + verify(mockDelegate, times(1)).onPause(); + verify(mockDelegate, times(1)).onDestroyView(); + verify(mockDelegate, times(1)).onDetach(); + + fragment.onStop(); + fragment.onDestroy(); + + verify(mockDelegate, never()).onStop(); + // 1 time same as before. + verify(mockDelegate, times(1)).onDestroyView(); + verify(mockDelegate, times(1)).onDetach(); + } } From 436ecd6db9fd0eda40750bebdc4077ca6e288adc Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Sun, 20 Sep 2020 02:22:58 -0700 Subject: [PATCH 05/12] format --- .../android/ExclusiveAppComponent.java | 30 +++++++++---------- .../FlutterActivityAndFragmentDelegate.java | 15 +++++----- .../activity/ActivityControlSurface.java | 12 ++++---- ...lutterActivityAndFragmentDelegateTest.java | 1 - .../android/FlutterActivityTest.java | 3 -- .../android/FlutterAndroidComponentTest.java | 5 ++-- .../android/FlutterFragmentTest.java | 1 - 7 files changed, 31 insertions(+), 36 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java b/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java index 77500d283ceff..e1c356a6ea6b4 100644 --- a/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java +++ b/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java @@ -7,29 +7,29 @@ import androidx.annotation.NonNull; /** - * An Android App Component exclusively attached to a {@link io.flutter.embedding.engine.FlutterEngine}. + * An Android App Component exclusively attached to a {@link + * io.flutter.embedding.engine.FlutterEngine}. * - * An exclusive App Component's {@link #detachFromFlutterEngine} is invoked when another - * App Component is becoming attached to the {@link io.flutter.embedding.engine.FlutterEngine}. + *

An exclusive App Component's {@link #detachFromFlutterEngine} is invoked when another App + * Component is becoming attached to the {@link io.flutter.embedding.engine.FlutterEngine}. * - * The term "App Component" refer to the 4 component types: Activity, Service, Broadcast Receiver, - * and Content Provider, as defined in https://developer.android.com/guide/components/fundamentals. + *

The term "App Component" refer to the 4 component types: Activity, Service, Broadcast + * Receiver, and Content Provider, as defined in + * https://developer.android.com/guide/components/fundamentals. * * @param The App Component behind this exclusive App Component. */ public interface ExclusiveAppComponent { /** - * Called when another App Component is about to become attached to the - * {@link io.flutter.embedding.engine.FlutterEngine} this App Component is currently attached - * to. + * Called when another App Component is about to become attached to the {@link + * io.flutter.embedding.engine.FlutterEngine} this App Component is currently attached to. * - * This App Component's connections to the {@link io.flutter.embedding.engine.FlutterEngine} are - * still valid at the moment of this call. + *

This App Component's connections to the {@link io.flutter.embedding.engine.FlutterEngine} + * are still valid at the moment of this call. */ void detachFromFlutterEngine(); - /** - * Retrieve the App Component behind this exclusive App Component. - */ - @NonNull T getAppComponent(); -} \ No newline at end of file + /** Retrieve the App Component behind this exclusive App Component. */ + @NonNull + T getAppComponent(); +} diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java index 84d0bbea764e8..c0cb3aca34fcd 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java @@ -62,8 +62,7 @@ * the same form. Do not use this class as a convenient shortcut for any other * behavior. */ -/* package */ class FlutterActivityAndFragmentDelegate - implements ExclusiveAppComponent { +/* package */ class FlutterActivityAndFragmentDelegate implements ExclusiveAppComponent { private static final String TAG = "FlutterActivityAndFragmentDelegate"; private static final String FRAMEWORK_RESTORATION_BUNDLE_KEY = "framework"; private static final String PLUGINS_RESTORATION_BUNDLE_KEY = "plugins"; @@ -495,9 +494,11 @@ public void detachFromFlutterEngine() { if (host.shouldDestroyEngineWithHost()) { // The host owns the engine and should never have its engine taken by another exclusive // activity. - throw new AssertionError("The internal FlutterEngine created by " + host + - " has been attached to by another activity. To persist a FlutterEngine beyond the " + - "ownership of this activity, explicitly create a FlutterEngine"); + throw new AssertionError( + "The internal FlutterEngine created by " + + host + + " has been attached to by another activity. To persist a FlutterEngine beyond the " + + "ownership of this activity, explicitly create a FlutterEngine"); } // Default, but customizable, behavior is for the host to call {@link #onDetach} @@ -771,8 +772,8 @@ private void ensureAlive() { * Callback called when the {@link FlutterEngine} has been attached to by another activity * before this activity was destroyed. * - * The expected behavior is for this activity to synchronously stop using the - * {@link FlutterEngine} to avoid lifecycle crosstalk with the new activity. + *

The expected behavior is for this activity to synchronously stop using the {@link + * FlutterEngine} to avoid lifecycle crosstalk with the new activity. */ void detachFromFlutterEngine(); diff --git a/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java b/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java index 88a2ad1c82711..276b54d56321a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java +++ b/shell/platform/android/io/flutter/embedding/engine/plugins/activity/ActivityControlSurface.java @@ -35,9 +35,9 @@ * {@link Activity} is no longer interested in displaying a {@link FlutterEngine}'s content, * the {@link Activity} should invoke {@link #detachFromActivity()}. *

  • When a {@link Activity} is being attached while an existing {@link ExclusiveAppComponent} - * is already attached, the existing {@link ExclusiveAppComponent} is given a chance to - * detach first via {@link ExclusiveAppComponent#detachFromFlutterEngine()} before the new - * activity attaches. + * is already attached, the existing {@link ExclusiveAppComponent} is given a chance to detach + * first via {@link ExclusiveAppComponent#detachFromFlutterEngine()} before the new activity + * attaches. * * * The attached {@link Activity} should also forward all {@link Activity} calls that this {@code @@ -55,9 +55,9 @@ public interface ActivityControlSurface { * FlutterEngine} is considered "attached" to the {@link Activity} and all {@link ActivityAware} * plugins are given access to the {@link Activity}. * - * @deprecated Prefer using the {@link #attachToActivity(ExclusiveAppComponent, Lifecycle)} API - * to avoid situations where multiple activities are driving the FlutterEngine simultaneously. - * See https://github.com/flutter/flutter/issues/66192. + * @deprecated Prefer using the {@link #attachToActivity(ExclusiveAppComponent, Lifecycle)} API to + * avoid situations where multiple activities are driving the FlutterEngine simultaneously. + * See https://github.com/flutter/flutter/issues/66192. */ @Deprecated void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle lifecycle); diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java index 06bdc15af37ee..4a621b3344107 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java @@ -18,7 +18,6 @@ import androidx.annotation.NonNull; import androidx.lifecycle.Lifecycle; import io.flutter.FlutterInjector; -import io.flutter.embedding.android.ExclusiveAppComponent; import io.flutter.embedding.android.FlutterActivityAndFragmentDelegate.Host; import io.flutter.embedding.engine.FlutterEngine; import io.flutter.embedding.engine.FlutterEngineCache; diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java index c753686c21550..2fe23de49b303 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java @@ -9,7 +9,6 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import android.content.Context; @@ -17,13 +16,11 @@ import android.os.Bundle; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import io.flutter.embedding.android.FlutterActivity; import io.flutter.embedding.android.FlutterActivityLaunchConfigs.BackgroundMode; import io.flutter.embedding.engine.FlutterEngine; import io.flutter.embedding.engine.FlutterEngineCache; import io.flutter.embedding.engine.FlutterJNI; import io.flutter.embedding.engine.loader.FlutterLoader; -import io.flutter.embedding.engine.plugins.activity.ActivityControlSurface; import io.flutter.plugins.GeneratedPluginRegistrant; import java.util.List; import org.junit.After; diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java index 3e702f0463a48..5cede33a1b833 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterAndroidComponentTest.java @@ -20,8 +20,6 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.lifecycle.Lifecycle; -import io.flutter.FlutterInjectorTest; -import io.flutter.embedding.android.FlutterActivity; import io.flutter.embedding.engine.FlutterEngine; import io.flutter.embedding.engine.FlutterEngineCache; import io.flutter.embedding.engine.FlutterJNI; @@ -190,7 +188,8 @@ public void normalLifecycleStepsDoNotTriggerADetachFromFlutterEngine() { FakeHost fakeHost = new FakeHost(cachedEngine); // Create the real object that we're testing. - FlutterActivityAndFragmentDelegate delegate = spy(new FlutterActivityAndFragmentDelegate(fakeHost)); + FlutterActivityAndFragmentDelegate delegate = + spy(new FlutterActivityAndFragmentDelegate(fakeHost)); // --- Execute the behavior under test --- // Push the delegate through all lifecycle methods all the way to destruction. diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java index 72c66047e581a..9327994e8d9e6 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterFragmentTest.java @@ -5,7 +5,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; - import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; From f3e00aade9d69dc40a5737fe9875a4fbec689baa Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Sun, 20 Sep 2020 02:36:08 -0700 Subject: [PATCH 06/12] missed a license file --- ci/licenses_golden/licenses_flutter | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2211299487ad3..66dd4e5d10262 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -708,6 +708,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/app/FlutterPluginRegist FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/AndroidTouchProcessor.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/DrawableSplashScreen.java +FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/ExclusiveAppComponent.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterActivityLaunchConfigs.java From 1f42415b66fc161c0057b916c1c67b8226651cbb Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Sun, 20 Sep 2020 11:43:33 -0700 Subject: [PATCH 07/12] review --- .../io/flutter/embedding/android/FlutterActivity.java | 1 - .../embedding/engine/FlutterEnginePluginRegistry.java | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java index 0ad28010898f8..7dd3fa7c99744 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java @@ -610,7 +610,6 @@ public void detachFromFlutterEngine() { @Override protected void onDestroy() { super.onDestroy(); - Log.e("meh", "Activity " + this + " onDestroy"); if (delegate != null) { release(); } else { diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java index 6b51b0307ba98..a713cb19259db 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java @@ -58,6 +58,8 @@ private final Map, ActivityAware> activityAwarePlugins = new HashMap<>(); + // TODO(xster): remove activity after 2021/03/01 since exclusiveActivity should be the API to use. + @Deprecated @Nullable private Activity activity; @Nullable private ExclusiveAppComponent exclusiveActivity; @Nullable private FlutterEngineActivityPluginBinding activityPluginBinding; @@ -306,6 +308,9 @@ public void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle life // If we were already attached to an app component, detach from it. detachFromAppComponent(); + if (this.exclusiveActivity != null) { + throw new AssertionError("Only activity or exclusiveActivity should be set"); + } this.activity = activity; attachToActivityInternal(activity, lifecycle); } @@ -326,6 +331,9 @@ public void attachToActivity( // If we were already attached to an app component, detach from it. detachFromAppComponent(); + if (this.activity != null) { + throw new AssertionError("Only activity or exclusiveActivity should be set"); + } this.exclusiveActivity = exclusiveActivity; attachToActivityInternal(exclusiveActivity.getAppComponent(), lifecycle); } From a8d3e4bdd307e0d1451503079702092719e6a218 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Sun, 20 Sep 2020 11:52:45 -0700 Subject: [PATCH 08/12] add some more explicit errors than null reference for using destroyed activity/fragment --- .../embedding/android/FlutterActivity.java | 16 ++++++++++++++++ .../embedding/android/FlutterFragment.java | 17 +++++++++++++++++ .../engine/FlutterEnginePluginRegistry.java | 3 +-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java index 7dd3fa7c99744..e3c4b6e82a34e 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java @@ -533,6 +533,7 @@ private void configureStatusBarForFullscreenFlutterExperience() { @Override protected void onStart() { super.onStart(); + ensureAlive(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_START); delegate.onStart(); } @@ -540,6 +541,7 @@ protected void onStart() { @Override protected void onResume() { super.onResume(); + ensureAlive(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_RESUME); delegate.onResume(); } @@ -547,12 +549,14 @@ protected void onResume() { @Override public void onPostResume() { super.onPostResume(); + ensureAlive(); delegate.onPostResume(); } @Override protected void onPause() { super.onPause(); + ensureAlive(); delegate.onPause(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_PAUSE); } @@ -620,6 +624,7 @@ protected void onDestroy() { @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { + ensureAlive(); delegate.onActivityResult(requestCode, resultCode, data); } @@ -627,28 +632,33 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) { protected void onNewIntent(@NonNull Intent intent) { // TODO(mattcarroll): change G3 lint rule that forces us to call super super.onNewIntent(intent); + ensureAlive(); delegate.onNewIntent(intent); } @Override public void onBackPressed() { + ensureAlive(); delegate.onBackPressed(); } @Override public void onRequestPermissionsResult( int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { + ensureAlive(); delegate.onRequestPermissionsResult(requestCode, permissions, grantResults); } @Override public void onUserLeaveHint() { + ensureAlive(); delegate.onUserLeaveHint(); } @Override public void onTrimMemory(int level) { super.onTrimMemory(level); + ensureAlive(); delegate.onTrimMemory(level); } @@ -1001,4 +1011,10 @@ public boolean shouldRestoreAndSaveState() { } return true; } + + private void ensureAlive() { + if (delegate == null) { + throw new IllegalStateException("Cannot execute method on a destroyed FlutterActivity."); + } + } } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java index 3101cbfd1eee1..e957106efdda7 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java @@ -598,12 +598,14 @@ public void onActivityCreated(@Nullable Bundle savedInstanceState) { @Override public void onStart() { super.onStart(); + ensureAlive(); delegate.onStart(); } @Override public void onResume() { super.onResume(); + ensureAlive(); delegate.onResume(); } @@ -611,12 +613,14 @@ public void onResume() { // possible. @ActivityCallThrough public void onPostResume() { + ensureAlive(); delegate.onPostResume(); } @Override public void onPause() { super.onPause(); + ensureAlive(); delegate.onPause(); } @@ -692,6 +696,7 @@ public void onDetach() { @ActivityCallThrough public void onRequestPermissionsResult( int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { + ensureAlive(); delegate.onRequestPermissionsResult(requestCode, permissions, grantResults); } @@ -707,6 +712,7 @@ public void onRequestPermissionsResult( */ @ActivityCallThrough public void onNewIntent(@NonNull Intent intent) { + ensureAlive(); delegate.onNewIntent(intent); } @@ -717,6 +723,7 @@ public void onNewIntent(@NonNull Intent intent) { */ @ActivityCallThrough public void onBackPressed() { + ensureAlive(); delegate.onBackPressed(); } @@ -732,6 +739,7 @@ public void onBackPressed() { */ @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { + ensureAlive(); delegate.onActivityResult(requestCode, resultCode, data); } @@ -743,6 +751,7 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) { */ @ActivityCallThrough public void onUserLeaveHint() { + ensureAlive(); delegate.onUserLeaveHint(); } @@ -757,6 +766,7 @@ public void onUserLeaveHint() { */ @ActivityCallThrough public void onTrimMemory(int level) { + ensureAlive(); delegate.onTrimMemory(level); } @@ -768,6 +778,7 @@ public void onTrimMemory(int level) { @Override public void onLowMemory() { super.onLowMemory(); + ensureAlive(); delegate.onLowMemory(); } @@ -1067,6 +1078,12 @@ public boolean shouldRestoreAndSaveState() { return true; } + private void ensureAlive() { + if (delegate == null) { + throw new IllegalStateException("Cannot execute method on a destroyed FlutterFragment."); + } + } + /** * Annotates methods in {@code FlutterFragment} that must be called by the containing {@code * Activity}. diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java index a713cb19259db..efd0b209609e3 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java @@ -59,8 +59,7 @@ new HashMap<>(); // TODO(xster): remove activity after 2021/03/01 since exclusiveActivity should be the API to use. - @Deprecated - @Nullable private Activity activity; + @Deprecated @Nullable private Activity activity; @Nullable private ExclusiveAppComponent exclusiveActivity; @Nullable private FlutterEngineActivityPluginBinding activityPluginBinding; private boolean isWaitingForActivityReattachment = false; From 4b9483af34b418e39fbe945e8e58cae60cca6890 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Mon, 21 Sep 2020 12:41:48 -0700 Subject: [PATCH 09/12] rename FlutterEnginePluginRegistry to FlutterEngineConectionRegistry --- ci/licenses_golden/licenses_flutter | 2 +- shell/platform/android/BUILD.gn | 4 ++-- .../android/FlutterActivityAndFragmentDelegate.java | 2 +- .../io/flutter/embedding/engine/FlutterEngine.java | 4 ++-- ...ry.java => FlutterEngineConnectionRegistry.java} | 13 ++++++++++--- .../android/test/io/flutter/FlutterTestSuite.java | 4 ++-- ...ava => FlutterEngineConnectionRegistryTest.java} | 10 +++++----- 7 files changed, 23 insertions(+), 16 deletions(-) rename shell/platform/android/io/flutter/embedding/engine/{FlutterEnginePluginRegistry.java => FlutterEngineConnectionRegistry.java} (98%) rename shell/platform/android/test/io/flutter/embedding/engine/{FlutterEnginePluginRegistryTest.java => FlutterEngineConnectionRegistryTest.java} (95%) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 66dd4e5d10262..bfa1b74aed56d 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -728,7 +728,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/Splas FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/TransparencyMode.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEngineCache.java -FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java +FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterOverlaySurface.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index b6cd832a83e6f..52373e3c74ffa 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -148,7 +148,7 @@ android_java_sources = [ "io/flutter/embedding/android/TransparencyMode.java", "io/flutter/embedding/engine/FlutterEngine.java", "io/flutter/embedding/engine/FlutterEngineCache.java", - "io/flutter/embedding/engine/FlutterEnginePluginRegistry.java", + "io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java", "io/flutter/embedding/engine/FlutterJNI.java", "io/flutter/embedding/engine/FlutterOverlaySurface.java", "io/flutter/embedding/engine/FlutterShellArgs.java", @@ -430,7 +430,7 @@ action("robolectric_tests") { "test/io/flutter/embedding/android/FlutterViewTest.java", "test/io/flutter/embedding/android/RobolectricFlutterActivity.java", "test/io/flutter/embedding/engine/FlutterEngineCacheTest.java", - "test/io/flutter/embedding/engine/FlutterEnginePluginRegistryTest.java", + "test/io/flutter/embedding/engine/FlutterEngineConnectionRegistryTest.java", "test/io/flutter/embedding/engine/FlutterEngineTest.java", "test/io/flutter/embedding/engine/FlutterJNITest.java", "test/io/flutter/embedding/engine/FlutterShellArgsTest.java", diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java index c0cb3aca34fcd..78585172dd683 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java @@ -187,7 +187,7 @@ void onAttach(@NonNull Context context) { "FlutterActivityAndFragmentDelegate's getAppComponent should only " + "be queried after onAttach, when the host's activity should always be non-null"); } - return host.getActivity(); + return activity; } /** diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java index 9ec50d7ef98eb..71730e4668a85 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java @@ -74,7 +74,7 @@ public class FlutterEngine { @NonNull private final FlutterJNI flutterJNI; @NonNull private final FlutterRenderer renderer; @NonNull private final DartExecutor dartExecutor; - @NonNull private final FlutterEnginePluginRegistry pluginRegistry; + @NonNull private final FlutterEngineConnectionRegistry pluginRegistry; @NonNull private final LocalizationPlugin localizationPlugin; // System channels. @@ -301,7 +301,7 @@ public FlutterEngine( this.platformViewsController.onAttachedToJNI(); this.pluginRegistry = - new FlutterEnginePluginRegistry(context.getApplicationContext(), this, flutterLoader); + new FlutterEngineConnectionRegistry(context.getApplicationContext(), this, flutterLoader); if (automaticallyRegisterPlugins) { registerPlugins(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java similarity index 98% rename from shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java rename to shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java index efd0b209609e3..bd6ff4a2c942e 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java @@ -37,13 +37,20 @@ import java.util.Map; import java.util.Set; -/* package */ class FlutterEnginePluginRegistry +/** + * This class is owned by the {@link FlutterEngine} and its role is to managed its connections with + * Android App Components and Flutter plugins. + * + * It enforces the {0|1}:1 relationship between activity and engine, and propagates the app + * component connection to the plugins. + */ +/* package */ class FlutterEngineConnectionRegistry implements PluginRegistry, ActivityControlSurface, ServiceControlSurface, BroadcastReceiverControlSurface, ContentProviderControlSurface { - private static final String TAG = "FlutterEnginePluginRegistry"; + private static final String TAG = "FlutterEngineConnectionRegistry"; // PluginRegistry @NonNull @@ -88,7 +95,7 @@ @Nullable private ContentProvider contentProvider; @Nullable private FlutterEngineContentProviderPluginBinding contentProviderPluginBinding; - FlutterEnginePluginRegistry( + FlutterEngineConnectionRegistry( @NonNull Context appContext, @NonNull FlutterEngine flutterEngine, @NonNull FlutterLoader flutterLoader) { diff --git a/shell/platform/android/test/io/flutter/FlutterTestSuite.java b/shell/platform/android/test/io/flutter/FlutterTestSuite.java index bb5c44c6568df..bdc130ef87e9a 100644 --- a/shell/platform/android/test/io/flutter/FlutterTestSuite.java +++ b/shell/platform/android/test/io/flutter/FlutterTestSuite.java @@ -12,7 +12,7 @@ import io.flutter.embedding.android.FlutterFragmentTest; import io.flutter.embedding.android.FlutterViewTest; import io.flutter.embedding.engine.FlutterEngineCacheTest; -import io.flutter.embedding.engine.FlutterEnginePluginRegistryTest; +import io.flutter.embedding.engine.FlutterEngineConnectionRegistryTest; import io.flutter.embedding.engine.FlutterJNITest; import io.flutter.embedding.engine.LocalizationPluginTest; import io.flutter.embedding.engine.RenderingComponentTest; @@ -52,7 +52,7 @@ FlutterActivityTest.class, FlutterAndroidComponentTest.class, FlutterEngineCacheTest.class, - FlutterEnginePluginRegistryTest.class, + FlutterEngineConnectionRegistryTest.class, FlutterEngineTest.class, FlutterFragmentActivityTest.class, FlutterFragmentTest.class, diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEnginePluginRegistryTest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineConnectionRegistryTest.java similarity index 95% rename from shell/platform/android/test/io/flutter/embedding/engine/FlutterEnginePluginRegistryTest.java rename to shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineConnectionRegistryTest.java index ee653348004a1..6037ca8c4e11e 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterEnginePluginRegistryTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterEngineConnectionRegistryTest.java @@ -26,7 +26,7 @@ // Run with Robolectric so that Log calls don't crash. @Config(manifest = Config.NONE) @RunWith(RobolectricTestRunner.class) -public class FlutterEnginePluginRegistryTest { +public class FlutterEngineConnectionRegistryTest { @Test public void itDoesNotRegisterTheSamePluginTwice() { Context context = mock(Context.class); @@ -40,8 +40,8 @@ public void itDoesNotRegisterTheSamePluginTwice() { FakeFlutterPlugin fakePlugin1 = new FakeFlutterPlugin(); FakeFlutterPlugin fakePlugin2 = new FakeFlutterPlugin(); - FlutterEnginePluginRegistry registry = - new FlutterEnginePluginRegistry(context, flutterEngine, flutterLoader); + FlutterEngineConnectionRegistry registry = + new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader); // Verify that the registry doesn't think it contains our plugin yet. assertFalse(registry.has(fakePlugin1.getClass())); @@ -80,8 +80,8 @@ public void activityResultListenerCanBeRemovedFromListener() { AtomicBoolean isFirstCall = new AtomicBoolean(true); // setup the environment to get the required internal data - FlutterEnginePluginRegistry registry = - new FlutterEnginePluginRegistry(context, flutterEngine, flutterLoader); + FlutterEngineConnectionRegistry registry = + new FlutterEngineConnectionRegistry(context, flutterEngine, flutterLoader); FakeActivityAwareFlutterPlugin fakePlugin = new FakeActivityAwareFlutterPlugin(); registry.add(fakePlugin); registry.attachToActivity(activity, lifecycle); From 9537f3313239f0336e57e7e1108d8def96d31c57 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Mon, 21 Sep 2020 16:12:57 -0700 Subject: [PATCH 10/12] autoformat --- .../embedding/engine/FlutterEngineConnectionRegistry.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java index bd6ff4a2c942e..729bc1d842595 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEngineConnectionRegistry.java @@ -41,7 +41,7 @@ * This class is owned by the {@link FlutterEngine} and its role is to managed its connections with * Android App Components and Flutter plugins. * - * It enforces the {0|1}:1 relationship between activity and engine, and propagates the app + *

    It enforces the {0|1}:1 relationship between activity and engine, and propagates the app * component connection to the plugins. */ /* package */ class FlutterEngineConnectionRegistry From 7281a80f047aa0ea72600e4a4e9bbbe2dcc487ca Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Mon, 21 Sep 2020 22:43:35 -0700 Subject: [PATCH 11/12] make events droppable --- .../embedding/android/FlutterActivity.java | 54 ++++---- .../embedding/android/FlutterFragment.java | 123 +++++++++--------- 2 files changed, 87 insertions(+), 90 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java index e3c4b6e82a34e..e542e56f0002a 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java @@ -533,7 +533,6 @@ private void configureStatusBarForFullscreenFlutterExperience() { @Override protected void onStart() { super.onStart(); - ensureAlive(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_START); delegate.onStart(); } @@ -541,7 +540,6 @@ protected void onStart() { @Override protected void onResume() { super.onResume(); - ensureAlive(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_RESUME); delegate.onResume(); } @@ -549,14 +547,12 @@ protected void onResume() { @Override public void onPostResume() { super.onPostResume(); - ensureAlive(); delegate.onPostResume(); } @Override protected void onPause() { super.onPause(); - ensureAlive(); delegate.onPause(); lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_PAUSE); } @@ -564,10 +560,8 @@ protected void onPause() { @Override protected void onStop() { super.onStop(); - if (delegate != null) { + if (stillAttachedForEvent("onStop")) { delegate.onStop(); - } else { - Log.v(TAG, "FlutterActivity " + this + " onStop called after release."); } lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_STOP); } @@ -575,10 +569,8 @@ protected void onStop() { @Override protected void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); - if (delegate != null) { + if (stillAttachedForEvent("onSaveInstanceState")) { delegate.onSaveInstanceState(outState); - } else { - Log.v(TAG, "FlutterActivity " + this + " onSaveInstanceState called after release."); } } @@ -614,52 +606,56 @@ public void detachFromFlutterEngine() { @Override protected void onDestroy() { super.onDestroy(); - if (delegate != null) { + if (stillAttachedForEvent("onDestroy")) { release(); - } else { - Log.v(TAG, "FlutterActivity " + this + " onDestroy called after release."); } lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY); } @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { - ensureAlive(); - delegate.onActivityResult(requestCode, resultCode, data); + if (stillAttachedForEvent("onActivityResult")) { + delegate.onActivityResult(requestCode, resultCode, data); + } } @Override protected void onNewIntent(@NonNull Intent intent) { // TODO(mattcarroll): change G3 lint rule that forces us to call super super.onNewIntent(intent); - ensureAlive(); - delegate.onNewIntent(intent); + if (stillAttachedForEvent("onNewIntent")) { + delegate.onNewIntent(intent); + } } @Override public void onBackPressed() { - ensureAlive(); - delegate.onBackPressed(); + if (stillAttachedForEvent("onBackPressed")) { + delegate.onBackPressed(); + } } @Override public void onRequestPermissionsResult( - int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { - ensureAlive(); - delegate.onRequestPermissionsResult(requestCode, permissions, grantResults); + int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { + if (stillAttachedForEvent("onRequestPermissionsResult")) { + delegate.onRequestPermissionsResult(requestCode, permissions, grantResults); + } } @Override public void onUserLeaveHint() { - ensureAlive(); - delegate.onUserLeaveHint(); + if (stillAttachedForEvent("onUserLeaveHint")) { + delegate.onUserLeaveHint(); + } } @Override public void onTrimMemory(int level) { super.onTrimMemory(level); - ensureAlive(); - delegate.onTrimMemory(level); + if (stillAttachedForEvent("onTrimMemory")) { + delegate.onTrimMemory(level); + } } /** @@ -1012,9 +1008,11 @@ public boolean shouldRestoreAndSaveState() { return true; } - private void ensureAlive() { + private boolean stillAttachedForEvent(String event) { if (delegate == null) { - throw new IllegalStateException("Cannot execute method on a destroyed FlutterActivity."); + Log.v(TAG, "FlutterActivity " + hashCode() + " " + event + " called after release."); + return false; } + return true; } } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java index e957106efdda7..2e5662900295a 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java @@ -598,14 +598,12 @@ public void onActivityCreated(@Nullable Bundle savedInstanceState) { @Override public void onStart() { super.onStart(); - ensureAlive(); delegate.onStart(); } @Override public void onResume() { super.onResume(); - ensureAlive(); delegate.onResume(); } @@ -613,91 +611,84 @@ public void onResume() { // possible. @ActivityCallThrough public void onPostResume() { - ensureAlive(); delegate.onPostResume(); } @Override public void onPause() { super.onPause(); - ensureAlive(); delegate.onPause(); } @Override public void onStop() { super.onStop(); - if (delegate != null) { + if (stillAttachedForEvent("onStop")) { delegate.onStop(); - } else { - Log.v(TAG, "FlutterFragment " + this + " onStop called after release."); } } @Override public void onDestroyView() { super.onDestroyView(); - if (delegate != null) { + if (stillAttachedForEvent("onDestroyView")) { delegate.onDestroyView(); - } else { - Log.v(TAG, "FlutterFragment " + this + " onDestroyView called after release."); } } @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); - if (delegate != null) { + if (stillAttachedForEvent("onSaveInstanceState")) { delegate.onSaveInstanceState(outState); - } else { - Log.v(TAG, "FlutterFragment " + this + " onSaveInstanceState called after release."); } } @Override public void detachFromFlutterEngine() { Log.v( - TAG, - "FlutterFragment " - + this - + " connection to the engine " - + getFlutterEngine() - + " evicted by another attaching activity"); - // Redundant calls are ok. - delegate.onDestroyView(); - delegate.onDetach(); - delegate.release(); - delegate = null; - } - - @Override - public void onDetach() { - super.onDetach(); - if (delegate != null) { + TAG, + "FlutterFragment " + + this + + " connection to the engine " + + getFlutterEngine() + + " evicted by another attaching activity"); + // Redundant calls are ok. + delegate.onDestroyView(); delegate.onDetach(); delegate.release(); delegate = null; - } else { - Log.v(TAG, "FlutterFragment " + this + " onDetach called after release."); } - } - /** - * The result of a permission request has been received. - * - *

    See {@link android.app.Activity#onRequestPermissionsResult(int, String[], int[])} - * - *

    - * - * @param requestCode identifier passed with the initial permission request - * @param permissions permissions that were requested - * @param grantResults permission grants or denials - */ - @ActivityCallThrough - public void onRequestPermissionsResult( + @Override + public void onDetach() { + super.onDetach(); + if (delegate != null) { + delegate.onDetach(); + delegate.release(); + delegate = null; + } else { + Log.v(TAG, "FlutterFragment " + this + " onDetach called after release."); + } + } + + /** + * The result of a permission request has been received. + * + *

    See {@link android.app.Activity#onRequestPermissionsResult(int, String[], int[])} + * + *

    + * + * @param requestCode identifier passed with the initial permission request + * @param permissions permissions that were requested + * @param grantResults permission grants or denials + */ + @ActivityCallThrough + public void onRequestPermissionsResult( int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { - ensureAlive(); - delegate.onRequestPermissionsResult(requestCode, permissions, grantResults); + if (stillAttachedForEvent("onRequestPermissionsResult")) { + delegate.onRequestPermissionsResult(requestCode, permissions, grantResults); + } } /** @@ -712,8 +703,9 @@ public void onRequestPermissionsResult( */ @ActivityCallThrough public void onNewIntent(@NonNull Intent intent) { - ensureAlive(); - delegate.onNewIntent(intent); + if (stillAttachedForEvent("onNewIntent")) { + delegate.onNewIntent(intent); + } } /** @@ -723,8 +715,9 @@ public void onNewIntent(@NonNull Intent intent) { */ @ActivityCallThrough public void onBackPressed() { - ensureAlive(); - delegate.onBackPressed(); + if (stillAttachedForEvent("onBackPressed")) { + delegate.onBackPressed(); + } } /** @@ -739,8 +732,9 @@ public void onBackPressed() { */ @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { - ensureAlive(); - delegate.onActivityResult(requestCode, resultCode, data); + if (stillAttachedForEvent("onActivityResult")) { + delegate.onActivityResult(requestCode, resultCode, data); + } } /** @@ -751,8 +745,9 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) { */ @ActivityCallThrough public void onUserLeaveHint() { - ensureAlive(); - delegate.onUserLeaveHint(); + if (stillAttachedForEvent("onUserLeaveHint")) { + delegate.onUserLeaveHint(); + } } /** @@ -766,8 +761,9 @@ public void onUserLeaveHint() { */ @ActivityCallThrough public void onTrimMemory(int level) { - ensureAlive(); - delegate.onTrimMemory(level); + if (stillAttachedForEvent("onTrimMemory")) { + delegate.onTrimMemory(level); + } } /** @@ -778,8 +774,9 @@ public void onTrimMemory(int level) { @Override public void onLowMemory() { super.onLowMemory(); - ensureAlive(); - delegate.onLowMemory(); + if (stillAttachedForEvent("onLowMemory")) { + delegate.onLowMemory(); + } } /** @@ -1078,10 +1075,12 @@ public boolean shouldRestoreAndSaveState() { return true; } - private void ensureAlive() { + private boolean stillAttachedForEvent(String event) { if (delegate == null) { - throw new IllegalStateException("Cannot execute method on a destroyed FlutterFragment."); + Log.v(TAG, "FlutterFragment " + hashCode() + " " + event + " called after release."); + return false; } + return true; } /** From 88442f56911867d4894fbeee0481444f8fbfd9ab Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Tue, 22 Sep 2020 09:43:04 -0700 Subject: [PATCH 12/12] autoformat --- .../embedding/android/FlutterActivity.java | 2 +- .../embedding/android/FlutterFragment.java | 66 +++++++++---------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java index e542e56f0002a..66c2acd8e886b 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterActivity.java @@ -637,7 +637,7 @@ public void onBackPressed() { @Override public void onRequestPermissionsResult( - int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { + int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { if (stillAttachedForEvent("onRequestPermissionsResult")) { delegate.onRequestPermissionsResult(requestCode, permissions, grantResults); } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java index 2e5662900295a..4d2438bd95935 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterFragment.java @@ -647,44 +647,44 @@ public void onSaveInstanceState(Bundle outState) { @Override public void detachFromFlutterEngine() { Log.v( - TAG, - "FlutterFragment " - + this - + " connection to the engine " - + getFlutterEngine() - + " evicted by another attaching activity"); - // Redundant calls are ok. - delegate.onDestroyView(); + TAG, + "FlutterFragment " + + this + + " connection to the engine " + + getFlutterEngine() + + " evicted by another attaching activity"); + // Redundant calls are ok. + delegate.onDestroyView(); + delegate.onDetach(); + delegate.release(); + delegate = null; + } + + @Override + public void onDetach() { + super.onDetach(); + if (delegate != null) { delegate.onDetach(); delegate.release(); delegate = null; + } else { + Log.v(TAG, "FlutterFragment " + this + " onDetach called after release."); } + } - @Override - public void onDetach() { - super.onDetach(); - if (delegate != null) { - delegate.onDetach(); - delegate.release(); - delegate = null; - } else { - Log.v(TAG, "FlutterFragment " + this + " onDetach called after release."); - } - } - - /** - * The result of a permission request has been received. - * - *

    See {@link android.app.Activity#onRequestPermissionsResult(int, String[], int[])} - * - *

    - * - * @param requestCode identifier passed with the initial permission request - * @param permissions permissions that were requested - * @param grantResults permission grants or denials - */ - @ActivityCallThrough - public void onRequestPermissionsResult( + /** + * The result of a permission request has been received. + * + *

    See {@link android.app.Activity#onRequestPermissionsResult(int, String[], int[])} + * + *

    + * + * @param requestCode identifier passed with the initial permission request + * @param permissions permissions that were requested + * @param grantResults permission grants or denials + */ + @ActivityCallThrough + public void onRequestPermissionsResult( int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { if (stillAttachedForEvent("onRequestPermissionsResult")) { delegate.onRequestPermissionsResult(requestCode, permissions, grantResults);