[HCPP] Clean up overlay layer when last frame had overlay content and current doesn't (flutter#173881)

gmackall · Gray Mackall · web-flow · commit 26bb33b9572b · 2025-08-22T16:39:11.000Z
Fixes flutter#173635 My current understanding of the issue is that we are never pushing a new empty frame for this layer, so the texture gets stuck as shown in the bug. If we keep track of when we had content last frame, and don't in the current frame, we can push a single empty frame for that layer when needed. See example in flutter#173635 (comment) ## 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.  [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>
diff --git a/dev/integration_tests/android_engine_test/lib/hcpp/platform_view_overlay_layer_cleared.dart b/dev/integration_tests/android_engine_test/lib/hcpp/platform_view_overlay_layer_cleared.dart
@@ -0,0 +1,120 @@
+// 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());
+}
+
+class MyApp extends StatefulWidget {
+  const MyApp({super.key});
+
+  @override
+  State<MyApp> createState() => _MyAppState();
+}
+
+class _MyAppState extends State<MyApp> {
+  bool _showTexture = true;
+
+  void _toggleTexture() {
+    setState(() {
+      _showTexture = !_showTexture;
+    });
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    return MaterialApp(
+      home: Scaffold(
+        appBar: AppBar(title: const Text('HCPP Platform View Bug Demo')),
+        body: Column(
+          children: <Widget>[
+            Padding(
+              padding: const EdgeInsets.all(16.0),
+              child: ElevatedButton(
+                key: const ValueKey<String>('ToggleTexture'),
+                onPressed: _toggleTexture,
+                child: Text(_showTexture ? 'Hide Texture' : 'Show Texture'),
+              ),
+            ),
+            Expanded(
+              child: Center(
+                child: Stack(
+                  children: <Widget>[
+                    Center(
+                      child: SizedBox(
+                        width: 300,
+                        height: 300,
+                        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();
+                          },
+                        ),
+                      ),
+                    ),
+
+                    if (_showTexture)
+                      const Center(
+                        child: SizedBox(
+                          width: 275,
+                          height: 275,
+                          child: Opacity(
+                            opacity: 0.5,
+                            child: Texture(
+                              // Intentionally use an unknown texture ID: this
+                              // results a black rectangle which is good enough
+                              // for our purposes.
+                              textureId: 1,
+                            ),
+                          ),
+                        ),
+                      ),
+                  ],
+                ),
+              ),
+            ),
+          ],
+        ),
+      ),
+    );
+  }
+}
diff --git a/dev/integration_tests/android_engine_test/test_driver/hcpp/platform_view_overlay_layer_cleared_test.dart b/dev/integration_tests/android_engine_test/test_driver/hcpp/platform_view_overlay_layer_cleared_test.dart
@@ -0,0 +1,60 @@
+// 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_overlay_layer_cleared.dart
+///
+/// # Make your changes.
+///
+/// # Run the test against baseline.
+/// flutter drive lib/hcpp/platform_view_overlay_layer_cleared.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.texture.png'));
+    await flutterDriver.tap(find.byValueKey('ToggleTexture'));
+    await expectLater(nativeDriver.screenshot(), matchesGoldenFile('$goldenPrefix.no_texture.png'));
+  }, timeout: Timeout.none);
+}
diff --git a/engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_2.cc b/engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_2.cc
@@ -73,14 +73,11 @@ void AndroidExternalViewEmbedder2::SubmitFlutterView(
   if (!FrameHasPlatformLayers()) {
     frame->Submit();
     // If the previous frame had platform views, hide the overlay surface.
-    if (previous_frame_view_count_ > 0) {
-      jni_facade_->hideOverlaySurface2();
-    }
+    HideOverlayLayerIfNeeded();
     jni_facade_->applyTransaction();
     return;
   }
 
-  bool prev_frame_no_platform_views = previous_frame_view_count_ == 0;
   std::unordered_map<int64_t, DlRect> view_rects;
   for (auto platform_id : composition_order_) {
     view_rects[platform_id] = GetViewRect(platform_id, view_params_);
@@ -146,20 +143,27 @@ void AndroidExternalViewEmbedder2::SubmitFlutterView(
       overlay_canvas->RestoreToCount(restore_count);
     }
   }
+  bool overlay_layer_has_content_this_frame_;
   if (overlay_frame != nullptr) {
     overlay_frame->set_submit_info({.frame_boundary = false});
     overlay_frame->Submit();
+    overlay_layer_has_content_this_frame_ = true;
+  } else {
+    overlay_layer_has_content_this_frame_ = false;
   }
   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_), prev_frame_no_platform_views]() -> void {
+       slices = std::move(slices_),
+       overlay_layer_has_content_this_frame_]() -> void {
         jni_facade->swapTransaction();
 
-        if (prev_frame_no_platform_views) {
-          jni_facade_->showOverlaySurface2();
+        if (overlay_layer_has_content_this_frame_) {
+          ShowOverlayLayerIfNeeded();
+        } else {
+          HideOverlayLayerIfNeeded();
         }
 
         for (int64_t view_id : composition_order) {
@@ -197,8 +201,6 @@ DlCanvas* AndroidExternalViewEmbedder2::GetRootCanvas() {
 }
 
 void AndroidExternalViewEmbedder2::Reset() {
-  previous_frame_view_count_ = composition_order_.size();
-
   composition_order_.clear();
   slices_.clear();
 }
@@ -257,6 +259,21 @@ void AndroidExternalViewEmbedder2::DestroySurfaces() {
                                       latch.Signal();
                                     });
   latch.Wait();
+  overlay_layer_is_shown_ = false;
+}
+
+void AndroidExternalViewEmbedder2::ShowOverlayLayerIfNeeded() {
+  if (!overlay_layer_is_shown_) {
+    jni_facade_->showOverlaySurface2();
+    overlay_layer_is_shown_ = true;
+  }
+}
+
+void AndroidExternalViewEmbedder2::HideOverlayLayerIfNeeded() {
+  if (overlay_layer_is_shown_) {
+    jni_facade_->hideOverlaySurface2();
+    overlay_layer_is_shown_ = false;
+  }
 }
 
 }  // namespace flutter
diff --git a/engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_2.h b/engine/src/flutter/shell/platform/android/external_view_embedder/external_view_embedder_2.h
@@ -112,6 +112,9 @@ class AndroidExternalViewEmbedder2 final : public ExternalViewEmbedder {
   // The task runners.
   const TaskRunners task_runners_;
 
+  // If the overlay layer is currently shown.
+  bool overlay_layer_is_shown_ = false;
+
   // The size of the root canvas.
   DlISize frame_size_;
 
@@ -132,9 +135,6 @@ class AndroidExternalViewEmbedder2 final : public ExternalViewEmbedder {
   // mutation stack.
   std::unordered_map<int64_t, EmbeddedViewParams> view_params_;
 
-  // The number of platform views in the previous frame.
-  int64_t previous_frame_view_count_;
-
   // 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.
@@ -145,6 +145,13 @@ class AndroidExternalViewEmbedder2 final : public ExternalViewEmbedder {
 
   // Whether the layer tree in the current frame has platform layers.
   bool FrameHasPlatformLayers();
+
+  // Shows the overlay layer if it has content and the previous frame did not.
+  void ShowOverlayLayerIfNeeded();
+
+  // Hides the overlay layer if it does not have content and the previous
+  // frame did have content.
+  void HideOverlayLayerIfNeeded();
 };
 
 }  // namespace flutter
