Refactor Android platform view code in advance of enabling HCPP on existing PV widgets (behind a flag) #170553

gmackall · 2025-06-12T22:28:35Z

Should introduce no behavior changes. Refactors the android platform view code so that

The delegator is ready to delegate between PlatformViewsController (PVC) 1 and 2, and implements a PlatformViewsChannel.PlatformViewsHandler.
The PlatformViewCreationRequest and PlatformViewTouch are fully shared between the two platform view modes
Introduces new factory constructors for the different types of PlatformViewCreationRequests, corresponding to the modes.

Making the flag work on existing widgets will be done in a follow up PR here

flutter/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewsChannel.java

Line 95 in 67262d9

// TODO(gmackall): Enable hcpp path in a follow up PR to

.

Pre-launch Checklist

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

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

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

gmackall · 2025-06-13T18:27:26Z

engine/src/flutter/shell/platform/android/build.gradle

    }
    dependencies {
-        classpath "com.android.tools.build:gradle:8.9.1"
+        classpath "com.android.tools.build:gradle:8.7.1"


Unrelated, but my AS doesn't support AGP 8.9.1 so I'd prefer to lower this (given that it is only used for our IDE support for our convenience while editing the engine). I currently modify it on every local branch and just don't commit, but that's sort of annoying.

I am ok with you modifying this but please add a comment above for how our team should decide what version of AGP should go here and when to update it. Without documentation anyone is likely to bump it.

Added a comment

Comment is fine, that said my intent was for the comment to tell the flutter-android team when to update.

Yeah, I think at the end of the day the answer is just going to be "whenever we as a group want to". I think it's not urgent to update this really, we should just avoid it being super old

reidbaker · 2025-07-25T16:49:48Z

dev/integration_tests/android_engine_test/lib/hcpp/README.md

Thank you for adding this readme

reidbaker · 2025-07-25T16:50:41Z

engine/src/flutter/shell/platform/android/build.gradle

    }
    dependencies {
-        classpath "com.android.tools.build:gradle:8.9.1"
+        classpath "com.android.tools.build:gradle:8.7.1"


Comment is fine, that said my intent was for the comment to tell the flutter-android team when to update.

...platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewCreationRequest.java

gmackall · 2025-07-29T17:36:33Z

/gemini review

gemini-code-assist

Code Review

This pull request is a great step forward in refactoring the Android platform view code. The introduction of the PlatformViewsControllerDelegator and the extraction of shared data classes like PlatformViewCreationRequest and PlatformViewTouch significantly improve the code's structure and maintainability. The changes are logical and well-implemented.

My feedback focuses on improving documentation and API clarity to ensure the new components are easy to understand and use correctly. I've pointed out a few places where Javadoc is missing for new public methods and suggested a clarification for a potentially nullable parameter. I also found a broken link in a new README file.

Overall, this is a solid refactoring. Addressing these minor points will make it even better.

gemini-code-assist · 2025-07-29T17:38:37Z

dev/integration_tests/android_engine_test/lib/hcpp/README.md

@@ -0,0 +1,2 @@
+Everything under this file path will have hcpp enabled, see the configuration here:
+https://github.com/flutter/flutter/blob/337a007f5cfb66d0ecc84b1545539231a0be4d61/dev/bots/suite_runners/run_android_engine_tests.dart#L34


The link to the configuration file uses a specific commit hash and an incorrect line number. This is fragile and will become outdated. To improve maintainability, please:

Use the main branch in the URL instead of a commit hash.

Update the line number to point to the correct code block that enables HCPP tests (around line 117).

Suggested change

https://github.com/flutter/flutter/blob/337a007f5cfb66d0ecc84b1545539231a0be4d61/dev/bots/suite_runners/run_android_engine_tests.dart#L34

https://github.com/flutter/flutter/blob/main/dev/bots/suite_runners/run_android_engine_tests.dart#L117-L125

NO, Commit hash's are required for stable links to line numbers and the relevant link is the method not the part of the method associated with hcpp.

yeah I did not adopt this one for that reason haha

gemini-code-assist · 2025-07-29T17:38:37Z

...platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewCreationRequest.java

+      double logicalWidth,
+      double logicalHeight,
+      int direction,
+      RequestedDisplayMode displayMode,


The displayMode parameter in this constructor is not annotated with @Nullable, but it is being passed null from the createHCPPRequest factory method. This could be misleading and potentially lead to an NPE if not handled carefully.

Please add the @Nullable annotation to the displayMode parameter for clarity and to align with its usage.

Suggested change

RequestedDisplayMode displayMode,

@Nullable RequestedDisplayMode displayMode,

gemini-code-assist · 2025-07-29T17:38:37Z

.../shell/platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewsChannel.java

+    public boolean isHcppEnabled();
+
+    void createPlatformViewHcpp(@NonNull PlatformViewCreationRequest request);


The new public methods isHcppEnabled and createPlatformViewHcpp in the PlatformViewsHandler interface are missing Javadoc comments. According to the style guide, all public members should be documented.¹

Please add Javadoc to explain what these methods do.

/** Returns true if Hybrid Composition Platform Views++ is enabled. */ public boolean isHcppEnabled(); /** * The Flutter application would like to display a new Android {@code View} using Hybrid * Composition++. * * @param request The creation request with the parameters for the platform view. */ void createPlatformViewHcpp(@NonNull PlatformViewCreationRequest request);

Style Guide References

Footnotes

All public members should have documentation. (link) ↩

gemini-code-assist · 2025-07-29T17:38:37Z

...e/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java

+  public PlatformViewsChannel getPlatformViewsChannel() {
+    return platformViewsChannel;
+  }


The new public method getPlatformViewsChannel is missing a Javadoc comment. According to the style guide, all public members should be documented.¹

/** * Returns the platform views channel. * * <p>This is intended for the {@code PlatformViewsControllerDelegator} to use. */ public PlatformViewsChannel getPlatformViewsChannel() { return platformViewsChannel; }

Style Guide References

Footnotes

All public members should have documentation. (link) ↩

gemini-code-assist · 2025-07-29T17:38:37Z

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

+  public boolean isHcppEnabled() {
+    if (flutterJNI == null) {
+      return false;
+    }
+    return flutterJNI.IsSurfaceControlEnabled();
+  }


The new public method isHcppEnabled is missing a Javadoc comment. According to the style guide, all public members should be documented.¹

Style Guide References

Suggested change

public boolean isHcppEnabled() {

if (flutterJNI == null) {

return false;

}

return flutterJNI.IsSurfaceControlEnabled();

}

/** Returns true if Hybrid Composition Platform Views++ is enabled. */

public boolean isHcppEnabled() {

if (flutterJNI == null) {

return false;

}

return flutterJNI.IsSurfaceControlEnabled();

}

Footnotes

All public members should have documentation. (link) ↩

gemini-code-assist · 2025-07-29T17:38:37Z

...tter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsControllerDelegator.java

+  public void attach(
+      @Nullable Context context,
+      @NonNull TextureRegistry textureRegistry,
+      @NonNull DartExecutor dartExecutor) {
+    platformViewsController.attach(context, textureRegistry, dartExecutor);
+    platformViewsController2.attach(context, dartExecutor);
+    platformViewsController.getPlatformViewsChannel().setPlatformViewsHandler(this);
+  }


The new public method attach is missing a Javadoc comment. According to the style guide, all public members should be documented.¹

/** * Attaches this platform views controller to its input and output channels. * * @param context The base context that will be passed to embedded views created by this * controller. This should be the context of the Activity hosting the Flutter application. * @param textureRegistry The texture registry which provides the output textures into which the * embedded views will be rendered. * @param dartExecutor The dart execution context, which is used to set up a system channel. */ public void attach( @Nullable Context context, @NonNull TextureRegistry textureRegistry, @NonNull DartExecutor dartExecutor) { platformViewsController.attach(context, textureRegistry, dartExecutor); platformViewsController2.attach(context, dartExecutor); platformViewsController.getPlatformViewsChannel().setPlatformViewsHandler(this); }

Style Guide References

Footnotes

All public members should have documentation. (link) ↩

reidbaker · 2025-07-29T18:36:28Z

...e/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java

    }
  }

+  /** Returns the platform views channel. */


https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-useless-documentation

reidbaker

Don't let ai encourage us to writing pointless documentation.

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

.../shell/platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewsChannel.java

…HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553)

flutter/flutter@46b097a...c3279ca 2025-07-30 [email protected] Roll Skia from 00765f238077 to da5a6704f8a3 (1 revision) (flutter/flutter#172966) 2025-07-30 [email protected] Roll Skia from 62476cd444d2 to 00765f238077 (3 revisions) (flutter/flutter#172955) 2025-07-30 [email protected] Roll Skia from 34693354289f to 62476cd444d2 (1 revision) (flutter/flutter#172950) 2025-07-30 [email protected] Roll Skia from f72bd01a49b1 to 34693354289f (1 revision) (flutter/flutter#172946) 2025-07-30 [email protected] [a11y] add RangeSlider to a11y test app as additional use-case (flutter/flutter#172922) 2025-07-30 [email protected] Roll Skia from a42898e5d622 to f72bd01a49b1 (21 revisions) (flutter/flutter#172944) 2025-07-29 [email protected] Fix SegmentedButton border doesn't reflect states (flutter/flutter#172754) 2025-07-29 [email protected] Fix documentation for FlutterEngineRunTask (flutter/flutter#172889) 2025-07-29 [email protected] Roll Fuchsia Linux SDK from tK_PAaLeo0pUxi8hv... to bQVQlLssTxxLjoDU0... (flutter/flutter#172925) 2025-07-29 [email protected] fix(scrollbar): Update padding type to EdgeInsetsGeometry (flutter/flutter#172056) 2025-07-29 [email protected] Refactor Android platform view code in advance of enabling HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553) 2025-07-29 [email protected] Roll Packages from 6b2e34e to ed235d1 (4 revisions) (flutter/flutter#172905) 2025-07-29 [email protected] Add package PR triage note (flutter/flutter#172898) 2025-07-29 [email protected] Roll Skia from 409e1c7ba09b to a42898e5d622 (29 revisions) (flutter/flutter#172906) 2025-07-29 [email protected] Made `android_gradle_print_build_variants_test.dart` more robust (flutter/flutter#172910) 2025-07-29 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland licenses cpp switch (#172671)" (flutter/flutter#172912) 2025-07-29 [email protected] Fix Gemini Code Assist for GitHub config yaml (flutter/flutter#172887) 2025-07-29 [email protected] Marks Linux_android_emu_unstable android_defines_test to be unflaky (flutter/flutter#171856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r#9699) flutter/flutter@46b097a...c3279ca 2025-07-30 [email protected] Roll Skia from 00765f238077 to da5a6704f8a3 (1 revision) (flutter/flutter#172966) 2025-07-30 [email protected] Roll Skia from 62476cd444d2 to 00765f238077 (3 revisions) (flutter/flutter#172955) 2025-07-30 [email protected] Roll Skia from 34693354289f to 62476cd444d2 (1 revision) (flutter/flutter#172950) 2025-07-30 [email protected] Roll Skia from f72bd01a49b1 to 34693354289f (1 revision) (flutter/flutter#172946) 2025-07-30 [email protected] [a11y] add RangeSlider to a11y test app as additional use-case (flutter/flutter#172922) 2025-07-30 [email protected] Roll Skia from a42898e5d622 to f72bd01a49b1 (21 revisions) (flutter/flutter#172944) 2025-07-29 [email protected] Fix SegmentedButton border doesn't reflect states (flutter/flutter#172754) 2025-07-29 [email protected] Fix documentation for FlutterEngineRunTask (flutter/flutter#172889) 2025-07-29 [email protected] Roll Fuchsia Linux SDK from tK_PAaLeo0pUxi8hv... to bQVQlLssTxxLjoDU0... (flutter/flutter#172925) 2025-07-29 [email protected] fix(scrollbar): Update padding type to EdgeInsetsGeometry (flutter/flutter#172056) 2025-07-29 [email protected] Refactor Android platform view code in advance of enabling HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553) 2025-07-29 [email protected] Roll Packages from 6b2e34e to ed235d1 (4 revisions) (flutter/flutter#172905) 2025-07-29 [email protected] Add package PR triage note (flutter/flutter#172898) 2025-07-29 [email protected] Roll Skia from 409e1c7ba09b to a42898e5d622 (29 revisions) (flutter/flutter#172906) 2025-07-29 [email protected] Made `android_gradle_print_build_variants_test.dart` more robust (flutter/flutter#172910) 2025-07-29 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland licenses cpp switch (#172671)" (flutter/flutter#172912) 2025-07-29 [email protected] Fix Gemini Code Assist for GitHub config yaml (flutter/flutter#172887) 2025-07-29 [email protected] Marks Linux_android_emu_unstable android_defines_test to be unflaky (flutter/flutter#171856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553)

Gray Mackall added 8 commits June 11, 2025 11:01

wip

7822085

just try it

451ad9e

format

0a71d9d

still wip

b29a0a5

nice, it actually works

d02cba9

format

6ca15f5

unify the pv touch type across the channels

381f911

get rid of new suffix

a428d75

github-actions bot added platform-android Android applications specifically engine flutter/engine related. See also e: labels. labels Jun 12, 2025

gmackall changed the title ~~Simplify unified pv api~~ Use HCPP if it is available for all android PV paths Jun 12, 2025

gmackall changed the title ~~Use HCPP if it is available for all android PV paths~~ Use HCPP if it is available/enabled for all android PV paths Jun 12, 2025

gmackall commented Jun 13, 2025

View reviewed changes

dev/integration_tests/android_engine_test/lib/hcpp/platform_view_main.dart Outdated Show resolved Hide resolved

gmackall commented Jun 13, 2025

View reviewed changes

Gray Mackall and others added 5 commits June 13, 2025 13:36

unify the creation request classes

e00f756

format

b301404

build

c3893fe

Merge branch 'master' into simplify_unified_pv_api

d87852a

Merge branch 'master' into simplify_unified_pv_api

56451ec

github-actions bot added the team-android Owned by Android platform team label Jul 8, 2025

gmackall and others added 7 commits July 9, 2025 11:25

Merge branch 'master' into simplify_unified_pv_api

ab7bc41

Merge branch 'master' into simplify_unified_pv_api

22a9ad2

Merge branch 'master' into simplify_unified_pv_api

b826fbc

Merge branch 'master' into simplify_unified_pv_api

27ba5c5

fix tests

5ca8279

format

3c76bae

fix tests for pvc2

5c5083a

This comment was marked as off-topic.

Sign in to view

Gray Mackall added 2 commits July 18, 2025 16:51

fix test

080b7e8

fix test for the nth time

d43744b

Gray Mackall and others added 3 commits July 25, 2025 12:09

comment

e7decc3

format

f0c3f28

Merge branch 'master' into simplify_unified_pv_api

50a7ec9

gmackall requested a review from reidbaker July 25, 2025 16:48

reidbaker approved these changes Jul 25, 2025

View reviewed changes

Merge branch 'master' into simplify_unified_pv_api

ed636fe

gemini-code-assist bot reviewed Jul 29, 2025

View reviewed changes

Gray Mackall added 2 commits July 29, 2025 10:51

random javadocs, a nullable

43f82f5

format

0b307f8

gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

reidbaker reviewed Jul 29, 2025

View reviewed changes

.../src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController2.java Outdated Show resolved Hide resolved

.../shell/platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewsChannel.java Outdated Show resolved Hide resolved

gmackall removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

undo some docs

46b4ad8

gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

auto-submit bot added this pull request to the merge queue Jul 29, 2025

Merged via the queue into flutter:master with commit 5f9b9ab Jul 29, 2025
174 of 175 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 30, 2025

5f9b9ab49 Refactor Android platform view code in advance of enabling …

7791dce

…HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553)

engine-flutter-autoroll mentioned this pull request Jul 30, 2025

Roll Flutter from 46b097a3070d to c3279caa127d (18 revisions) flutter/packages#9699

Merged

Piinks mentioned this pull request Aug 1, 2025

Flutter Notable Commits - 2025 #161460

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025

5f9b9ab49 Refactor Android platform view code in advance of enabling …

169d156

…HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553)

		@@ -0,0 +1,2 @@
		Everything under this file path will have hcpp enabled, see the configuration here:
		https://github.com/flutter/flutter/blob/337a007f5cfb66d0ecc84b1545539231a0be4d61/dev/bots/suite_runners/run_android_engine_tests.dart#L34 No newline at end of file

	RequestedDisplayMode displayMode,
	@Nullable RequestedDisplayMode displayMode,

		public boolean isHcppEnabled();

		void createPlatformViewHcpp(@NonNull PlatformViewCreationRequest request);

Refactor Android platform view code in advance of enabling HCPP on existing PV widgets (behind a flag) #170553

Refactor Android platform view code in advance of enabling HCPP on existing PV widgets (behind a flag) #170553

Uh oh!

Conversation

gmackall commented Jun 12, 2025 • edited by reidbaker Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Uh oh!

gmackall Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gmackall commented Jul 29, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 29, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

gemini-code-assist bot Jul 29, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

gemini-code-assist bot Jul 29, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

gemini-code-assist bot Jul 29, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reidbaker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gmackall commented Jun 12, 2025 •

edited by reidbaker

Loading

gmackall Jun 13, 2025 •

edited

Loading