Skip to content

Clean up the bindings APIs#86438

Merged
fluttergithubbot merged 5 commits intoflutter:masterfrom
Hixie:binding4
Jul 14, 2021
Merged

Clean up the bindings APIs#86438
fluttergithubbot merged 5 commits intoflutter:masterfrom
Hixie:binding4

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented Jul 14, 2021

  • Add more documentation.
  • Make premature binding accesses have better runtime errors.
  • Make accesses to instance not required !.
  • Rationalize our "ensureInitialized" etc logic.

Fixes #64830.

History: This was first landed in #83843 (which was reverted in #86386), then landed in #86388 (which was reverted in #86388). Relative to the last time this landed, this adds some defensive code in timeDilation's setter to make it not depend on the scheduler binding being initialized.

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, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests d: examples Sample code and demos f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Jul 14, 2021
@google-cla google-cla bot added the cla: yes label Jul 14, 2021
@Hixie
Copy link
Contributor Author

Hixie commented Jul 14, 2021

cc @goderbauer for review

Gardeners: In case this causes any devicelab failures, please do not hesitate to revert it. Thanks!

goderbauer
goderbauer approved these changes Jul 14, 2021
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

packages/flutter/lib/src/scheduler/binding.dart Outdated
Copy link
Member

@goderbauer goderbauer Jul 14, 2021

Choose a reason for hiding this comment

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

Maybe add a unit test to cover this case?

Copy link
Contributor Author

@Hixie Hixie Jul 14, 2021

Choose a reason for hiding this comment

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

done

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jul 14, 2021

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac module_test_ios has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite analyze-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

Hixie added 5 commits July 14, 2021 13:39
@Hixie
Clean up the bindings APIs.
22bca9f 
* Add more documentation.
* Make premature binding accesses have better runtime errors.
* Make accesses to `instance` not required `!`.
* Rationalize our "ensureInitialized" etc logic.
@Hixie
dart fix --apply
3d7123b
@Hixie
Review comments
a19e638
@Hixie
Remove now-bogus assert from Driver code
3c7919a
@Hixie
Allow timeDilation to be called before the binding is initialized
854ecf1
@fluttergithubbot fluttergithubbot merged commit d056500 into flutter:master Jul 14, 2021
@Hixie Hixie deleted the binding4 branch July 14, 2021 22:57
renyou added a commit that referenced this pull request Jul 15, 2021
@renyou
Revert "Clean up the bindings APIs (#86438)"
d5921f3 
This reverts commit d056500.
@renyou renyou mentioned this pull request Jul 15, 2021
Merged
renyou added a commit that referenced this pull request Jul 15, 2021
@renyou
Revert "Clean up the bindings APIs (#86438)" (#86484)
3dea9f0 
This reverts commit d056500.
@Hixie Hixie mentioned this pull request Oct 8, 2021
Merged
8 tasks
Hixie added a commit to Hixie/flutter that referenced this pull request Feb 3, 2022
@Hixie
Clean up the bindings APIs.
5910971 
* Add more documentation.
* Make premature binding accesses have better runtime errors.
* Make accesses to `instance` not required `!`.
* Rationalize our "ensureInitialized" etc logic.

Fixes flutter#64830.

Most of these changes were done using `dart fix --apply`, but they have been squashed into one commit for easier merging.
To see previous reviews of this PR, see
flutter#83843,
flutter#86388,
flutter#86438, and
flutter#89451.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer approved these changes

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests c: contributor-productivity Team-specific productivity, code health, technical debt. d: examples Sample code and demos f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Clean up Binding.instance

3 participants

@Hixie @fluttergithubbot @goderbauer