Revert "Enable hybrid composition by default on Android (flutter#20722)" (flutter#20745)

Emmanuel Garcia · web-flow · commit 4de62c7c2659 · 2020-08-25T09:54:13.000-07:00
This reverts commit d16ba48.
diff --git a/common/settings.h b/common/settings.h
@@ -223,6 +223,15 @@ struct Settings {
   /// to log a timeline event that tracks the latency of engine startup.
   std::chrono::microseconds engine_start_timestamp = {};
 
+  /// Whether the application claims that it uses the android embedded view for
+  /// platform views.
+  ///
+  /// A `true` value will result the raster task runner always run on the
+  /// platform thread.
+  // TODO(cyanlaz): Remove this when dynamic thread merging is done.
+  // https://github.com/flutter/flutter/issues/59930
+  bool use_embedded_view = false;
+
   std::string ToString() const;
 };
 
diff --git a/flow/surface.cc b/flow/surface.cc
@@ -18,8 +18,4 @@ std::unique_ptr<GLContextResult> Surface::MakeRenderContextCurrent() {
   return std::make_unique<GLContextDefaultResult>(true);
 }
 
-bool Surface::ClearRenderContext() {
-  return false;
-}
-
 }  // namespace flutter
diff --git a/flow/surface.h b/flow/surface.h
@@ -33,8 +33,6 @@ class Surface {
 
   virtual std::unique_ptr<GLContextResult> MakeRenderContextCurrent();
 
-  virtual bool ClearRenderContext();
-
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(Surface);
 };
diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc
@@ -143,10 +143,6 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
     consume_result = PipelineConsumeResult::MoreAvailable;
   }
 
-  if (surface_ != nullptr) {
-    surface_->ClearRenderContext();
-  }
-
   // Merging the thread as we know the next `Draw` should be run on the platform
   // thread.
   if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) {
diff --git a/shell/common/switches.cc b/shell/common/switches.cc
@@ -229,6 +229,9 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {
                                                             : "127.0.0.1";
   }
 
+  settings.use_embedded_view =
+      command_line.HasOption(FlagForSwitch(Switch::UseEmbeddedView));
+
   // Set Observatory Port
   if (command_line.HasOption(FlagForSwitch(Switch::DeviceObservatoryPort))) {
     if (!GetSwitchValue(command_line, Switch::DeviceObservatoryPort,
diff --git a/shell/common/switches.h b/shell/common/switches.h
@@ -196,7 +196,15 @@ DEF_SWITCH(
     "Uses separate threads for the platform, UI, GPU and IO task runners. "
     "By default, a single thread is used for all task runners. Only available "
     "in the flutter_tester.")
-
+// TODO(cyanlaz): Remove this when dynamic thread merging is done.
+// https://github.com/flutter/flutter/issues/59930
+DEF_SWITCH(UseEmbeddedView,
+           "use-embedded-view",
+           "Whether an android application uses embedded views."
+           "This is a temporary flag to make the raster task runner runs on "
+           "the platform thread."
+           "This flag should be removed once the dynamic thread merging is "
+           "enabled on android.")
 DEF_SWITCHES_END
 
 void PrintUsage(const std::string& executable_name);
diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc
@@ -341,9 +341,4 @@ std::unique_ptr<GLContextResult> GPUSurfaceGL::MakeRenderContextCurrent() {
   return delegate_->GLContextMakeCurrent();
 }
 
-// |Surface|
-bool GPUSurfaceGL::ClearRenderContext() {
-  return delegate_->GLContextClearCurrent();
-}
-
 }  // namespace flutter
diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h
@@ -48,9 +48,6 @@ class GPUSurfaceGL : public Surface {
   // |Surface|
   std::unique_ptr<GLContextResult> MakeRenderContextCurrent() override;
 
-  // |Surface|
-  bool ClearRenderContext() override;
-
  private:
   GPUSurfaceGLDelegate* delegate_;
   sk_sp<GrDirectContext> context_;
diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc
@@ -28,6 +28,8 @@ static PlatformData GetDefaultPlatformData() {
   return platform_data;
 }
 
+bool AndroidShellHolder::use_embedded_view;
+
 AndroidShellHolder::AndroidShellHolder(
     flutter::Settings settings,
     std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
@@ -100,21 +102,47 @@ AndroidShellHolder::AndroidShellHolder(
     ui_runner = thread_host_.ui_thread->GetTaskRunner();
     io_runner = thread_host_.io_thread->GetTaskRunner();
   }
-
-  flutter::TaskRunners task_runners(thread_label,     // label
-                                    platform_runner,  // platform
-                                    gpu_runner,       // raster
-                                    ui_runner,        // ui
-                                    io_runner         // io
-  );
-
-  shell_ =
-      Shell::Create(task_runners,              // task runners
-                    GetDefaultPlatformData(),  // window data
-                    settings_,                 // settings
-                    on_create_platform_view,   // platform view create callback
-                    on_create_rasterizer       // rasterizer create callback
-      );
+  if (settings.use_embedded_view) {
+    use_embedded_view = true;
+    // Embedded views requires the gpu and the platform views to be the same.
+    // The plan is to eventually dynamically merge the threads when there's a
+    // platform view in the layer tree.
+    // For now we use a fixed thread configuration with the same thread used as
+    // the gpu and platform task runner.
+    // TODO(amirh/chinmaygarde): remove this, and dynamically change the thread
+    // configuration. https://github.com/flutter/flutter/issues/23975
+    // https://github.com/flutter/flutter/issues/59930
+    flutter::TaskRunners task_runners(thread_label,     // label
+                                      platform_runner,  // platform
+                                      platform_runner,  // raster
+                                      ui_runner,        // ui
+                                      io_runner         // io
+    );
+
+    shell_ =
+        Shell::Create(task_runners,              // task runners
+                      GetDefaultPlatformData(),  // window data
+                      settings_,                 // settings
+                      on_create_platform_view,  // platform view create callback
+                      on_create_rasterizer      // rasterizer create callback
+        );
+  } else {
+    use_embedded_view = false;
+    flutter::TaskRunners task_runners(thread_label,     // label
+                                      platform_runner,  // platform
+                                      gpu_runner,       // raster
+                                      ui_runner,        // ui
+                                      io_runner         // io
+    );
+
+    shell_ =
+        Shell::Create(task_runners,              // task runners
+                      GetDefaultPlatformData(),  // window data
+                      settings_,                 // settings
+                      on_create_platform_view,  // platform view create callback
+                      on_create_rasterizer      // rasterizer create callback
+        );
+  }
 
   platform_view_ = weak_platform_view;
   FML_DCHECK(platform_view_);
diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h
@@ -21,6 +21,14 @@ namespace flutter {
 
 class AndroidShellHolder {
  public:
+  // Whether the application sets to use embedded_view view
+  // `io.flutter.embedded_views_preview` flag. This can be static because it is
+  // determined by the application and it is safe when there are multiple
+  // `AndroidSurface`s.
+  // TODO(cyanglaz): remove this when dynamic thread merging is enabled on
+  // android. https://github.com/flutter/flutter/issues/59930
+  static bool use_embedded_view;
+
   AndroidShellHolder(flutter::Settings settings,
                      std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
                      bool is_background_view);
diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc
@@ -123,6 +123,9 @@ intptr_t AndroidSurfaceGL::GLContextFBO(GLFrameInfo frame_info) const {
 
 // |GPUSurfaceGLDelegate|
 ExternalViewEmbedder* AndroidSurfaceGL::GetExternalViewEmbedder() {
+  if (!AndroidShellHolder::use_embedded_view) {
+    return nullptr;
+  }
   return external_view_embedder_.get();
 }
 
diff --git a/shell/platform/android/android_surface_software.cc b/shell/platform/android/android_surface_software.cc
@@ -146,6 +146,9 @@ bool AndroidSurfaceSoftware::PresentBackingStore(
 
 // |GPUSurfaceSoftwareDelegate|
 ExternalViewEmbedder* AndroidSurfaceSoftware::GetExternalViewEmbedder() {
+  if (!AndroidShellHolder::use_embedded_view) {
+    return nullptr;
+  }
   return external_view_embedder_.get();
 }
 
diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/ApplicationInfoLoader.java b/shell/platform/android/io/flutter/embedding/engine/loader/ApplicationInfoLoader.java
@@ -79,6 +79,11 @@ private static String getNetworkPolicy(ApplicationInfo appInfo, Context context)
     return output.toString();
   }
 
+  private static boolean getUseEmbeddedView(ApplicationInfo appInfo) {
+    Bundle bundle = appInfo.metaData;
+    return bundle != null && bundle.getBoolean("io.flutter.embedded_views_preview");
+  }
+
   private static void parseDomainConfig(
       XmlResourceParser xrp, JSONArray output, boolean inheritedCleartextPermitted)
       throws IOException, XmlPullParserException {
@@ -150,6 +155,7 @@ public static FlutterApplicationInfo load(@NonNull Context applicationContext) {
         getString(appInfo.metaData, PUBLIC_FLUTTER_ASSETS_DIR_KEY),
         getNetworkPolicy(appInfo, applicationContext),
         appInfo.nativeLibraryDir,
-        clearTextPermitted);
+        clearTextPermitted,
+        getUseEmbeddedView(appInfo));
   }
 }
diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterApplicationInfo.java b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterApplicationInfo.java
@@ -18,6 +18,9 @@ public final class FlutterApplicationInfo {
   final String domainNetworkPolicy;
   final String nativeLibraryDir;
   final boolean clearTextPermitted;
+  // TODO(cyanlaz): Remove this when dynamic thread merging is done.
+  // https://github.com/flutter/flutter/issues/59930
+  final boolean useEmbeddedView;
 
   public FlutterApplicationInfo(
       String aotSharedLibraryName,
@@ -26,7 +29,8 @@ public FlutterApplicationInfo(
       String flutterAssetsDir,
       String domainNetworkPolicy,
       String nativeLibraryDir,
-      boolean clearTextPermitted) {
+      boolean clearTextPermitted,
+      boolean useEmbeddedView) {
     this.aotSharedLibraryName =
         aotSharedLibraryName == null ? DEFAULT_AOT_SHARED_LIBRARY_NAME : aotSharedLibraryName;
     this.vmSnapshotData = vmSnapshotData == null ? DEFAULT_VM_SNAPSHOT_DATA : vmSnapshotData;
@@ -37,5 +41,6 @@ public FlutterApplicationInfo(
     this.nativeLibraryDir = nativeLibraryDir;
     this.domainNetworkPolicy = domainNetworkPolicy == null ? "" : domainNetworkPolicy;
     this.clearTextPermitted = clearTextPermitted;
+    this.useEmbeddedView = useEmbeddedView;
   }
 }
diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java
@@ -223,6 +223,9 @@ public void ensureInitializationComplete(
       if (flutterApplicationInfo.domainNetworkPolicy != null) {
         shellArgs.add("--domain-network-policy=" + flutterApplicationInfo.domainNetworkPolicy);
       }
+      if (flutterApplicationInfo.useEmbeddedView) {
+        shellArgs.add("--use-embedded-view");
+      }
       if (settings.getLogTag() != null) {
         shellArgs.add("--log-tag=" + settings.getLogTag());
       }
diff --git a/shell/platform/android/test/io/flutter/embedding/engine/PluginComponentTest.java b/shell/platform/android/test/io/flutter/embedding/engine/PluginComponentTest.java
@@ -28,7 +28,7 @@ public void pluginsCanAccessFlutterAssetPaths() {
     FlutterJNI flutterJNI = mock(FlutterJNI.class);
     when(flutterJNI.isAttached()).thenReturn(true);
     FlutterApplicationInfo emptyInfo =
-        new FlutterApplicationInfo(null, null, null, null, null, null, false);
+        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
diff --git a/shell/platform/android/test/io/flutter/embedding/engine/loader/ApplicationInfoLoaderTest.java b/shell/platform/android/test/io/flutter/embedding/engine/loader/ApplicationInfoLoaderTest.java
@@ -43,6 +43,7 @@ public void itGeneratesCorrectApplicationInfoWithDefaultManifest() {
     assertEquals("", info.domainNetworkPolicy);
     assertNull(info.nativeLibraryDir);
     assertEquals(true, info.clearTextPermitted);
+    assertEquals(false, info.useEmbeddedView);
   }
 
   @Config(shadows = {ApplicationInfoLoaderTest.ShadowNetworkSecurityPolicy.class})
@@ -87,6 +88,7 @@ public void itGeneratesCorrectApplicationInfoWithCustomValues() throws Exception
     bundle.putString(ApplicationInfoLoader.PUBLIC_VM_SNAPSHOT_DATA_KEY, "testvmsnapshot");
     bundle.putString(ApplicationInfoLoader.PUBLIC_ISOLATE_SNAPSHOT_DATA_KEY, "testisolatesnapshot");
     bundle.putString(ApplicationInfoLoader.PUBLIC_FLUTTER_ASSETS_DIR_KEY, "testassets");
+    bundle.putBoolean("io.flutter.embedded_views_preview", true);
     Context context = generateMockContext(bundle, null);
     FlutterApplicationInfo info = ApplicationInfoLoader.load(context);
     assertNotNull(info);
@@ -96,6 +98,7 @@ public void itGeneratesCorrectApplicationInfoWithCustomValues() throws Exception
     assertEquals("testassets", info.flutterAssetsDir);
     assertNull(info.nativeLibraryDir);
     assertEquals("", info.domainNetworkPolicy);
+    assertEquals(true, info.useEmbeddedView);
   }
 
   @Test
diff --git a/testing/scenario_app/android/app/src/main/AndroidManifest.xml b/testing/scenario_app/android/app/src/main/AndroidManifest.xml
@@ -35,6 +35,9 @@
                 <category android:name="android.intent.category.LAUNCHER" />
             </intent-filter>
         </activity>
+        <meta-data
+            android:name="io.flutter.embedded_views_preview"
+            android:value="true" />
     </application>
     <uses-permission android:name="android.permission.INTERNET" />
 </manifest>

Original file line number	Diff line number	Diff line change
`@@ -18,8 +18,4 @@ std::unique_ptr<GLContextResult> Surface::MakeRenderContextCurrent() {`
`18`	`18`	`return std::make_unique<GLContextDefaultResult>(true);`
`19`	`19`	`}`
`20`	`20`
`21`		`-bool Surface::ClearRenderContext() {`
`22`		`- return false;`
`23`		`-}`
`24`		`-`
`25`	`21`	`} // namespace flutter`
Original file line number	Diff line number	Diff line change
`@@ -229,6 +229,9 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {`
`229`	`229`	`: "127.0.0.1";`
`230`	`230`	`}`
`231`	`231`
	`232`	`+ settings.use_embedded_view =`
	`233`	`+ command_line.HasOption(FlagForSwitch(Switch::UseEmbeddedView));`
	`234`	`+`
`232`	`235`	`// Set Observatory Port`
`233`	`236`	`if (command_line.HasOption(FlagForSwitch(Switch::DeviceObservatoryPort))) {`
`234`	`237`	`if (!GetSwitchValue(command_line, Switch::DeviceObservatoryPort,`
Original file line number	Diff line number	Diff line change
`@@ -341,9 +341,4 @@ std::unique_ptr<GLContextResult> GPUSurfaceGL::MakeRenderContextCurrent() {`
`341`	`341`	`return delegate_->GLContextMakeCurrent();`
`342`	`342`	`}`
`343`	`343`
`344`		`-// \|Surface\|`
`345`		`-bool GPUSurfaceGL::ClearRenderContext() {`
`346`		`- return delegate_->GLContextClearCurrent();`
`347`		`-}`
`348`		`-`
`349`	`344`	`} // namespace flutter`