Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Remove single view assumptions from window.dart #38453

Merged
auto-submit merged 46 commits into from
Jan 5, 2023
Merged

Remove single view assumptions from window.dart #38453

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
5f9cbb0
Refactor `window.dart`
Dec 21, 2022
1f5376a
Remove single window assumption from platform dispatcher
Dec 21, 2022
def8a49
Expose viewId
Dec 21, 2022
ee1d3a3
Remove FlutterWindow from web_ui
Dec 22, 2022
59a1b34
Refactor EngineFlutterWindow to inherit from FlutterView
Dec 22, 2022
559ab5c
Rename window property
Dec 22, 2022
3ef5f40
Undo Iterable -> Map conversion
Dec 22, 2022
368ec15
Add window and deprecate it so that the change isn't breaking
Dec 22, 2022
570cc18
Name resolution
Dec 22, 2022
109c396
Revisions
Dec 22, 2022
b08531d
Doc changes
Dec 22, 2022
d8425c2
Refactor deprecation message
Jan 3, 2023
6a8fd2e
Newline
Jan 3, 2023
4c659b1
Expose getViewById, hide map interface
Jan 3, 2023
48832da
Fix compilaiton errors
Jan 3, 2023
9504d93
Introduce addView API
Jan 3, 2023
bdb74d2
Change deprecation message
Jan 3, 2023
4453c99
Update lib/ui/window.dart
a-wallen Jan 3, 2023
5103c3a
Take greg's todos
Jan 3, 2023
184e658
Add mutual exclusion assertion
Jan 3, 2023
864a0ee
Fix trailing whitespace lint
Jan 3, 2023
0ae4700
Use only one property to store backing view
Dec 22, 2022
56e558c
Add doc comment to view
Dec 22, 2022
278f724
Document view and window parameters in the constructor of ViewConfigu…
Dec 22, 2022
ce6db62
Sync web api
Dec 22, 2022
23be98b
Refactor assertion
Dec 22, 2022
40922b5
Improve deprecation message
Dec 22, 2022
75d1cbe
Improve window documentation
Dec 22, 2022
e8df09f
Assert one of window/view is null in copyWith ViewConfiguration
Dec 22, 2022
0209cf6
Remove EngineFlutterWindowView
Dec 22, 2022
ec25975
Make dartdoc happy
Dec 22, 2022
8199e72
Refactor copyWith()
Dec 22, 2022
b3164c8
Change to internal map implementation
Dec 22, 2022
d42d9ec
final private refactor
a-wallen Jan 4, 2023
5c2e0a9
Deprecate window parameter in copyWith
Dec 22, 2022
12d4871
Repl Window w/ View
Dec 22, 2022
15ebe59
Add tests for viewConfiguration
Dec 22, 2022
1f71e01
Make test descriptions better
Dec 22, 2022
6d531ba
Add ViewConfiguration initialization with window tests
Dec 22, 2022
3a70482
Update lib/web_ui/lib/src/engine/window.dart
a-wallen Jan 5, 2023
2a4f807
Refactor TODO message
Dec 22, 2022
9f4ac26
Add expectation :)
Dec 22, 2022
b2b410e
Fix viewId in window.dart
Dec 22, 2022
4bbd09d
Remove double deprecation access
Dec 22, 2022
82defe4
punctuation
a-wallen Jan 5, 2023
8fa153b
Add kSingletonWindowID const
Dec 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions lib/ui/platform_dispatcher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,10 @@ class PlatformDispatcher {
final ViewConfiguration previousConfiguration =
_viewConfigurations[id] ?? const ViewConfiguration();
if (!_views.containsKey(id)) {
_views[id] = FlutterWindow._(id, this);
_views[id] = FlutterView._(id, this);
}
_viewConfigurations[id] = previousConfiguration.copyWith(
view: _views[id],
window: _views[id],
devicePixelRatio: devicePixelRatio,
geometry: Rect.fromLTWH(0.0, 0.0, width, height),
Expand Down Expand Up @@ -1290,6 +1291,10 @@ class PlatformConfiguration {
class ViewConfiguration {
/// A const constructor for an immutable [ViewConfiguration].
Comment thread
dkwingsmt marked this conversation as resolved.
const ViewConfiguration({
this.view,
@Deprecated('''
Comment thread
goderbauer marked this conversation as resolved.

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ultimately, it's up to you, but I would probably do the rename (with deprecation) in a separate PR that's dedicated to just that. That may make it easier to land these two changes and potentially roll back if there are issues.

@a-wallen a-wallen Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds like a good idea. I'd like to address all the feedback and get an LGTM before I cherry-pick the commits from this PR into another one.

Renaming window to view since `FlutterWindow` has been removed from dart::ui.

@dkwingsmt dkwingsmt Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. I don't understand this deprecation message. It should be something actionable in the form of "Use xxxx instead." For example, "Use the view argument instead. The window argument has been renamed to view."
  2. If it's a rename, shouldn't we point them to the same value? The two parameter should be exclusive (assert that only one of them is non-null) and be assigned to the same field (_view) and both getter should return this field.

@a-wallen a-wallen Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right 👍

@a-wallen a-wallen Jan 3, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How about

      `FlutterWindow` has been removed from dart::ui, and the window property is no longer semantically sound
       use the `view` property instead.

@dkwingsmt dkwingsmt Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In my opinion the explanation is unnecessary here. Just tell the user how to change it. If you really want to explain it, add a link to the design doc, or to this PR with an expanded paragraph of description.

@loic-sharma loic-sharma Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reopening as I don't think the second part was addressed:

If it's a rename, shouldn't we point them to the same value? The two parameter should be exclusive (assert that only one of them is non-null) and be assigned to the same field (_view) and both getter should return this field.

''')
this.window,
this.devicePixelRatio = 1.0,
this.geometry = Rect.zero,
Expand All @@ -1304,6 +1309,7 @@ class ViewConfiguration {

/// Copy this configuration with some fields replaced.
ViewConfiguration copyWith({
FlutterView? view,

@goderbauer goderbauer Jan 4, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Deprecate the window parameter in the copyWith functions as well? That one will also need to be removed at some point, right?

@a-wallen a-wallen Jan 4, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

5c2e0a9

FlutterView? window,

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

copy with should do something with the window property instead of silently ignoring it.

double? devicePixelRatio,
Rect? geometry,
Expand All @@ -1316,6 +1322,7 @@ class ViewConfiguration {
List<DisplayFeature>? displayFeatures,
}) {
return ViewConfiguration(

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should probably assert that at least one of window, view is null

@a-wallen a-wallen Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added 7d2ad3e.

view: view ?? this.view,

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you need to deal with the window here in addition to the view for backwards compatibility reasons:

window: window ?? this.window,

@a-wallen a-wallen Jan 4, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll get an error that the assert(window != null || view != null) cannot be optimized since both the window and view properties have the same backing property.

@a-wallen a-wallen Jan 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This snippet would guarantee that both window and view aren't provided to the ViewConfiguration constructor and ensure that the backing property, _view gets the proper value during .copyWith()

assert(window != null || view != null);
return ViewConfiguration(
	view: view ?? window ?? _view,
);

@dkwingsmt dkwingsmt Jan 4, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah this is also what I would go for.

@a-wallen a-wallen Jan 4, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @dkwingsmt 👍

@a-wallen a-wallen Jan 4, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

8199e72 has the changes requested.

window: window ?? this.window,
devicePixelRatio: devicePixelRatio ?? this.devicePixelRatio,
geometry: geometry ?? this.geometry,
Expand All @@ -1333,7 +1340,11 @@ class ViewConfiguration {
/// relative to.

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not directly related to your PR, but this doc comment doesn't make much sense: "[The] view into which the view is placed"?

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similarly, the comment below: What is the "top level view itself"?

@a-wallen a-wallen Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How about this 75d1cbe?

///
/// If null, then this configuration represents a top level view itself.
@Deprecated('''
Renaming window to view since `FlutterWindow` has been removed from dart::ui.
''')
final FlutterView? window;
final FlutterView? view;
Comment thread
a-wallen marked this conversation as resolved.
Outdated

/// The pixel density of the output surface.
final double devicePixelRatio;
Expand Down Expand Up @@ -1427,7 +1438,7 @@ class ViewConfiguration {

@override
String toString() {
return '$runtimeType[window: $window, geometry: $geometry]';
return '$runtimeType[view: $view, geometry: $geometry]';
}
}

Expand Down Expand Up @@ -1666,7 +1677,7 @@ enum AppLifecycleState {
/// On Android, this corresponds to an app or the Flutter host view running
/// in the foreground inactive state. Apps transition to this state when
/// another activity is focused, such as a split-screen app, a phone call,
/// a picture-in-picture app, a system dialog, or another window.
/// a picture-in-picture app, a system dialog, or another view.
///
/// Apps in this state should assume that they may be [paused] at any time.
inactive,
Expand Down
60 changes: 16 additions & 44 deletions lib/ui/window.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ part of dart.ui;

/// A view into which a Flutter [Scene] is drawn.
///
/// Each [FlutterView] has its own layer tree that is rendered into an area
/// inside of a [FlutterWindow] whenever [render] is called with a [Scene].
/// Each [FlutterView] has its own layer tree that is shown whenever [render]
/// is called on it with a [Scene].
Comment thread
a-wallen marked this conversation as resolved.
Outdated
///
/// ## Insets and Padding
///
Expand Down Expand Up @@ -51,18 +51,22 @@ part of dart.ui;
/// the [viewPadding] anyway, so there is no need to account for
/// that in the [padding], which is always safe to use for such
/// calculations.
///
/// See also:
///
/// * [FlutterWindow], a special case of a [FlutterView] that is represented on
/// the platform as a separate window which can host other [FlutterView]s.
abstract class FlutterView {
class FlutterView {
FlutterView._(this._viewId, this.platformDispatcher);

/// The opaque ID for this view.
final Object _viewId;

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

move the private field below the getter. Our style is to have the following order: getter, private field, setter. The setter may be optional, though.

@a-wallen a-wallen Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.

Is that documented somewhere?

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I actually couldn't find this in the style guide, but it is something we try to follow. We should add it.

Comment thread
dkwingsmt marked this conversation as resolved.
Outdated
Object get viewId => _viewId;
Comment thread
a-wallen marked this conversation as resolved.
Outdated

/// The platform dispatcher that this view is registered with, and gets its
/// information from.
PlatformDispatcher get platformDispatcher;
final PlatformDispatcher platformDispatcher;

/// The configuration of this view.
ViewConfiguration get viewConfiguration;
ViewConfiguration get viewConfiguration {
assert(platformDispatcher._viewConfigurations.containsKey(_viewId));
return platformDispatcher._viewConfigurations[_viewId]!;
}

/// The number of device pixels for each logical pixel for the screen this
/// view is displayed on.
Expand Down Expand Up @@ -281,39 +285,7 @@ abstract class FlutterView {
external static void _updateSemantics(SemanticsUpdate update);
}

/// A top-level platform window displaying a Flutter layer tree drawn from a
/// [Scene].
///
/// The current list of all Flutter views for the application is available from
/// `WidgetsBinding.instance.platformDispatcher.views`. Only views that are of type
/// [FlutterWindow] are top level platform windows.
///
/// There is also a [PlatformDispatcher.instance] singleton object in `dart:ui`
/// if `WidgetsBinding` is unavailable, but we strongly advise avoiding a static
/// reference to it. See the documentation for [PlatformDispatcher.instance] for
/// more details about why it should be avoided.
///
/// See also:
///
/// * [PlatformDispatcher], which manages the current list of [FlutterView] (and
/// thus [FlutterWindow]) instances.
class FlutterWindow extends FlutterView {
FlutterWindow._(this._windowId, this.platformDispatcher);

/// The opaque ID for this view.
final Object _windowId;

@override
final PlatformDispatcher platformDispatcher;

@override
ViewConfiguration get viewConfiguration {
assert(platformDispatcher._viewConfigurations.containsKey(_windowId));
return platformDispatcher._viewConfigurations[_windowId]!;
}
}

/// A [FlutterWindow] that includes access to setting callbacks and retrieving
/// A [FlutterView] that includes access to setting callbacks and retrieving
/// properties that reside on the [PlatformDispatcher].
///
/// It is the type of the global [window] singleton used by applications that
Expand All @@ -328,7 +300,7 @@ class FlutterWindow extends FlutterView {
/// `WidgetsBinding.instance.platformDispatcher` over a static reference to
/// [window], or [PlatformDispatcher.instance]. See the documentation for
/// [PlatformDispatcher.instance] for more details about this recommendation.
class SingletonFlutterWindow extends FlutterWindow {
class SingletonFlutterWindow extends FlutterView {
Comment thread
dkwingsmt marked this conversation as resolved.
SingletonFlutterWindow._(super.windowId, super.platformDispatcher)
: super._();

Expand Down
16 changes: 13 additions & 3 deletions lib/web_ui/lib/platform_dispatcher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ class PlatformConfiguration {

class ViewConfiguration {
const ViewConfiguration({
this.view,

@dkwingsmt dkwingsmt Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The web side should be treated the same as ui.

@Deprecated('''
Renaming window to view as the class `FlutterWindow` has been deprecated.

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: keep these consistent with the other file?

''')
this.window,
this.devicePixelRatio = 1.0,
this.geometry = Rect.zero,
Expand All @@ -203,7 +207,8 @@ class ViewConfiguration {
});

ViewConfiguration copyWith({
FlutterWindow? window,
FlutterView? view,
FlutterView? window,

@goderbauer goderbauer Jan 4, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Deprecate window param?

@a-wallen a-wallen Jan 4, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

5c2e0a9

double? devicePixelRatio,
Rect? geometry,
bool? visible,
Expand All @@ -215,6 +220,7 @@ class ViewConfiguration {
List<DisplayFeature>? displayFeatures,
}) {
return ViewConfiguration(

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

assert here that at most of view, window is not-null.

view: view ?? this.view,

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

assert that only one is provided and add

window: window ?? this.window,

@a-wallen a-wallen Jan 4, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved. See #38453 (comment).

window: window ?? this.window,
devicePixelRatio: devicePixelRatio ?? this.devicePixelRatio,
geometry: geometry ?? this.geometry,
Expand All @@ -228,7 +234,11 @@ class ViewConfiguration {
);
}

final FlutterWindow? window;
@Deprecated('''
Renaming window to view as the class `FlutterWindow` has been deprecated.

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

keep these consistent.

''')
final FlutterView? window;
final FlutterView? view;
final double devicePixelRatio;
final Rect geometry;
final bool visible;
Expand All @@ -241,7 +251,7 @@ class ViewConfiguration {

@override
String toString() {
return '$runtimeType[window: $window, geometry: $geometry]';
return '$runtimeType[view: $view, geometry: $geometry]';
}
}

Expand Down
13 changes: 7 additions & 6 deletions lib/web_ui/lib/src/engine/platform_dispatcher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,10 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {

/// The current list of windows,
Comment thread
a-wallen marked this conversation as resolved.
Outdated
@override
Iterable<ui.FlutterView> get views => _windows.values;
Map<Object, ui.FlutterWindow> get windows => _windows;
final Map<Object, ui.FlutterWindow> _windows = <Object, ui.FlutterWindow>{};
Iterable<ui.FlutterView> get views => _views.values;
final Map<Object, ui.FlutterView> _views = <Object, ui.FlutterView>{};
ui.FlutterView? getViewById(Object id) => _views[id];
void addView(Object id, ui.FlutterView view) => _views[id] = view;

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is all private, you could just replace all of this with something like:

Suggested change
final Map<Object, ui.FlutterView> _views = <Object, ui.FlutterView>{};
ui.FlutterView? getViewById(Object id) => _views[id];
void addView(Object id, ui.FlutterView view) => _views[id] = view;
final Map<Object, ui.FlutterView> idToView = <Object, ui.FlutterView>{};

... and then elsewhere operate directly with that map.


/// A map of opaque platform window identifiers to window configurations.
///
Expand Down Expand Up @@ -481,7 +482,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
// TODO(gspencergoog): As multi-window support expands, the pop call
// will need to include the window ID. Right now only one window is
Comment thread
a-wallen marked this conversation as resolved.
Outdated
// supported.
(_windows[0]! as EngineFlutterWindow)
(_views[0]! as EngineFlutterWindow)
.browserHistory
.exit()
.then((_) {
Expand Down Expand Up @@ -571,7 +572,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
// TODO(gspencergoog): As multi-window support expands, the navigation call
// will need to include the window ID. Right now only one window is
// supported.
(_windows[0]! as EngineFlutterWindow)
(_views[0]! as EngineFlutterWindow)
.handleNavigationMessage(data)
.then((bool handled) {
if (handled) {
Expand Down Expand Up @@ -1160,7 +1161,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
@override
String get defaultRouteName {
return _defaultRouteName ??=
(_windows[0]! as EngineFlutterWindow).browserHistory.currentPath;
(_views[0]! as EngineFlutterWindow).browserHistory.currentPath;
}

/// Lazily initialized when the `defaultRouteName` getter is invoked.
Expand Down
20 changes: 13 additions & 7 deletions lib/web_ui/lib/src/engine/window.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ set customUrlStrategy(UrlStrategy? strategy) {

/// The Web implementation of [ui.SingletonFlutterWindow].
class EngineFlutterWindow extends ui.SingletonFlutterWindow {
EngineFlutterWindow(this._windowId, this.platformDispatcher) {
EngineFlutterWindow(this._viewId, this.platformDispatcher) {
final EnginePlatformDispatcher engineDispatcher =
platformDispatcher as EnginePlatformDispatcher;
engineDispatcher.windows[_windowId] = this;
engineDispatcher.windowConfigurations[_windowId] = const ui.ViewConfiguration();
engineDispatcher.addView(_viewId, this);
engineDispatcher.windowConfigurations[_viewId] = const ui.ViewConfiguration();
if (_isUrlStrategySet) {
_browserHistory = createHistoryForExistingState(_customUrlStrategy);
}
Expand All @@ -58,7 +58,10 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow {
});
}

final Object _windowId;
final Object _viewId;

@override
Object get viewId => _viewId;
Comment thread
a-wallen marked this conversation as resolved.
Outdated

@override
final ui.PlatformDispatcher platformDispatcher;
Expand Down Expand Up @@ -202,8 +205,8 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow {
ui.ViewConfiguration get viewConfiguration {
final EnginePlatformDispatcher engineDispatcher =
platformDispatcher as EnginePlatformDispatcher;
assert(engineDispatcher.windowConfigurations.containsKey(_windowId));
return engineDispatcher.windowConfigurations[_windowId] ??
assert(engineDispatcher.windowConfigurations.containsKey(_viewId));
return engineDispatcher.windowConfigurations[_viewId] ??
const ui.ViewConfiguration();
}

Expand Down Expand Up @@ -340,11 +343,14 @@ class EngineSingletonFlutterWindow extends EngineFlutterWindow {
}

/// A type of [FlutterView] that can be hosted inside of a [FlutterWindow].

@goderbauer goderbauer Jan 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

update the comment since FlutterWindow doesn't exist anymore.

The name of the class "EngineFlutterWindowView" also seems odd with the "Window" in there. Not sure what this is used for, but is this still needed?

class EngineFlutterWindowView extends ui.FlutterWindow {
class EngineFlutterWindowView extends ui.FlutterView {
EngineFlutterWindowView._(this._viewId, this.platformDispatcher);

final Object _viewId;

@override
Object get viewId => _viewId;

@override
final ui.PlatformDispatcher platformDispatcher;

Expand Down
11 changes: 2 additions & 9 deletions lib/web_ui/lib/window.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ part of ui;
abstract class FlutterView {
PlatformDispatcher get platformDispatcher;
ViewConfiguration get viewConfiguration;
Object get viewId;
double get devicePixelRatio => viewConfiguration.devicePixelRatio;
Rect get physicalGeometry => viewConfiguration.geometry;
Size get physicalSize => viewConfiguration.geometry.size;
Expand All @@ -19,15 +20,7 @@ abstract class FlutterView {
void updateSemantics(SemanticsUpdate update) => platformDispatcher.updateSemantics(update);
}

abstract class FlutterWindow extends FlutterView {
@override
PlatformDispatcher get platformDispatcher;

@override
ViewConfiguration get viewConfiguration;
}

abstract class SingletonFlutterWindow extends FlutterWindow {
abstract class SingletonFlutterWindow extends FlutterView {
VoidCallback? get onMetricsChanged => platformDispatcher.onMetricsChanged;
set onMetricsChanged(VoidCallback? callback) {
platformDispatcher.onMetricsChanged = callback;
Expand Down