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

Conversation

@ankitdave06
Copy link
Contributor

This cl does following:

  • Replaces SemanticActionListener with SemanticListener.
  • Replaces Commit with CommitUpdates.
  • Replaces RegisterView with RegisterViewForSemantics.
  • Not changes fakes since they are not present in engine yet.

Testing:

  • Build Passed

BUG:35718

This cl does following:
* Replaces SemanticActionListener with SemanticListener.
* Replaces Commit with CommitUpdates.
* Replaces RegisterView with RegisterViewForSemantics.
* Not changes fakes since they are not present in engine yet.

Testing:
  * Build Passed

BUG:35718
dnfield
dnfield previously approved these changes Sep 16, 2019
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM.


tree_ptr_->UpdateSemanticNodes(std::move(nodes));
tree_ptr_->Commit();
// TODO(35718): Implement the callback here.
Copy link
Contributor

Choose a reason for hiding this comment

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

For Flutter style, TODOs should be:

// TODO(username): Implement callback here https://link-to-bug.

But why do we have a todo here? What needs to be implemented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If Semantics manager sees any error while applying a commit, it will call this callback.
Semantics manager will also close the channel for this view. Flutter should re-register this view.
For more information look at semantics_manager.fidl

@dnfield dnfield dismissed their stale review September 16, 2019 23:22

Answer about todo first

This cl does following:
* Replaces SemanticActionListener with SemanticListener.
* Replaces Commit with CommitUpdates.
* Replaces RegisterView with RegisterViewForSemantics.
* Not changes fakes since they are not present in engine yet.

Testing:
  * Build Passed

BUG:35718
@ankitdave06
Copy link
Contributor Author

Resolved the comment.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM

@ankitdave06
Copy link
Contributor Author

I ran the command to format the code and uploaded another patch.

@dnfield dnfield merged commit 3b09d9f into flutter:master Sep 17, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 18, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Sep 18, 2019
[email protected]:flutter/engine.git/compare/6a96417416b4...36be89d

git log 6a96417..36be89d --no-merges --oneline
2019-09-17 [email protected] Added javadoc comments to FlutterActivity and FlutterFragmentActivity. (flutter/engine#12328)
2019-09-17 [email protected] Roll src/third_party/dart 7505b3a5f0..f22f62c85d (5 commits)
2019-09-17 [email protected] Revert "Provide dart vm initalize isolate callback so that children isolates belong to parent's isolate group. (#9888)" (flutter/engine#12327)
2019-09-17 [email protected] [flutter] Remove old A11y API's. (flutter/engine#12308)


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
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
[email protected]:flutter/engine.git/compare/6a96417416b4...36be89d

git log 6a96417..36be89d --no-merges --oneline
2019-09-17 [email protected] Added javadoc comments to FlutterActivity and FlutterFragmentActivity. (flutter/engine#12328)
2019-09-17 [email protected] Roll src/third_party/dart 7505b3a5f0..f22f62c85d (5 commits)
2019-09-17 [email protected] Revert "Provide dart vm initalize isolate callback so that children isolates belong to parent's isolate group. (flutter#9888)" (flutter/engine#12327)
2019-09-17 [email protected] [flutter] Remove old A11y API's. (flutter/engine#12308)


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
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants