Skip to content

Conversation

@robert-ancell
Copy link
Contributor

This function doesn't seem to serve any purpose.
Top level is not a term we are currently using in the multi window code. If we want to track if we have open windows, this can be done in the Flutter framework - it doesn't need to be done at the platform level.

This function doesn't seem to serve any purpose.
Top level is not a term we are currently using in the multi window code.
If we want to track if we have open windows, this can be done in the Flutter framework - it doesn't need to be done at the platform level.
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically a: desktop Running on desktop labels Nov 5, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the WindowingOwner.hasTopLevelWindows function, which is no longer needed. The changes are applied consistently across the C++ engine, the Dart framework, and related tests. The removal is clean and aligns with the goal of simplifying the multi-windowing code. I've identified one test case that appears to have been removed unintentionally and have suggested its restoration to maintain test coverage.

@robert-ancell
Copy link
Contributor Author

Perhaps this function was pre-FFI where you needed to get this information from native code? If the purpose is to track windows created in native code and not in the Dart platform then this should be hasNativeWindows() and the Dart created windows tracked in a platform independent way.

@mattkae mattkae requested a review from knopp November 6, 2025 09:15
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! Just need to fix CI

@robert-ancell robert-ancell added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Nov 6, 2025
Merged via the queue into flutter:master with commit c5e809a Nov 6, 2025
186 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2025
@robert-ancell robert-ancell deleted the remove-has-toplevel-windows branch November 6, 2025 13:55
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 6, 2025
flutter/flutter@e5d5c01...c5e809a

2025-11-06 [email protected] Remove WindowingOwner.hasTopLevelWindows (flutter/flutter#178033)
2025-11-06 [email protected] Fix DropdownMenu escape key does not close the menu (flutter/flutter#178002)
2025-11-06 [email protected] Update documentation tool reference in image.dart (flutter/flutter#177782)
2025-11-06 [email protected] Roll Skia from 4eb2383d38f2 to 5c4e1352128f (5 revisions) (flutter/flutter#178094)
2025-11-06 [email protected] Revert "Refactor OverlayPortal semantics (#173005)" (flutter/flutter#178007)
2025-11-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fix verified input test failure in CI (attempt 4) (#178018)" (flutter/flutter#178089)
2025-11-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[web] Unify Surface code between Skwasm and CanvasKit (#177138)" (flutter/flutter#178085)
2025-11-05 [email protected] Fix verified input test failure in CI (attempt 4) (flutter/flutter#178018)
2025-11-05 [email protected] fix: inconsistent horizontal spacing between hours and mins in time picker for non-english language (flutter/flutter#173706)
2025-11-05 [email protected] Roll Fuchsia Linux SDK from mpsxF1gd-jbKNvmpm... to cm88aTLui5yorSGYQ... (flutter/flutter#178074)
2025-11-05 [email protected] Fix(ios): Remove arm64 exclusion to support Xcode 26 simulators (flutter/flutter#177065)
2025-11-05 [email protected] Roll Skia from 2ff897e9b440 to 4eb2383d38f2 (18 revisions) (flutter/flutter#178070)
2025-11-05 [email protected] [web] Unify Surface code between Skwasm and CanvasKit (flutter/flutter#177138)
2025-11-05 [email protected] Update more missing ninja deps (flutter/flutter#178079)
2025-11-05 [email protected] Add ninja / cmake deps to failing tests (flutter/flutter#178054)
2025-11-05 [email protected] [web] Don't add webparagraph suite to CI (flutter/flutter#177681)
2025-11-05 [email protected] Fixing broken link in engine readme (flutter/flutter#177987)
2025-11-05 [email protected] Print reason for adb command failure in verified input test (attempt 3) (flutter/flutter#178005)
2025-11-05 [email protected] Fix `ReorderableList` items jumping when drag direction reverses mid-animation (flutter/flutter#173241)
2025-11-05 [email protected] Replace deprecated `withOpacity` in `overflow_bar.0.dart‎` example (flutter/flutter#177813)
2025-11-05 [email protected] Replace deprecated `withOpacity` in `data_table.1.dart‎` example (flutter/flutter#177812)
2025-11-05 [email protected] Replace deprecated `withOpacity` in `switch.1.dart` example (flutter/flutter#177811)
2025-11-04 [email protected] Roll Skia from c89b6118266b to 2ff897e9b440 (6 revisions) (flutter/flutter#177999)
2025-11-04 [email protected] Fix verified input test in CI (attempt 2) (flutter/flutter#177961)
2025-11-04 [email protected] Replace rendering for solid color circles (both filled and stroked) to use SDFs (flutter/flutter#177482)
2025-11-04 [email protected] Roll Fuchsia Linux SDK from vxK5obzfr1X9P2kSh... to mpsxF1gd-jbKNvmpm... (flutter/flutter#177996)
2025-11-04 [email protected] Validate that platforms specified in .ci.yaml target names match the platforms defined in the platform_properties section (flutter/flutter#177523)
2025-11-04 [email protected] Roll Packages from 1a7075b to 3d926aa (6 revisions) (flutter/flutter#177998)
2025-11-04 [email protected] Remove dead code from snippet_generator (flutter/flutter#174440)

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] 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
IvoneDjaja pushed a commit to IvoneDjaja/flutter that referenced this pull request Nov 22, 2025
This function doesn't seem to serve any purpose.
Top level is not a term we are currently using in the multi window code.
If we want to track if we have open windows, this can be done in the
Flutter framework - it doesn't need to be done at the platform level.
bufffun pushed a commit to bufffun/flutter that referenced this pull request Nov 26, 2025
This function doesn't seem to serve any purpose.
Top level is not a term we are currently using in the multi window code.
If we want to track if we have open windows, this can be done in the
Flutter framework - it doesn't need to be done at the platform level.
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
This function doesn't seem to serve any purpose.
Top level is not a term we are currently using in the multi window code.
If we want to track if we have open windows, this can be done in the
Flutter framework - it doesn't need to be done at the platform level.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop engine flutter/engine related. See also e: labels. framework flutter/packages/flutter repository. See also f: labels. platform-windows Building on or for Windows specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants