review

Author: xster

shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java

@@ -14,6 +14,7 @@
 import android.view.WindowManager;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import io.flutter.BuildConfig;
 import io.flutter.embedding.engine.FlutterJNI;
 import io.flutter.util.PathUtils;
@@ -47,7 +48,10 @@ public class FlutterLoader {
    * <p>The returned instance loads Flutter native libraries in the standard way. A singleton object
    * is used instead of static methods to facilitate testing without actually running native library
    * linking.
+   *
+   * @deprecated Use the {@link io.flutter.FlutterInjector} instead.
    */
+  @Deprecated
   @NonNull
   public static FlutterLoader getInstance() {
     if (instance == null) {
@@ -56,13 +60,6 @@ public static FlutterLoader getInstance() {
     return instance;
   }
 
-  @NonNull
-  public static FlutterLoader getInstanceForTest(FlutterApplicationInfo flutterApplicationInfo) {
-    FlutterLoader loader = new FlutterLoader();
-    loader.flutterApplicationInfo = flutterApplicationInfo;
-    return loader;
-  }
-
   private boolean initialized = false;
   @Nullable private Settings settings;
   private long initStartTimestampMillis;
@@ -91,6 +88,12 @@ public void startInitialization(@NonNull Context applicationContext) {
     startInitialization(applicationContext, new Settings());
   }
 
+  @VisibleForTesting
+  public void loadNativeLibrary() {
+    // Let this be mockable.
+    System.loadLibrary("flutter");
+  }
+
   /**
    * Starts initialization of the native system.
    *
@@ -128,7 +131,7 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se
           public InitResult call() {
             ResourceExtractor resourceExtractor = initResources(appContext);
 
-            System.loadLibrary("flutter");
+            loadNativeLibrary();
 
             // Prefetch the default font manager as soon as possible on a background thread.
             // It helps to reduce time cost of engine setup that blocks the platform thread.

shell/platform/android/test/io/flutter/embedding/engine/PluginComponentTest.java

@@ -2,7 +2,9 @@
 
 import static junit.framework.TestCase.assertEquals;
 import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import androidx.annotation.NonNull;
@@ -13,8 +15,6 @@
 import io.flutter.embedding.engine.plugins.FlutterPlugin;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
@@ -30,43 +30,17 @@ public void pluginsCanAccessFlutterAssetPaths() {
     FlutterApplicationInfo emptyInfo =
         new FlutterApplicationInfo(null, null, null, null, null, null, false, false);
 
-    // FlutterLoader is the object to which the PluginRegistry defers for obtaining
-    // the path to a Flutter asset. Ideally in this component test we would use a
-    // real FlutterLoader and directly verify the relationship between FlutterAssets
-    // and FlutterLoader. However, a real FlutterLoader cannot be used in a JVM test
-    // because it would attempt to load native libraries. Therefore, we create a fake
-    // FlutterLoader, but then we defer the corresponding asset lookup methods to the
-    // real FlutterLoader singleton. This test ends up verifying that when FlutterAssets
-    // is queried for an asset path, it returns the real expected path based on real
-    // FlutterLoader behavior.
-    FlutterLoader flutterLoader = mock(FlutterLoader.class);
-    when(flutterLoader.getLookupKeyForAsset(any(String.class)))
-        .thenAnswer(
-            new Answer<String>() {
-              @Override
-              public String answer(InvocationOnMock invocation) throws Throwable {
-                // Defer to a real FlutterLoader to return the asset path.
-                String fileNameOrSubpath = (String) invocation.getArguments()[0];
-                return FlutterLoader.getInstanceForTest(emptyInfo)
-                    .getLookupKeyForAsset(fileNameOrSubpath);
-              }
-            });
-    when(flutterLoader.getLookupKeyForAsset(any(String.class), any(String.class)))
-        .thenAnswer(
-            new Answer<String>() {
-              @Override
-              public String answer(InvocationOnMock invocation) throws Throwable {
-                // Defer to a real FlutterLoader to return the asset path.
-                String fileNameOrSubpath = (String) invocation.getArguments()[0];
-                String packageName = (String) invocation.getArguments()[1];
-                return FlutterLoader.getInstanceForTest(emptyInfo)
-                    .getLookupKeyForAsset(fileNameOrSubpath, packageName);
-              }
-            });
+    FlutterLoader realFlutterLoader = new FlutterLoader();
+    FlutterLoader spyFlutterLoader = spy(realFlutterLoader);
+
+    // Let the real startInitialization be called, but mock the rest so it doesn't try to load
+    // the real native library.
+    doNothing().when(spyFlutterLoader).loadNativeLibrary();
+    doNothing().when(spyFlutterLoader).ensureInitializationComplete(any(), any());
 
     // Execute behavior under test.
     FlutterEngine flutterEngine =
-        new FlutterEngine(RuntimeEnvironment.application, flutterLoader, flutterJNI);
+        new FlutterEngine(RuntimeEnvironment.application, spyFlutterLoader, flutterJNI);
 
     // As soon as our plugin is registered it will look up asset paths and store them
     // for our verification.