Release shim bindings when detaching (#13432)

Author: xster

shell/platform/android/BUILD.gn

@@ -424,6 +424,7 @@ action("robolectric_tests") {
     "test/io/flutter/embedding/engine/FlutterJNITest.java",
     "test/io/flutter/embedding/engine/RenderingComponentTest.java",
     "test/io/flutter/embedding/engine/dart/DartExecutorTest.java",
+    "test/io/flutter/embedding/engine/plugins/shim/ShimPluginRegistryTest.java",
     "test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java",
     "test/io/flutter/embedding/engine/systemchannels/PlatformChannelTest.java",
     "test/io/flutter/plugin/common/StandardMessageCodecTest.java",

shell/platform/android/io/flutter/embedding/engine/plugins/shim/ShimPluginRegistry.java

@@ -19,8 +19,8 @@
 import io.flutter.plugin.common.PluginRegistry;
 
 /**
- * A {@link PluginRegistry} that is shimmed to use the new Android embedding and plugin API behind
- * the scenes.
+ * A {@link PluginRegistry} that is shimmed to let old plugins use the new Android embedding and
+ * plugin API behind the scenes.
  * <p>
  * The following is an example usage of {@code ShimPluginRegistry} within a {@code FlutterActivity}:
  * {@code
@@ -114,6 +114,7 @@ public void onDetachedFromEngine(@NonNull FlutterPluginBinding binding) {
         shimRegistrar.onDetachedFromEngine(binding);
       }
       flutterPluginBinding = null;
+      activityPluginBinding = null;
     }
 
     @Override
@@ -134,6 +135,7 @@ public void onDetachedFromActivityForConfigChanges() {
 
     @Override
     public void onReattachedToActivityForConfigChanges(@NonNull ActivityPluginBinding binding) {
+      activityPluginBinding = binding;
       for (ShimRegistrar shimRegistrar : shimRegistrars) {
         shimRegistrar.onReattachedToActivityForConfigChanges(binding);
       }
@@ -144,6 +146,7 @@ public void onDetachedFromActivity() {
       for (ShimRegistrar shimRegistrar : shimRegistrars) {
         shimRegistrar.onDetachedFromActivity();
       }
+      activityPluginBinding = null;
     }
   }
 }

shell/platform/android/io/flutter/embedding/engine/plugins/shim/ShimRegistrar.java

@@ -24,8 +24,8 @@
 import io.flutter.view.TextureRegistry;
 
 /**
- * A {@link PluginRegistry.Registrar} that is shimmed to use the new Android embedding and plugin
- * API behind the scenes.
+ * A {@link PluginRegistry.Registrar} that is shimmed let old plugins use the new Android embedding
+ * and plugin API behind the scenes.
  * <p>
  * Instances of {@code ShimRegistrar}s are vended internally by a {@link ShimPluginRegistry}.
  */
@@ -165,6 +165,7 @@ public void onDetachedFromEngine(@NonNull FlutterPluginBinding binding) {
     }
 
     pluginBinding = null;
+    activityPluginBinding = null;
   }
 
   @Override

shell/platform/android/test/io/flutter/FlutterTestSuite.java

@@ -15,6 +15,7 @@
 import io.flutter.embedding.engine.FlutterEngineCacheTest;
 import io.flutter.embedding.engine.FlutterJNITest;
 import io.flutter.embedding.engine.RenderingComponentTest;
+import io.flutter.embedding.engine.plugins.shim.ShimPluginRegistryTest;
 import io.flutter.embedding.engine.renderer.FlutterRendererTest;
 import io.flutter.embedding.engine.systemchannels.PlatformChannelTest;
 import io.flutter.plugin.common.StandardMessageCodecTest;
@@ -40,6 +41,7 @@
     PreconditionsTest.class,
     RenderingComponentTest.class,
     StandardMessageCodecTest.class,
+    ShimPluginRegistryTest.class,
     SingleViewPresentationTest.class,
     SmokeTest.class,
     TextInputPluginTest.class,

shell/platform/android/test/io/flutter/embedding/engine/plugins/shim/ShimPluginRegistryTest.java

@@ -0,0 +1,144 @@
+package io.flutter.embedding.engine.plugins.shim;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+import android.app.Activity;
+import android.content.Context;
+import io.flutter.embedding.engine.FlutterEngine;
+import io.flutter.embedding.engine.plugins.FlutterPlugin;
+import io.flutter.embedding.engine.plugins.PluginRegistry;
+import io.flutter.embedding.engine.plugins.FlutterPlugin.FlutterPluginBinding;
+import io.flutter.embedding.engine.plugins.activity.ActivityAware;
+import io.flutter.embedding.engine.plugins.activity.ActivityPluginBinding;
+import io.flutter.embedding.engine.plugins.shim.ShimPluginRegistry;
+import io.flutter.plugin.common.PluginRegistry.Registrar;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+@Config(manifest=Config.NONE)
+@RunWith(RobolectricTestRunner.class)
+public class ShimPluginRegistryTest {
+
+  @Mock private FlutterEngine mockFlutterEngine;
+  @Mock private FlutterPluginBinding mockFlutterPluginBinding;
+  @Mock private ActivityPluginBinding mockActivityPluginBinding;
+  @Mock private PluginRegistry mockPluginRegistry;
+  @Mock private Context mockApplicationContext;
+  @Mock private Activity mockActivity;
+
+  @Before
+  public void setup() {
+    MockitoAnnotations.initMocks(this);
+    when(mockFlutterEngine.getPlugins()).thenReturn(mockPluginRegistry);
+    when(mockFlutterPluginBinding.getApplicationContext()).thenReturn(mockApplicationContext);
+    when(mockActivityPluginBinding.getActivity()).thenReturn(mockActivity);
+  }
+
+  @Test
+  public void itSuppliesOldAPIsViaTheNewFlutterPluginBinding() {
+    ShimPluginRegistry registryUnderTest = new ShimPluginRegistry(mockFlutterEngine);
+    // This is the consumption side of the old plugins.
+    Registrar registrarUnderTest = registryUnderTest.registrarFor("test");
+
+    ArgumentCaptor<FlutterPlugin> shimAggregateCaptor = ArgumentCaptor.forClass(FlutterPlugin.class);
+    // A single shim aggregate was added as a new plugin to the FlutterEngine's PluginRegistry.
+    verify(mockPluginRegistry).add(shimAggregateCaptor.capture());
+    // This is really a ShimRegistrarAggregate acting as a FlutterPlugin which is the
+    // intermediate consumption side of the new plugin inside the shim.
+    FlutterPlugin shimAggregateUnderTest = shimAggregateCaptor.getValue();
+    // The FlutterPluginBinding is the supply side of the new plugin.
+    shimAggregateUnderTest.onAttachedToEngine(mockFlutterPluginBinding);
+
+    // Consume something from the old plugin API.
+    assertEquals(mockApplicationContext, registrarUnderTest.context());
+    // Check that the value comes from the supply side of the new plugin.
+    verify(mockFlutterPluginBinding).getApplicationContext();
+  }
+
+  @Test
+  public void itSuppliesMultipleOldPlugins() {
+    ShimPluginRegistry registryUnderTest = new ShimPluginRegistry(mockFlutterEngine);
+    Registrar registrarUnderTest1 = registryUnderTest.registrarFor("test1");
+    Registrar registrarUnderTest2 = registryUnderTest.registrarFor("test2");
+
+    ArgumentCaptor<FlutterPlugin> shimAggregateCaptor = ArgumentCaptor.forClass(FlutterPlugin.class);
+    verify(mockPluginRegistry).add(shimAggregateCaptor.capture());
+    // There's only one aggregate for many old plugins.
+    FlutterPlugin shimAggregateUnderTest = shimAggregateCaptor.getValue();
+
+    // The FlutterPluginBinding is the supply side of the new plugin.
+    shimAggregateUnderTest.onAttachedToEngine(mockFlutterPluginBinding);
+
+    // Since the 2 old plugins are supplied by the same intermediate FlutterPlugin, they should
+    // get the same value.
+    assertEquals(registrarUnderTest1.context(), registrarUnderTest2.context());
+    verify(mockFlutterPluginBinding, times(2)).getApplicationContext();
+  }
+
+  @Test
+  public void itCanOnlySupplyActivityBindingWhenUpstreamActivityIsAttached() {
+    ShimPluginRegistry registryUnderTest = new ShimPluginRegistry(mockFlutterEngine);
+    Registrar registrarUnderTest = registryUnderTest.registrarFor("test");
+
+    ArgumentCaptor<FlutterPlugin> shimAggregateCaptor = ArgumentCaptor.forClass(FlutterPlugin.class);
+    verify(mockPluginRegistry).add(shimAggregateCaptor.capture());
+    FlutterPlugin shimAggregateAsPlugin = shimAggregateCaptor.getValue();
+    ActivityAware shimAggregateAsActivityAware = (ActivityAware) shimAggregateCaptor.getValue();
+
+    // Nothing is retrievable when nothing is attached.
+    assertNull(registrarUnderTest.context());
+    assertNull(registrarUnderTest.activity());
+
+    shimAggregateAsPlugin.onAttachedToEngine(mockFlutterPluginBinding);
+
+    assertEquals(mockApplicationContext, registrarUnderTest.context());
+    assertNull(registrarUnderTest.activity());
+
+    shimAggregateAsActivityAware.onAttachedToActivity(mockActivityPluginBinding);
+
+    // Now context is the activity context.
+    assertEquals(mockActivity, registrarUnderTest.activeContext());
+    assertEquals(mockActivity, registrarUnderTest.activity());
+
+    shimAggregateAsActivityAware.onDetachedFromActivityForConfigChanges();
+
+    assertEquals(mockApplicationContext, registrarUnderTest.activeContext());
+    assertNull(registrarUnderTest.activity());
+
+    shimAggregateAsActivityAware.onReattachedToActivityForConfigChanges(mockActivityPluginBinding);
+    assertEquals(mockActivity, registrarUnderTest.activeContext());
+    assertEquals(mockActivity, registrarUnderTest.activity());
+
+    shimAggregateAsActivityAware.onDetachedFromActivity();
+
+    assertEquals(mockApplicationContext, registrarUnderTest.activeContext());
+    assertNull(registrarUnderTest.activity());
+
+    // Attach an activity again.
+    shimAggregateAsActivityAware.onAttachedToActivity(mockActivityPluginBinding);
+
+    assertEquals(mockActivity, registrarUnderTest.activeContext());
+    assertEquals(mockActivity, registrarUnderTest.activity());
+
+    // Now rip out the whole engine.
+    shimAggregateAsPlugin.onDetachedFromEngine(mockFlutterPluginBinding);
+
+    // And everything should have been made unavailable.
+    assertNull(registrarUnderTest.activeContext());
+    assertNull(registrarUnderTest.activity());
+  }
+}