[Windows] Introduce FlutterWindowsViewController #47242

loic-sharma · 2023-10-24T00:45:14Z

This change begins the migration to the Windows embedder's new ownership model:

Renames FlutterDesktopViewControllerState to FlutterWindowsViewController
Moves ownership of the FlutterWindowsEngine from the FlutterWindowsView to the FlutterWindowsViewController

For more information, refer to: flutter.dev/go/windows-multi-view-ownership-updates

Part of flutter/flutter#137267

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 and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

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

loic-sharma · 2023-10-24T22:15:20Z

shell/platform/windows/window_state.h

-struct FlutterDesktopViewControllerState {
-  // The view that backs this state object.
-  std::unique_ptr<flutter::FlutterWindowsView> view;
-};


This is a legacy struct that is intended to be phased out:

engine/shell/platform/windows/window_state.h

Lines 12 to 15 in e3e59ba

// Structs backing the opaque references used in the C API.

//

// DO NOT ADD ANY NEW CODE HERE. These are legacy, and are being phased out

// in favor of objects that own and manage the relevant functionality.

yaakovschectman

LGTM

dkwingsmt

LGTM

shell/platform/windows/window_state.h

cbracken

lgtm with one question to ponder.

cbracken · 2023-10-25T20:39:36Z

shell/platform/windows/public/flutter_windows.h


-// Opaque reference to a Flutter window controller.
-typedef struct FlutterDesktopViewControllerState*
-    FlutterDesktopViewControllerRef;


Question since this is a public header that is, in rare cases, used directly rather than via the client wrapper: who is this likely to break?

Looking at the current code, I don't actually see anywhere where we've publicly defined FlutterDesktopViewControllerState itself, so it seems unlikely that anyone's going to be depending on it other than as an opaque pointer.

who is this likely to break?

The affected users I'm aware of I found are FFI projects:

Rust: https://github.com/nativeshell/nativeshell/blob/b78fba7ae453fee73ca5666d2175a72d872f77c9/nativeshell/src/shell/platform/win32/flutter_sys.rs#L96-L101

Dart: https://github.com/dart-windows/win32_runner/blob/45207ec3c7ac4b410b1df0d573e8903ee084ad58/lib/src/ffi.dart#L59

.NET: https://github.com/LiveOrNot/FlutterSharp/blob/8b24bdf14465c090b53ecc04c0c2c2598ae7aff3/FlutterSharp/Integrations/FlutterInterop.cs#L21

I didn't find any other uses of this type on GitHub and google3 code searches. It seems the impact of this breaking change is limited. /cc @knopp

I don't actually see anywhere where we've publicly defined FlutterDesktopViewControllerState itself

AFAIK this has always been an opaque pointer.

…ngine#47242)

…137310) Roll Flutter Engine from 6e09ee14e244 to 6428ed576841 (38 revisions) flutter/engine@6e09ee1...6428ed5 2023-10-26 [email protected] Revert Dart SDK to 360370ff93b053253343832432f8329a11372ffc (flutter/engine#47326) 2023-10-25 [email protected] Roll Skia from fb979d0ce053 to a5da4435bb5c (1 revision) (flutter/engine#47325) 2023-10-25 [email protected] [Impeller] Cache location in metadata. (flutter/engine#46640) 2023-10-25 [email protected] Roll Skia from 8492914821fa to fb979d0ce053 (1 revision) (flutter/engine#47324) 2023-10-25 [email protected] [Windows] Introduce FlutterWindowsViewController (flutter/engine#47242) 2023-10-25 [email protected] Roll Skia from 773abacc1581 to 8492914821fa (2 revisions) (flutter/engine#47322) 2023-10-25 [email protected] Roll Dart SDK from 73fa7b3e048a to 7745c140d1fe (1 revision) (flutter/engine#47321) 2023-10-25 [email protected] Cleanup Dart package dependencies a bit (flutter/engine#47306) 2023-10-25 [email protected] Roll Skia from d511fa1f1533 to 773abacc1581 (1 revision) (flutter/engine#47312) 2023-10-25 [email protected] Roll Fuchsia Mac SDK from SOv1RJmbyDByvud8D... to kNdyn03p28H7VeMcd... (flutter/engine#47313) 2023-10-25 [email protected] Roll Fuchsia Linux SDK from vX5n99OIWgLX6bObI... to zTq0jH2Y3Fl0uUUSa... (flutter/engine#47309) 2023-10-25 [email protected] Roll Skia from 0748053419cd to d511fa1f1533 (1 revision) (flutter/engine#47308) 2023-10-25 [email protected] Missed linter argument comment violations (flutter/engine#47310) 2023-10-25 [email protected] [Impeller] Remove use of FML_DISALLOW_<FOO> macros in Impeller. (flutter/engine#47307) 2023-10-25 [email protected] Adds lint for checking argument commments (flutter/engine#47305) 2023-10-25 [email protected] Roll Dart SDK from 2791495ed583 to 73fa7b3e048a (1 revision) (flutter/engine#47304) 2023-10-25 [email protected] Roll Skia from 4848dd9b5dab to 0748053419cd (2 revisions) (flutter/engine#47302) 2023-10-25 [email protected] Roll Skia from 788eafdcf70d to 4848dd9b5dab (1 revision) (flutter/engine#47301) 2023-10-25 [email protected] Roll Skia from 5c315b761a24 to 788eafdcf70d (1 revision) (flutter/engine#47300) 2023-10-25 [email protected] Roll Skia from 18ad006ec7f9 to 5c315b761a24 (1 revision) (flutter/engine#47299) 2023-10-25 [email protected] Roll Dart SDK from 57661d5dbc1e to 2791495ed583 (1 revision) (flutter/engine#47298) 2023-10-25 [email protected] Roll Skia from 9537c25509ea to 18ad006ec7f9 (1 revision) (flutter/engine#47297) 2023-10-25 [email protected] Roll Fuchsia Mac SDK from YqSO1OByhoexFJSCr... to SOv1RJmbyDByvud8D... (flutter/engine#47296) 2023-10-25 [email protected] Roll Skia from d51d3bcbcacd to 9537c25509ea (1 revision) (flutter/engine#47295) 2023-10-25 [email protected] Roll Skia from f98fc9536c43 to d51d3bcbcacd (1 revision) (flutter/engine#47294) 2023-10-25 [email protected] Roll Fuchsia Linux SDK from tqukMrdJ6j-845-57... to vX5n99OIWgLX6bObI... (flutter/engine#47293) 2023-10-25 [email protected] Roll Skia from 0d4fcf388a6f to f98fc9536c43 (1 revision) (flutter/engine#47292) 2023-10-24 [email protected] [Impeller] Update shader compilation pipeline documentation to detail multi-arch archives. (flutter/engine#47289) 2023-10-24 [email protected] Roll Skia from 076a9dd0094f to 0d4fcf388a6f (2 revisions) (flutter/engine#47290) 2023-10-24 [email protected] OpenGL <4.x does not support stencil-only formats, so delete the TODO (flutter/engine#47286) 2023-10-24 [email protected] [Impeller] Unconditionally dither in gradient shader fragments. (flutter/engine#46746) 2023-10-24 [email protected] Roll Skia from f491209e969d to 076a9dd0094f (1 revision) (flutter/engine#47285) 2023-10-24 [email protected] [Impeller] Add support for multi-rendering-backend fat shader archives. (flutter/engine#47278) 2023-10-24 [email protected] Roll Skia from 502277be15cf to f491209e969d (1 revision) (flutter/engine#47281) 2023-10-24 [email protected] Roll Dart SDK from 360370ff93b0 to 57661d5dbc1e (1 revision) (flutter/engine#47280) 2023-10-24 [email protected] Migrated away from UnmodifiableUint8ListView (flutter/engine#47276) 2023-10-24 [email protected] Roll Skia from bc90585b0dd4 to 502277be15cf (1 revision) (flutter/engine#47277) 2023-10-24 [email protected] Reland "Use a single OffscreenCanvas for rendering in CanvasKit (#45744)" (flutter/engine#47241) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from tqukMrdJ6j-8 to zTq0jH2Y3Fl0 fuchsia/sdk/core/mac-amd64 from YqSO1OByhoex to kNdyn03p28H7 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll ...

[Windows] Introduce FlutterWindowsViewController

5fd3b80

github-actions bot added affects: desktop platform-windows labels Oct 24, 2023

Update license

a5e77c3

loic-sharma marked this pull request as ready for review October 24, 2023 21:51

loic-sharma requested review from cbracken, dkwingsmt and yaakovschectman October 24, 2023 21:51

loic-sharma commented Oct 24, 2023

View reviewed changes

yaakovschectman approved these changes Oct 25, 2023

View reviewed changes

dkwingsmt approved these changes Oct 25, 2023

View reviewed changes

shell/platform/windows/window_state.h Outdated Show resolved Hide resolved

loic-sharma added 2 commits October 25, 2023 13:20

Feedback

f7a8ced

Tweak comment

f03eb6b

cbracken approved these changes Oct 25, 2023

View reviewed changes

loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 25, 2023

auto-submit bot merged commit df0b200 into flutter:main Oct 25, 2023

engine-flutter-autoroll mentioned this pull request Oct 25, 2023

Roll Flutter Engine from 6e09ee14e244 to 6de8127713bc (36 revisions) flutter/flutter#137300

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 25, 2023

df0b20020 [Windows] Introduce FlutterWindowsViewController (flutter/e…

fea7a97

…ngine#47242)

engine-flutter-autoroll mentioned this pull request Oct 26, 2023

Roll Flutter Engine from 6e09ee14e244 to 236562dacd71 (37 revisions) flutter/flutter#137305

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2023

df0b20020 [Windows] Introduce FlutterWindowsViewController (flutter/e…

59a9f23

…ngine#47242)

engine-flutter-autoroll mentioned this pull request Oct 26, 2023

Roll Flutter Engine from 6e09ee14e244 to 6428ed576841 (38 revisions) flutter/flutter#137310

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2023

df0b20020 [Windows] Introduce FlutterWindowsViewController (flutter/e…

e152357

…ngine#47242)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Windows] Introduce FlutterWindowsViewController #47242

[Windows] Introduce FlutterWindowsViewController #47242

Uh oh!

loic-sharma commented Oct 24, 2023 •

edited

Loading

Uh oh!

loic-sharma Oct 24, 2023

Uh oh!

yaakovschectman left a comment

Uh oh!

dkwingsmt left a comment

Uh oh!

Uh oh!

cbracken left a comment

Uh oh!

cbracken Oct 25, 2023

Uh oh!

loic-sharma Oct 25, 2023 •

edited

Loading

Uh oh!

Uh oh!

	// Structs backing the opaque references used in the C API.
	//
	// DO NOT ADD ANY NEW CODE HERE. These are legacy, and are being phased out
	// in favor of objects that own and manage the relevant functionality.

[Windows] Introduce FlutterWindowsViewController #47242

[Windows] Introduce FlutterWindowsViewController #47242

Uh oh!

Conversation

loic-sharma commented Oct 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

loic-sharma Oct 24, 2023

Choose a reason for hiding this comment

Uh oh!

yaakovschectman left a comment

Choose a reason for hiding this comment

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cbracken left a comment

Choose a reason for hiding this comment

Uh oh!

cbracken Oct 25, 2023

Choose a reason for hiding this comment

Uh oh!

loic-sharma Oct 25, 2023 • 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!

Uh oh!

loic-sharma commented Oct 24, 2023 •

edited

Loading

loic-sharma Oct 25, 2023 •

edited

Loading