Refactor and rename methods

GaryQian · GaryQian · commit e075ef5823ba · 2020-11-03T23:53:17.000-08:00
diff --git a/shell/common/engine.cc b/shell/common/engine.cc
@@ -511,8 +511,8 @@ const std::string& Engine::GetLastEntrypointLibrary() const {
 // feature. Left commented out until it lands:
 
 // // |RuntimeDelegate|
-// void Engine::OnDartLoadLibrary(intptr_t loading_unit_id) {
-//   return delegate_.OnDartLoadLibrary(loading_unit_id);
+// void Engine::InstallDeferredLibrary(intptr_t loading_unit_id) {
+//   return delegate_.InstallDeferredLibrary(loading_unit_id);
 // }
 
 void Engine::CompleteDartLoadLibrary(intptr_t loading_unit_id,
diff --git a/shell/common/engine.h b/shell/common/engine.h
@@ -270,7 +270,7 @@ class Engine final : public RuntimeDelegate,
     /// @param[in]  loading_unit_id  The unique id of the deferred library's
     ///                              loading unit.
     ///
-    virtual void OnDartLoadLibrary(intptr_t loading_unit_id) = 0;
+    virtual void InstallDeferredLibrary(intptr_t loading_unit_id) = 0;
   };
 
   //----------------------------------------------------------------------------
@@ -851,7 +851,7 @@ class Engine final : public RuntimeDelegate,
   // feature. Left commented out until it lands:
 
   // // |RuntimeDelegate|
-  // void OnDartLoadLibrary(intptr_t loading_unit_id) override;
+  // void InstallDeferredLibrary(intptr_t loading_unit_id) override;
 
   void SetNeedsReportTimings(bool value) override;
 
diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc
@@ -33,7 +33,7 @@ class MockDelegate : public Engine::Delegate {
   MOCK_METHOD1(ComputePlatformResolvedLocale,
                std::unique_ptr<std::vector<std::string>>(
                    const std::vector<std::string>&));
-  MOCK_METHOD1(OnDartLoadLibrary, Dart_Handle(intptr_t));
+  MOCK_METHOD1(InstallDeferredLibrary, void(intptr_t));
 };
 
 class MockResponse : public PlatformMessageResponse {
@@ -57,7 +57,7 @@ class MockRuntimeDelegate : public RuntimeDelegate {
   MOCK_METHOD1(ComputePlatformResolvedLocale,
                std::unique_ptr<std::vector<std::string>>(
                    const std::vector<std::string>&));
-  MOCK_METHOD1(OnDartLoadLibrary, Dart_Handle(intptr_t));
+  MOCK_METHOD1(InstallDeferredLibrary, void(intptr_t));
 };
 
 class MockRuntimeController : public RuntimeController {
diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc
@@ -152,7 +152,7 @@ PlatformView::ComputePlatformResolvedLocales(
   return out;
 }
 
-void PlatformView::OnDartLoadLibrary(intptr_t loading_unit_id) {}
+void PlatformView::InstallDeferredLibrary(intptr_t loading_unit_id) {}
 
 void PlatformView::CompleteDartLoadLibrary(
     intptr_t loading_unit_id,
diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h
@@ -629,7 +629,7 @@ class PlatformView {
   /// @param[in]  loading_unit_id  The unique id of the deferred library's
   ///                              loading unit.
   ///
-  virtual void OnDartLoadLibrary(intptr_t loading_unit_id);
+  virtual void InstallDeferredLibrary(intptr_t loading_unit_id);
 
   //--------------------------------------------------------------------------
   /// @brief      Loads the dart shared library into the dart VM. When the
diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -1200,13 +1200,13 @@ void Shell::UpdateAssetManager(std::shared_ptr<AssetManager> asset_manager) {
 }
 
 // |Engine::Delegate|
-void Shell::OnDartLoadLibrary(intptr_t loading_unit_id) {
+void Shell::InstallDeferredLibrary(intptr_t loading_unit_id) {
   OnPlatformViewDartLoadLibrary(loading_unit_id);
 }
 
 // |PlatformView::Delegate|
 void Shell::OnPlatformViewDartLoadLibrary(intptr_t loading_unit_id) {
-  platform_view_->OnDartLoadLibrary(loading_unit_id);
+  platform_view_->InstallDeferredLibrary(loading_unit_id);
 }
 
 void Shell::ReportTimings() {
diff --git a/shell/common/shell.h b/shell/common/shell.h
@@ -565,7 +565,7 @@ class Shell final : public PlatformView::Delegate,
       const std::vector<std::string>& supported_locale_data) override;
 
   // |Engine::Delegate|
-  void OnDartLoadLibrary(intptr_t loading_unit_id) override;
+  void InstallDeferredLibrary(intptr_t loading_unit_id) override;
 
   // |Rasterizer::Delegate|
   void OnFrameRasterized(const FrameTiming&) override;
diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java
@@ -306,7 +306,6 @@ public FlutterEngine(
 
     dynamicFeatureManager = new PlayStoreDynamicFeatureManager(context, flutterJNI);
     flutterJNI.setDynamicFeatureManager(dynamicFeatureManager);
-    flutterJNI.setDynamicFeatureContext(context);
 
     attachToJni();
 
diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
@@ -951,31 +951,11 @@ public void setDynamicFeatureManager(@NonNull DynamicFeatureManager dynamicFeatu
     this.dynamicFeatureManager = dynamicFeatureManager;
   }
 
-  private Context dynamicFeatureContext;
-
-  @UiThread
-  public void setDynamicFeatureContext(@NonNull Context context) {
-    ensureRunningOnMainThread();
-    this.dynamicFeatureContext = context;
-  }
-
-  @SuppressWarnings("unused")
-  @UiThread
-  public void downloadDynamicFeature(int loadingUnitId) {
-    int moduleNameIdentifier =
-        dynamicFeatureContext
-            .getResources()
-            .getIdentifier(
-                "loadingUnit" + loadingUnitId, "string", dynamicFeatureContext.getPackageName());
-    String moduleName = dynamicFeatureContext.getResources().getString(moduleNameIdentifier);
-    downloadDynamicFeature(moduleName, loadingUnitId);
-  }
-
-  // Called by the engine upon invocation of dart loadLibrary() request
   @SuppressWarnings("unused")
   @UiThread
-  public void downloadDynamicFeature(String moduleName, int loadingUnitId) {
-    dynamicFeatureManager.downloadFeature(moduleName, loadingUnitId);
+  // public void downloadDynamicFeature(int loadingUnitId) {
+  public void installDeferredLibrary(int loadingUnitId) {
+    dynamicFeatureManager.downloadDynamicFeature(loadingUnitId);
   }
 
   /**
diff --git a/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/DynamicFeatureManager.java b/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/DynamicFeatureManager.java
@@ -26,7 +26,7 @@ public interface DynamicFeatureManager {
    * download, loadAssets and loadDartLibrary should be called to complete the dynamic feature load
    * process.
    */
-  public abstract void downloadFeature(String moduleName, int loadingUnitId);
+  public abstract void downloadDynamicFeature(int loadingUnitId);
 
   /**
    * Extract and load any assets and resources from the module for use by Flutter.
@@ -36,7 +36,7 @@ public interface DynamicFeatureManager {
    *
    * <p>Depending on the structure of the feature module, there may or may not be assets to extract.
    */
-  public abstract void loadAssets(String moduleName, int loadingUnitId);
+  public abstract void loadAssets(int loadingUnitId);
 
   /**
    * Load the .so shared library file into the Dart VM.
@@ -45,8 +45,8 @@ public interface DynamicFeatureManager {
    * considered finished loading, and the dart future completes. Developers are expected to begin
    * using symbols and assets from the feature module after completion.
    */
-  public abstract void loadDartLibrary(String moduleName, int loadingUnitId);
+  public abstract void loadDartLibrary(int loadingUnitId);
 
   /** Uninstall the specified feature module. */
-  public abstract void uninstallFeature(String moduleName, int loadingUnitId);
+  public abstract void uninstallFeature(int loadingUnitId);
 }
diff --git a/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java b/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java
@@ -73,13 +73,11 @@ public void onStateUpdate(SplitInstallSessionState state) {
                       "Module \"%s\" (sessionId %d) install successfully.",
                       sessionIdToName.get(state.sessionId()), state.sessionId()));
               loadAssets(
-                  sessionIdToName.get(state.sessionId()),
                   sessionIdToLoadingUnitId.get(state.sessionId()));
               // We only load dart shared lib for the loading unit id requested. Other loading units
               // (if present) in the dynamic feature module are not loaded, but can be loaded by
               // calling again with their loading unit id.
               loadDartLibrary(
-                  sessionIdToName.get(state.sessionId()),
                   sessionIdToLoadingUnitId.get(state.sessionId()));
               sessionIdToName.remove(state.sessionId());
               sessionIdToLoadingUnitId.remove(state.sessionId());
@@ -166,7 +164,17 @@ public PlayStoreDynamicFeatureManager(@NonNull Context context, @NonNull Flutter
     sessionIdToLoadingUnitId = new HashMap();
   }
 
-  public void downloadFeature(String moduleName, int loadingUnitId) {
+  private String loadingUnitIdToModuleName(int loadingUnitId) {
+    int moduleNameIdentifier =
+        context
+            .getResources()
+            .getIdentifier(
+                "loadingUnit" + loadingUnitId, "string", context.getPackageName());
+    return context.getResources().getString(moduleNameIdentifier);
+  }
+
+  public void downloadDynamicFeature(int loadingUnitId) {
+    String moduleName = loadingUnitIdToModuleName(loadingUnitId);
     if (moduleName == null) {
       Log.e(TAG, "Dynamic feature module name was null.");
       return;
@@ -222,7 +230,7 @@ public void downloadFeature(String moduleName, int loadingUnitId) {
             });
   }
 
-  public void loadAssets(@NonNull String moduleName, int loadingUnitId) {
+  public void loadAssets(int loadingUnitId) {
     try {
       context = context.createPackageContext(context.getPackageName(), 0);
 
@@ -232,12 +240,13 @@ public void loadAssets(@NonNull String moduleName, int loadingUnitId) {
           // TODO(garyq): Made the "flutter_assets" directory dynamic based off of DartEntryPoint.
           "flutter_assets");
     } catch (NameNotFoundException e) {
-      Log.d(TAG, "NameNotFoundException creating context for " + moduleName);
       throw new RuntimeException(e);
     }
   }
 
-  public void loadDartLibrary(String moduleName, int loadingUnitId) {
+  public void loadDartLibrary(int loadingUnitId) {
+    String moduleName = loadingUnitIdToModuleName(loadingUnitId);
+
     // This matches/depends on dart's loading unit naming convention, which we use unchanged.
     String aotSharedLibraryName = "app.so-" + loadingUnitId + ".part.so";
 
@@ -285,7 +294,7 @@ public void loadDartLibrary(String moduleName, int loadingUnitId) {
         soPath);
   }
 
-  public void uninstallFeature(String moduleName, int loadingUnitId) {
+  public void uninstallFeature(int loadingUnitId) {
     // TODO(garyq): support uninstalling.
   }
 
diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h
@@ -196,7 +196,7 @@ class PlatformViewAndroidJNI {
 
   virtual double GetDisplayRefreshRate() = 0;
 
-  virtual bool FlutterViewDownloadDynamicFeature(int loading_unit_id) = 0;
+  virtual bool InstallDeferredLibrary(int loading_unit_id) = 0;
 };
 
 }  // namespace flutter
diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc
@@ -343,8 +343,8 @@ PlatformViewAndroid::ComputePlatformResolvedLocales(
 }
 
 // |PlatformView|
-void PlatformViewAndroid::OnDartLoadLibrary(intptr_t loading_unit_id) {
-  if (jni_facade_->FlutterViewDownloadDynamicFeature(loading_unit_id)) {
+void PlatformViewAndroid::InstallDeferredLibrary(intptr_t loading_unit_id) {
+  if (jni_facade_->InstallDeferredLibrary(loading_unit_id)) {
     return;
   }
   return;  // TODO(garyq): Call CompleteDartLoadLibraryFailure()
diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h
@@ -146,7 +146,7 @@ class PlatformViewAndroid final : public PlatformView {
       const std::vector<std::string>& supported_locale_data) override;
 
   // |PlatformView|
-  void OnDartLoadLibrary(intptr_t loading_unit_id) override;
+  void InstallDeferredLibrary(intptr_t loading_unit_id) override;
 
   void InstallFirstFrameCallback();
 
diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc
@@ -102,7 +102,7 @@ static jmethodID g_detach_from_gl_context_method = nullptr;
 
 static jmethodID g_compute_platform_resolved_locale_method = nullptr;
 
-static jmethodID g_download_dynamic_feature_method = nullptr;
+static jmethodID g_install_dynamic_library_method = nullptr;
 
 // Called By Java
 static jmethodID g_on_display_platform_view_method = nullptr;
@@ -1044,10 +1044,10 @@ bool PlatformViewAndroid::Register(JNIEnv* env) {
     return false;
   }
 
-  g_download_dynamic_feature_method = env->GetMethodID(
-      g_flutter_jni_class->obj(), "downloadDynamicFeature", "(I)V");
+  g_install_dynamic_library_method = env->GetMethodID(
+      g_flutter_jni_class->obj(), "installDeferredLibrary", "(I)V");
 
-  if (g_download_dynamic_feature_method == nullptr) {
+  if (g_install_dynamic_library_method == nullptr) {
     FML_LOG(ERROR) << "Could not locate downloadDynamicFeature method";
     return false;
   }
@@ -1479,16 +1479,15 @@ double PlatformViewAndroidJNIImpl::GetDisplayRefreshRate() {
   return static_cast<double>(env->GetStaticFloatField(clazz, fid));
 }
 
-bool PlatformViewAndroidJNIImpl::FlutterViewDownloadDynamicFeature(
-    int loading_unit_id) {
+bool PlatformViewAndroidJNIImpl::InstallDeferredLibrary(int loading_unit_id) {
   JNIEnv* env = fml::jni::AttachCurrentThread();
 
   auto java_object = java_object_.get(env);
   if (java_object.is_null()) {
     return true;
   }
 
-  env->CallObjectMethod(java_object.obj(), g_download_dynamic_feature_method,
+  env->CallObjectMethod(java_object.obj(), g_install_dynamic_library_method,
                         loading_unit_id);
 
   FML_CHECK(CheckException(env));
diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h
@@ -80,7 +80,7 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI {
 
   double GetDisplayRefreshRate() override;
 
-  bool FlutterViewDownloadDynamicFeature(int loading_unit_id) override;
+  bool InstallDeferredLibrary(int loading_unit_id) override;
 
  private:
   // Reference to FlutterJNI object.

Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ PlatformView::ComputePlatformResolvedLocales(`
`152`	`152`	`return out;`
`153`	`153`	`}`
`154`	`154`
`155`		`-void PlatformView::OnDartLoadLibrary(intptr_t loading_unit_id) {}`
	`155`	`+void PlatformView::InstallDeferredLibrary(intptr_t loading_unit_id) {}`
`156`	`156`
`157`	`157`	`void PlatformView::CompleteDartLoadLibrary(`
`158`	`158`	`intptr_t loading_unit_id,`
Original file line number	Diff line number	Diff line change
`@@ -1200,13 +1200,13 @@ void Shell::UpdateAssetManager(std::shared_ptr<AssetManager> asset_manager) {`
`1200`	`1200`	`}`
`1201`	`1201`
`1202`	`1202`	`// \|Engine::Delegate\|`
`1203`		`-void Shell::OnDartLoadLibrary(intptr_t loading_unit_id) {`
	`1203`	`+void Shell::InstallDeferredLibrary(intptr_t loading_unit_id) {`
`1204`	`1204`	`OnPlatformViewDartLoadLibrary(loading_unit_id);`
`1205`	`1205`	`}`
`1206`	`1206`
`1207`	`1207`	`// \|PlatformView::Delegate\|`
`1208`	`1208`	`void Shell::OnPlatformViewDartLoadLibrary(intptr_t loading_unit_id) {`
`1209`		`- platform_view_->OnDartLoadLibrary(loading_unit_id);`
	`1209`	`+ platform_view_->InstallDeferredLibrary(loading_unit_id);`
`1210`	`1210`	`}`
`1211`	`1211`
`1212`	`1212`	`void Shell::ReportTimings() {`