-
Notifications
You must be signed in to change notification settings - Fork 6k
[dart:ui] Introduce PlatformDispatcher.implicitView
#39553
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,6 +168,36 @@ class PlatformDispatcher { | |
| // A map of opaque platform view identifiers to view configurations. | ||
| final Map<Object, ViewConfiguration> _viewConfigurations = <Object, ViewConfiguration>{}; | ||
|
|
||
| /// The [FlutterView] provided by the engine if the platform is unable to | ||
| /// create windows, or, for backwards compatibility. | ||
| /// | ||
| /// If the platform provides an implicit view, it can be used to bootstrap | ||
| /// the framework. This is common for platforms designed for single-view | ||
| /// applications like mobile devices with a single display. | ||
| /// | ||
| /// Applications and libraries must not rely on this property being set | ||
| /// as it may be null depending on the engine's configuration. Instead, | ||
| /// consider using [View.of] to lookup the [FlutterView] the current | ||
| /// [BuildContext] is drawing into. | ||
| /// | ||
| /// While the properties on the referenced [FlutterView] may change, | ||
| /// the reference itself is guaranteed to never change over the lifetime | ||
| /// of the application: if this property is null at startup, it will remain | ||
| /// so throughout the entire lifetime of the application. If it points to a | ||
| /// specific [FlutterView], it will continue to point to the same view until | ||
| /// the application is shut down (although the engine may replace or remove | ||
| /// the underlying backing surface of the view at its discretion). | ||
| /// | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add some kind of a warning here that applications and packages should not rely on this property being set and instead acquire the view via other means (e.g.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I guess you're sort of hinting at this with the "see also" section)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you take another look? I updated this comment to push people towards
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took a stab below at how I would write the documentation for this property. Feel free to copy it as a whole or bits and pieces that you like or ignore it completely. The implicit [FlutterView] provided by the platform, if any. If the platform provides an implicit view, it can be used to bootstrap the framework. This is common for platforms designed for single-view applications (e.g. on mobile devices with a single display). Applications and libraries must not rely on this property being set as it may be null depending on the engine's configuration. Instead, consider using [View.of] to lookup the [FlutterView] the current [BuildContext] is drawing into. While the properties on the referenced [FlutterView] may change, the reference itself is guaranteed to never change over the lifetime of the application: If this property is null at startup, it will remain so throughout the entire lifetime of the application. If it points to a specific [FlutterView], it will continue to point to the same view until the application is shut down (although the engine may replace or remove the underlying backing surface of the view at its discretion). See also:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's perfect, will update!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated! FYI, I added
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added just a couple more suggestions above but this is looking pretty great! |
||
| /// See also: | ||
| /// | ||
| /// * [View.of], for accessing the current view. | ||
| /// * [PlatformDisptacher.views] for a list of all [FlutterView]s provided | ||
| /// by the platform. | ||
| FlutterView? get implicitView => _implicitViewEnabled() ? _views[0] : null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will/should this always return the 0th view even in multi-window cases?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, if there is an implicit view it is guaranteed to have view ID "Modern" desktop apps won't have an implicit view. These are apps that use the new runner templates that don't exist yet. For these future apps, view ID |
||
|
|
||
| @Native<Handle Function()>(symbol: 'PlatformConfigurationNativeApi::ImplicitViewEnabled') | ||
| external static bool _implicitViewEnabled(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering if this maybe should just return the implicit view ID (or null) to avoid that we have to hard-code the magic number here and in platform_configuration.cc.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had this approach originally but I switched to a boolean instead as we don't have any scenarios where the implicit view will have a non-zero ID. I don't feel strongly either ways though, so let me know if you have a preference.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No preference, really. It's all private and internal anyways, so easy to change if it has to.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I'll leave this open so others can chime in :) |
||
|
|
||
| /// A callback that is invoked whenever the [ViewConfiguration] of any of the | ||
| /// [views] changes. | ||
| /// | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,13 @@ void testMain() { | |
| await window.resetHistory(); | ||
| }); | ||
|
|
||
| // For now, web always has an implicit view provided by the web engine. | ||
| test('EnginePlatformDispatcher.instance.implicitView should be non-null', () async { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my understanding, only desktop platforms will be able to create windows (and therefore views). All other platforms - mobile and web - will have an implicit view instead. In other words, this test will be correct for the foreseeable future. Side note: in theory we could allow all embedders to opt-out of the implicit view for add-to-app scenarios. We don't have any concrete plans there though... /cc @goderbauer
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think we'd rather not assume that (and we don't have to, right?). The user might very possibly attach multiple views from the native code, and the framework will handle them. And with that, the user might even choose not to use the implicit view.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Web will (possibly) want to render to multiple different target DIVs at some point, so I guess we're going to have multiple "views" too?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
To clarify there's two semi-related things here:
Having an implicit view does not block multi-view. It's just the replacement for the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the clarification @loic-sharma (expect a quick call from me soon-ish though, I want to start redoing stuff in the web engine so the multi-view lands more easily!) |
||
| expect(EnginePlatformDispatcher.instance.implicitView, isNotNull); | ||
| expect(EnginePlatformDispatcher.instance.implicitView?.viewId, 0); | ||
| expect(window.viewId, 0); | ||
| }); | ||
|
|
||
| test('window.defaultRouteName should work with JsUrlStrategy', () async { | ||
| dynamic state = <dynamic, dynamic>{}; | ||
| final JsUrlStrategy jsUrlStrategy = JsUrlStrategy( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,8 +45,15 @@ void executableNameNotNull() { | |
| notifyStringValue(Platform.executable); | ||
| } | ||
|
|
||
| @pragma('vm:entry-point') | ||
| void implicitViewNotNull() { | ||
| notifyBoolValue(PlatformDispatcher.instance.implicitView != null); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this test do? To ensure that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just a fixture to check whether the implicit view is non-null. The real test is
Yup that's correct.
Good idea, added a TODO to |
||
| } | ||
|
|
||
| @pragma('vm:external-name', 'NotifyStringValue') | ||
| external void notifyStringValue(String value); | ||
| @pragma('vm:external-name', 'NotifyBoolValue') | ||
| external void notifyBoolValue(bool value); | ||
|
|
||
| @pragma('vm:entry-point') | ||
| void invokePlatformTaskRunner() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.