[HCPP] Properly remove hcpp views that are no longer visible (#176742)

Fixes https://github.com/flutter/flutter/issues/175882.

This ended up being a very similar bug to
https://github.com/flutter/flutter/pull/173881, and in my understanding
comes from the same incorrect assumption. In old PV land these views
would stop displaying if we stopped sending updates to them. But in HCPP
the underlying implementation ends up using an Android SurfaceFlinger
which continues displaying what it was last told to. So again we have a
bug here were we aren't taking the new responsibility of clearing that
we need to. See
https://github.com/flutter/flutter/pull/173881#discussion_r2283287104

Other solutions would be to just always call hide on the set of (keys of
view_params which are not in composition order) on every frame (which
would avoid this mirroring of state), or to modify the jni here to pass
over one big bundle with all the information required to do all the
existing calls as well as determine which views need to be hidden. But
we are already doing this same mirroring of visibility state for the
overlay layer (see Hide/ShowOverlayLayerIfNeeded()), so I think it is
reasonable to take the same approach here.

This is sort of a follow up to
https://github.com/flutter/flutter/pull/162908 as well, this PR properly
removes the view when it is the last platform view, but does not fix the
case that this PR attempts to more generally fix where we go from 2->1
(or n->n-1 > 0) pvs.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Gray Mackall <mackall@google.com>

dev/integration_tests/android_engine_test/lib/hcpp/platform_view_clear_hidden.dart

@@ -0,0 +1,122 @@
+// Copyright 2014 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.
+
+import 'dart:convert';
+
+import 'package:android_driver_extensions/extension.dart';
+import 'package:flutter/foundation.dart';
+import 'package:flutter/gestures.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter/rendering.dart';
+import 'package:flutter/services.dart';
+import 'package:flutter_driver/driver_extension.dart';
+
+import '../src/allow_list_devices.dart';
+
+void main() {
+  ensureAndroidDevice();
+  enableFlutterDriverExtension(
+    handler: (String? command) async {
+      return json.encode(<String, Object?>{
+        'supported': await HybridAndroidViewController.checkIfSupported(),
+      });
+    },
+    commands: <CommandExtension>[nativeDriverCommands],
+  );
+
+  runApp(const MyApp());
+}
+
+/// A reusable widget that encapsulates the creation of the platform view.
+class PlatformViewWidget extends StatelessWidget {
+  const PlatformViewWidget({super.key});
+
+  @override
+  Widget build(BuildContext context) {
+    return Center(
+      child: SizedBox(
+        width: 200,
+        height: 200,
+        child: PlatformViewLink(
+          viewType: 'changing_color_button_platform_view',
+          surfaceFactory: (BuildContext context, PlatformViewController controller) {
+            return AndroidViewSurface(
+              controller: controller as AndroidViewController,
+              hitTestBehavior: PlatformViewHitTestBehavior.transparent,
+              gestureRecognizers: const <Factory<OneSequenceGestureRecognizer>>{},
+            );
+          },
+          onCreatePlatformView: (PlatformViewCreationParams params) {
+            return PlatformViewsService.initHybridAndroidView(
+                id: params.id,
+                viewType: 'changing_color_button_platform_view',
+                layoutDirection: TextDirection.ltr,
+                creationParamsCodec: const StandardMessageCodec(),
+              )
+              ..addOnPlatformViewCreatedListener(params.onPlatformViewCreated)
+              ..create();
+          },
+        ),
+      ),
+    );
+  }
+}
+
+class MyApp extends StatefulWidget {
+  const MyApp({super.key});
+
+  @override
+  State<MyApp> createState() => _MyAppState();
+}
+
+class _MyAppState extends State<MyApp> {
+  bool _showRightView = true;
+
+  void _toggleRightView() {
+    setState(() {
+      _showRightView = !_showRightView;
+    });
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    return MaterialApp(
+      home: Scaffold(
+        appBar: AppBar(title: const Text('Platform View Toggle Demo')),
+        body: Column(
+          children: <Widget>[
+            Padding(
+              padding: const EdgeInsets.all(16.0),
+              child: ElevatedButton(
+                key: const ValueKey<String>('ToggleRightView'),
+                onPressed: _toggleRightView,
+                child: Text(_showRightView ? 'Hide Right View' : 'Show Right View'),
+              ),
+            ),
+            Expanded(
+              child: Row(
+                children: <Widget>[
+                  // The left platform view is always visible.
+                  const Expanded(child: PlatformViewWidget()),
+
+                  // The right platform view's visibility is toggled,
+                  // but it always occupies space in the layout.
+                  Expanded(
+                    child: Visibility(
+                      visible: _showRightView,
+                      maintainState: true,
+                      maintainSize: true,
+                      maintainAnimation: true,
+                      child: const PlatformViewWidget(),
+                    ),
+                  ),
+                ],
+              ),
+            ),
+          ],
+        ),
+      ),
+    );
+  }
+}

dev/integration_tests/android_engine_test/test_driver/hcpp/platform_view_clear_hidden_test.dart

@@ -0,0 +1,63 @@
+// Copyright 2014 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.
+
+import 'dart:convert';
+
+import 'package:android_driver_extensions/native_driver.dart';
+import 'package:android_driver_extensions/skia_gold.dart';
+import 'package:flutter_driver/flutter_driver.dart';
+import 'package:test/test.dart';
+
+import '../_luci_skia_gold_prelude.dart';
+
+/// For local debugging, a (local) golden-file is required as a baseline:
+///
+/// ```sh
+/// # Checkout HEAD, i.e. *before* changes you want to test.
+/// UPDATE_GOLDENS=1 flutter drive lib/hcpp/platform_view_clear_hidden.dart
+///
+/// # Make your changes.
+///
+/// # Run the test against baseline.
+/// flutter drive lib/hcpp/platform_view_clear_hidden.dart
+/// ```
+///
+/// For a convenient way to deflake a test, see `tool/deflake.dart`.
+void main() async {
+  const String goldenPrefix = 'hybrid_composition_pp_platform_view';
+
+  late final FlutterDriver flutterDriver;
+  late final NativeDriver nativeDriver;
+
+  setUpAll(() async {
+    if (isLuci) {
+      await enableSkiaGoldComparator(namePrefix: 'android_engine_test$goldenVariant');
+    }
+    flutterDriver = await FlutterDriver.connect();
+    nativeDriver = await AndroidNativeDriver.connect(flutterDriver);
+    await nativeDriver.configureForScreenshotTesting();
+    await flutterDriver.waitUntilFirstFrameRasterized();
+  });
+
+  tearDownAll(() async {
+    await nativeDriver.close();
+    await flutterDriver.close();
+  });
+
+  test('verify that HCPP is supported and enabled', () async {
+    final Map<String, Object?> response =
+        json.decode(await flutterDriver.requestData('')) as Map<String, Object?>;
+
+    expect(response['supported'], true);
+  }, timeout: Timeout.none);
+
+  test('should start with texture, and toggle to no texture', () async {
+    await expectLater(nativeDriver.screenshot(), matchesGoldenFile('$goldenPrefix.two_boxes.png'));
+    await flutterDriver.tap(find.byValueKey('ToggleRightView'));
+    await expectLater(
+      nativeDriver.screenshot(),
+      matchesGoldenFile('$goldenPrefix.only_one_box.png'),
+    );
+  }, timeout: Timeout.none);
+}

engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc

@@ -114,6 +114,7 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI {
                int32_t viewHeight,
                MutatorsStack mutators_stack),
               (override));
+  MOCK_METHOD(void, hidePlatformView2, (int32_t view_id), (override));
   MOCK_METHOD(void, onEndFrame2, (), (override));
   MOCK_METHOD(void, showOverlaySurface2, (), (override));
   MOCK_METHOD(void, hideOverlaySurface2, (), (override));

engine/src/flutter/shell/platform/android/external_view_embedder/BUILD.gn

@@ -29,6 +29,9 @@ source_set("external_view_embedder") {
     "//flutter/shell/platform/android/surface",
     "//flutter/skia",
   ]
+
+  public_deps =
+      [ "//flutter/third_party/abseil-cpp/absl/container:flat_hash_set" ]
 }
 
 test_fixtures("external_view_embedder_fixtures") {

engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_2.cc

@@ -72,9 +72,20 @@ void AndroidExternalViewEmbedder2::SubmitFlutterView(
 
   if (!FrameHasPlatformLayers()) {
     frame->Submit();
-    // If the previous frame had platform views, hide the overlay surface.
-    HideOverlayLayerIfNeeded();
-    jni_facade_->applyTransaction();
+    task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable(
+        [this, jni_facade = jni_facade_,
+         views_visible_last_frame = views_visible_last_frame_]() {
+          // This pointer is guaranteed to not be dangling as long as
+          // DestroySurfaces is called before the embedder is deleted. See
+          // https://github.com/flutter/flutter/pull/176742#discussion_r2415229396.
+          this->HideOverlayLayerIfNeeded();
+          for (int64_t view_id : views_visible_last_frame) {
+            jni_facade->hidePlatformView2(view_id);
+          }
+
+          jni_facade->applyTransaction();
+        }));
+    views_visible_last_frame_.clear();
     return;
   }
 
@@ -151,13 +162,14 @@ void AndroidExternalViewEmbedder2::SubmitFlutterView(
   } else {
     overlay_layer_has_content_this_frame_ = false;
   }
-  frame->Submit();
 
+  frame->Submit();
   task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable(
       [&, composition_order = composition_order_, view_params = view_params_,
        jni_facade = jni_facade_, device_pixel_ratio = device_pixel_ratio_,
        slices = std::move(slices_),
-       overlay_layer_has_content_this_frame_]() -> void {
+       views_visible_last_frame = views_visible_last_frame_,
+       overlay_layer_has_content_this_frame_]() mutable -> void {
         jni_facade->swapTransaction();
 
         if (overlay_layer_has_content_this_frame_) {
@@ -179,9 +191,20 @@ void AndroidExternalViewEmbedder2::SubmitFlutterView(
               params.sizePoints().height * device_pixel_ratio,
               params.mutatorsStack()  //
           );
+          // Remove from views visible last frame, so we can hide the rest.
+          views_visible_last_frame.erase(view_id);
         }
+        // Hide views that were visible last frame, but not in this frame.
+        for (int64_t view_id : views_visible_last_frame) {
+          jni_facade->hidePlatformView2(view_id);
+        }
+
         jni_facade_->onEndFrame2();
       }));
+
+  views_visible_last_frame_.clear();
+  views_visible_last_frame_.insert(composition_order_.begin(),
+                                   composition_order_.end());
 }
 
 // |ExternalViewEmbedder|
@@ -259,20 +282,20 @@ void AndroidExternalViewEmbedder2::DestroySurfaces() {
                                       latch.Signal();
                                     });
   latch.Wait();
-  overlay_layer_is_shown_ = false;
+  overlay_layer_is_shown_.store(false);
 }
 
 void AndroidExternalViewEmbedder2::ShowOverlayLayerIfNeeded() {
-  if (!overlay_layer_is_shown_) {
+  if (!overlay_layer_is_shown_.load()) {
     jni_facade_->showOverlaySurface2();
-    overlay_layer_is_shown_ = true;
+    overlay_layer_is_shown_.store(true);
   }
 }
 
 void AndroidExternalViewEmbedder2::HideOverlayLayerIfNeeded() {
-  if (overlay_layer_is_shown_) {
+  if (overlay_layer_is_shown_.load()) {
     jni_facade_->hideOverlaySurface2();
-    overlay_layer_is_shown_ = false;
+    overlay_layer_is_shown_.store(false);
   }
 }
 

engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_2.h

@@ -5,6 +5,7 @@
 #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_EXTERNAL_VIEW_EMBEDDER_2_H_
 #define FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_EXTERNAL_VIEW_EMBEDDER_2_H_
 
+#include <atomic>
 #include <unordered_map>
 
 #include "flutter/common/task_runners.h"
@@ -13,6 +14,7 @@
 #include "flutter/shell/platform/android/external_view_embedder/surface_pool.h"
 #include "flutter/shell/platform/android/jni/platform_view_android_jni.h"
 #include "flutter/shell/platform/android/surface/android_surface.h"
+#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_set.h"
 
 namespace flutter {
 
@@ -113,7 +115,7 @@ class AndroidExternalViewEmbedder2 final : public ExternalViewEmbedder {
   const TaskRunners task_runners_;
 
   // If the overlay layer is currently shown.
-  bool overlay_layer_is_shown_ = false;
+  std::atomic_bool overlay_layer_is_shown_{false};
 
   // The size of the root canvas.
   DlISize frame_size_;
@@ -135,6 +137,9 @@ class AndroidExternalViewEmbedder2 final : public ExternalViewEmbedder {
   // mutation stack.
   std::unordered_map<int64_t, EmbeddedViewParams> view_params_;
 
+  // The set of platform views that were visible in the last frame.
+  absl::flat_hash_set<int64_t> views_visible_last_frame_;
+
   // Destroys the surfaces created from the surface factory.
   // This method schedules a task on the platform thread, and waits for
   // the task until it completes.

engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

@@ -1387,6 +1387,18 @@ public class FlutterJNI {
         viewId, x, y, width, height, viewWidth, viewHeight, mutatorsStack);
   }
 
+  @UiThread
+  @SuppressWarnings("unused")
+  @SuppressLint("NewApi")
+  public void hidePlatformView2(int viewId) {
+    ensureRunningOnMainThread();
+    if (platformViewsController2 == null) {
+      throw new RuntimeException(
+          "platformViewsController must be set before attempting to hide a platform view");
+    }
+    platformViewsController2.hidePlatformView(viewId);
+  }
+
   // ----- Start Localization Support ----
 
   /** Sets the localization plugin that is used in various localization methods. */

engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController2.java

@@ -526,6 +526,15 @@ public class PlatformViewsController2 implements PlatformViewsAccessibilityDeleg
     }
   }
 
+  public void hidePlatformView(int viewId) {
+    if (!initializePlatformViewIfNeeded(viewId)) {
+      return;
+    }
+
+    final FlutterMutatorView parentView = platformViewParent.get(viewId);
+    parentView.setVisibility(View.GONE);
+  }
+
   @RequiresApi(API_LEVELS.API_34)
   public void onEndFrame() {
     SurfaceControl.Transaction tx = new SurfaceControl.Transaction();

engine/src/flutter/shell/platform/android/jni/jni_mock.h

@@ -143,6 +143,7 @@ class JNIMock final : public PlatformViewAndroidJNI {
                MutatorsStack mutators_stack),
               (override));
 
+  MOCK_METHOD(void, hidePlatformView2, (int32_t view_id), (override));
   MOCK_METHOD(void, onEndFrame2, (), (override));
   MOCK_METHOD(void, hideOverlaySurface2, (), (override));
   MOCK_METHOD(void, showOverlaySurface2, (), (override));

engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h

@@ -244,6 +244,8 @@ class PlatformViewAndroidJNI {
                                       int32_t viewHeight,
                                       MutatorsStack mutators_stack) = 0;
 
+  virtual void hidePlatformView2(int32_t view_id) = 0;
+
   virtual void showOverlaySurface2() = 0;
 
   virtual void hideOverlaySurface2() = 0;

engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc

@@ -87,6 +87,7 @@ static jfieldID g_jni_shell_holder_field = nullptr;
   V(g_on_display_platform_view2_method, onDisplayPlatformView2,               \
     "(IIIIIIILio/flutter/embedding/engine/mutatorsstack/"                     \
     "FlutterMutatorsStack;)V")                                                \
+  V(g_hide_platform_view2_method, hidePlatformView2, "(I)V")                  \
   V(g_on_end_frame2_method, endFrame2, "()V")                                 \
   V(g_show_overlay_surface2_method, showOverlaySurface2, "()V")               \
   V(g_hide_overlay_surface2_method, hideOverlaySurface2, "()V")               \
@@ -2305,6 +2306,16 @@ void PlatformViewAndroidJNIImpl::onDisplayPlatformView2(
   FML_CHECK(fml::jni::CheckException(env));
 }
 
+void PlatformViewAndroidJNIImpl::hidePlatformView2(int32_t view_id) {
+  JNIEnv* env = fml::jni::AttachCurrentThread();
+  auto java_object = java_object_.get(env);
+  if (java_object.is_null()) {
+    return;
+  }
+
+  env->CallVoidMethod(java_object.obj(), g_hide_platform_view2_method, view_id);
+}
+
 void PlatformViewAndroidJNIImpl::onEndFrame2() {
   JNIEnv* env = fml::jni::AttachCurrentThread();
 

engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h

@@ -126,6 +126,8 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI {
                               int32_t viewHeight,
                               MutatorsStack mutators_stack) override;
 
+  void hidePlatformView2(int32_t view_id) override;
+
   void showOverlaySurface2() override;
 
   void hideOverlaySurface2() override;