Deprecate single semantics tree assumption from platform dispatcher by a-wallen · Pull Request #36675 · flutter/engine

a-wallen · 2022-10-08T03:39:08Z

In an effort to move from a single-rooted flutter view to multiple views in a flutter app, dart::ui should be able to dispatch semantics tree updates to selected views. This PR deprecates semantics APIs that assume one flutter view per app.

This patch will be succeeded by framework changes that migrate from the deprecated APIs. After, the deprecated APIs will be removed in a subsequent PR.

This is a partial fix to flutter/flutter#112221. that issue will be fully fixed when all 3 PRs are landed.

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 Hixie said 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.

goderbauer · 2022-10-10T14:39:53Z

lib/ui/platform_dispatcher.dart

+  In a multi-view world, the platform dispatcher can no longer provide apis
+  to update semantics since each view will host its own semantics tree.
+
+  Semantics updates must be passed to an individual flutter view. To update


nit: "flutter view" -> FlutterView since its an actual class

goderbauer · 2022-10-10T14:40:16Z

lib/ui/window.dart

  @FfiNative<Void Function(Pointer<Void>)>('PlatformConfigurationNativeApi::Render')
  external static void _render(Scene scene);
+
+  /// Change the retained semantics data about this platform dispatcher.


nit: replace "platform dispatcher" with flutter view?

goderbauer · 2022-10-10T14:41:38Z

lib/ui/window.dart

+
+  /// Change the retained semantics data about this platform dispatcher.
+  ///
+  /// If [semanticsEnabled] is true, the user has requested that this function


nit" semanticsEnabled is a property on the PlatformDispatcher, not the view, right? So, this should probable change to [PlatformDispatcher.semanticsEnabled].

goderbauer · 2022-10-10T14:41:58Z

lib/ui/window.dart

+  /// Change the retained semantics data about this platform dispatcher.
+  ///
+  /// If [semanticsEnabled] is true, the user has requested that this function
+  /// be called whenever the semantic content of this platform dispatcher


platform dispatcher -> flutter view (or maybe just view)

goderbauer · 2022-10-10T14:42:50Z

lib/ui/window.dart

+  /// be called whenever the semantic content of this platform dispatcher
+  /// changes.
+  ///
+  /// In either case, this function disposes the given update, which means the


nit: Maybe remove "in either case" as its just empty prose?

goderbauer · 2022-10-10T14:45:19Z

lib/ui/window.dart

+  @override
+  @Deprecated('''
+  A singleton flutter window no longer manages semantics trees. In a multi-view
+  world, each flutter view must manage its own semantics tree.
+
+  Call updateSemantics() from FlutterView instead.
+  ''')
  void updateSemantics(SemanticsUpdate update) => platformDispatcher.updateSemantics(update);


"SingletonFlutterWindow" extends FlutterView, so updateSemantics continues to be available. Instead of deprecating it, I think you just want to remove this override altogether, so it uses FlutterView.updateSemantics from the base class instead of forwarding to the platform dispatcher.

goderbauer · 2022-10-10T14:46:52Z

lib/ui/platform_dispatcher.dart

  /// In either case, this function disposes the given update, which means the
  /// semantics update cannot be used further.
+  @Deprecated('''
+  In a multi-view world, the platform dispatcher can no longer provide apis


nit: indent these lines by two so they are indented more than the @Deprecated line.

(here and for all other @Deprecarted instances below as well.)

goderbauer · 2022-10-10T14:48:33Z

lib/web_ui/lib/window.dart

+  @override
+  @Deprecated('''
+  A singleton flutter window no longer manages semantics trees. In a multi-view
+  world, each flutter view must manage its own semantics tree.
+
+  Call updateSemantics() from FlutterView instead.
+  ''')
  void updateSemantics(SemanticsUpdate update) => platformDispatcher.updateSemantics(update);


same as for the other SingletonFlutterWindow, just remove this orverride.

goderbauer · 2022-10-10T14:51:55Z

testing/scenario_app/lib/src/locale_initialization.dart

+    for (final FlutterView view in dispatcher.views) {
+      view.updateSemantics(semanticsUpdate);
+    }


In the other tests you online dispatched it to the first view, here its going to all views? Why this difference?

FWIW, choosing the first one everywhere is probably good enough.

goderbauer

LGTM

goderbauer · 2022-10-10T17:06:31Z

lib/web_ui/lib/platform_dispatcher.dart

  set onAccessibilityFeaturesChanged(VoidCallback? callback);

+  @Deprecated('''
+  In a multi-view world, the platform dispatcher can no longer provide apis


nit: indent by two more

goderbauer · 2022-10-10T17:06:40Z

lib/web_ui/lib/src/engine/platform_dispatcher.dart

  /// semantics update cannot be used further.
  @override
+  @Deprecated('''
+  In a multi-view world, the platform dispatcher can no longer provide apis


nit: indentation

…atcher (flutter/engine#36675)

…atcher (#36675)" This reverts commit 7e03ebc.

flutter-dashboard bot added embedder Related to the embedder API platform-web Code specifically for the web engine labels Oct 8, 2022

a-wallen requested review from goderbauer and yjbanov October 8, 2022 03:39

a-wallen changed the title ~~Remove single semantics tree assumption from platform dispatcher~~ Deprecate single semantics tree assumption from platform dispatcher Oct 8, 2022

Deprecate single semantics tree assumption from platform dispatcher

bdcc158

a-wallen force-pushed the refactor_update_semantics branch from ccc5c13 to 033f9df Compare October 8, 2022 11:33

goderbauer reviewed Oct 10, 2022

View reviewed changes

a-wallen force-pushed the refactor_update_semantics branch 3 times, most recently from 3e93250 to 88c65fa Compare October 10, 2022 16:48

goderbauer approved these changes Oct 10, 2022

View reviewed changes

Refactor tests. Same logic, different calls.

b382bf6

a-wallen force-pushed the refactor_update_semantics branch from 88c65fa to b382bf6 Compare October 10, 2022 17:14

Indentation and deprecation message consistency

b61c913

a-wallen marked this pull request as ready for review October 10, 2022 17:32

a-wallen added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 10, 2022

auto-submit bot merged commit 7e03ebc into flutter:main Oct 10, 2022

engine-flutter-autoroll mentioned this pull request Oct 10, 2022

Roll Flutter Engine from 83c6a1c1d7be to 7e03ebcbee0d (3 revisions) flutter/flutter#113229

Closed

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

7e03ebc Deprecate single semantics tree assumption from platform disp…

5dc00c8

…atcher (flutter/engine#36675)

engine-flutter-autoroll mentioned this pull request Oct 10, 2022

Roll Flutter Engine from 83c6a1c1d7be to f87062cfc0bc (4 revisions) flutter/flutter#113233

Closed

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

7e03ebc Deprecate single semantics tree assumption from platform disp…

4b4a150

…atcher (flutter/engine#36675)

engine-flutter-autoroll mentioned this pull request Oct 10, 2022

Roll Flutter Engine from 83c6a1c1d7be to 1c67a217c1d2 (5 revisions) flutter/flutter#113236

Closed

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

7e03ebc Deprecate single semantics tree assumption from platform disp…

0b6b8b0

…atcher (flutter/engine#36675)

engine-flutter-autoroll mentioned this pull request Oct 10, 2022

Roll Flutter Engine from 83c6a1c1d7be to 915d521f48be (6 revisions) flutter/flutter#113239

Closed

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

7e03ebc Deprecate single semantics tree assumption from platform disp…

5d52f8f

…atcher (flutter/engine#36675)

engine-flutter-autoroll mentioned this pull request Oct 10, 2022

Roll Flutter Engine from 83c6a1c1d7be to 915d521f48be (6 revisions) flutter/flutter#113241

Closed

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

7e03ebc Deprecate single semantics tree assumption from platform disp…

6510c7f

…atcher (flutter/engine#36675)

engine-flutter-autoroll mentioned this pull request Oct 10, 2022

Roll Flutter Engine from 83c6a1c1d7be to 623d2d91802e (7 revisions) flutter/flutter#113243

Merged

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

7e03ebc Deprecate single semantics tree assumption from platform disp…

a82d163

…atcher (flutter/engine#36675)

zanderso added a commit that referenced this pull request Oct 10, 2022

Revert "Deprecate single semantics tree assumption from platform disp…

8b0186b

…atcher (#36675)" This reverts commit 7e03ebc.

zanderso mentioned this pull request Oct 10, 2022

Revert "Deprecate single semantics tree assumption from platform dispatcher" #36704

Closed

a-wallen mentioned this pull request Oct 13, 2022

Remove deprecated updateSemantics API usage. flutter/flutter#113382

Merged

8 tasks

a-wallen mentioned this pull request Nov 9, 2022

Remove deprecated updateSemantics APIs. #37396

Closed

8 tasks

Conversation

a-wallen commented Oct 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

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!

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!

goderbauer left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

a-wallen commented Oct 8, 2022 •

edited

Loading