Reland VelocityTracker update (again) #138843

Piinks · 2023-11-21T21:05:08Z

This updates the implementation to use the stopwatch from the Clock object and piping it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync.

Relands #137381 which attempted to reland #132291
Fixes #97761

The original change was reverted due to flakiness it introduced in tests that use fling gestures.

[web] persistent_bottom_sheet_test.dart is infrequently flaky #135728

It was reverted again due to a change in the leak tracking tests that broke it.

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 this 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.

chunhtai

Since this is a reland, RSLGTM

chunhtai · 2023-11-21T21:38:45Z

packages/flutter/lib/src/gestures/binding.dart

+  ///
+  /// This value is ignored in non-debug builds.
+  @protected
+  SamplingClock? get debugSamplingClock => null;


This should probably be visibleForTesting

chunhtai · 2023-11-21T21:46:16Z

packages/flutter/lib/src/gestures/velocity_tracker.dart

  /// Returns null if there is no data on which to base an estimate.
  VelocityEstimate? getVelocityEstimate() {
+    // Has user recently moved since last sample?
+    if (_sinceLastSample.elapsedMilliseconds > VelocityTracker._assumePointerMoveStoppedMilliseconds) {


Do we have style guide around using static member in the same class? I think in other class we don't include the class name

The analyzer required it

chunhtai · 2023-11-21T21:46:50Z

packages/flutter/lib/src/gestures/velocity_tracker.dart

      final double delta = (sample.time - previousSample.time).inMicroseconds.abs().toDouble() / 1000;
      previousSample = sample;
-      if (age > _horizonMilliseconds || delta > _assumePointerMoveStoppedMilliseconds) {
+      if (age > _horizonMilliseconds || delta > VelocityTracker._assumePointerMoveStoppedMilliseconds) {


same as the styling question above, either both _horizonMilliseconds and _assumePointerMoveStoppedMilliseconds prefixed with class name or none

yjbanov · 2023-11-21T21:48:52Z

packages/flutter/test/rendering/editable_gesture_test.dart


-  test('attach and detach correctly handle gesture', () {
+  testWidgets('attach and detach correctly handle gesture', (_) async {
+    assert(WidgetsBinding.instance == binding);


nit: in tests it is more idiomatic to use expect instead of assert, but either will do the job.

This reverts commit 3e4e280.

Reverts #138843 This has broken the Mac_ios microbenchmarks_ios. Example failure: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20microbenchmarks_ios/9339/overview

Manual roll requested by [email protected] flutter/flutter@ab721f9...14549b3 2023-11-22 [email protected] make FakeView not send Scene and semantics to the engine (flutter/flutter#138849) 2023-11-22 [email protected] Update the default outline color for `OutlinedButton` (flutter/flutter#138768) 2023-11-22 [email protected] Roll Flutter Engine from c954a64c509b to 342fb003b11f (1 revision) (flutter/flutter#138903) 2023-11-22 [email protected] Roll Packages from c9933fc to 2102327 (3 revisions) (flutter/flutter#138895) 2023-11-22 [email protected] Marks Mac_build_test flutter_gallery__transition_perf_e2e_ios to be unflaky (flutter/flutter#138886) 2023-11-22 [email protected] Roll Flutter Engine from d55b673f895c to c954a64c509b (1 revision) (flutter/flutter#138894) 2023-11-22 [email protected] Add `commandHasTerminal` parameter + apple usage event + `sendException` events for `package:unified_analytics` (flutter/flutter#138806) 2023-11-22 [email protected] Roll Flutter Engine from 6c8f7b566a22 to d55b673f895c (1 revision) (flutter/flutter#138890) 2023-11-22 [email protected] Roll Flutter Engine from dda2499df48a to 6c8f7b566a22 (1 revision) (flutter/flutter#138875) 2023-11-22 [email protected] Roll Flutter Engine from 1ae1d5318257 to dda2499df48a (4 revisions) (flutter/flutter#138872) 2023-11-22 [email protected] Roll Flutter Engine from 7cf9d90d59e1 to 1ae1d5318257 (8 revisions) (flutter/flutter#138861) 2023-11-22 [email protected] Revert "Reland VelocityTracker update (again)" (flutter/flutter#138863) 2023-11-21 [email protected] In `flutter doctor -v`, when JRE is too out-of-date to run `sdkmanager`, print a helpful error message (flutter/flutter#138762) 2023-11-21 [email protected] Roll Flutter Engine from a8e9b8dd562b to 7cf9d90d59e1 (1 revision) (flutter/flutter#138847) 2023-11-21 [email protected] Roll Flutter Engine from 90d1ff04ae02 to a8e9b8dd562b (1 revision) (flutter/flutter#138846) 2023-11-21 [email protected] Reland VelocityTracker update (again) (flutter/flutter#138843) 2023-11-21 [email protected] Roll Flutter Engine from 31799868224d to 90d1ff04ae02 (2 revisions) (flutter/flutter#138844) 2023-11-21 [email protected] Fix M3 Tabs Specs links (flutter/flutter#138808) 2023-11-21 [email protected] Roll Flutter Engine from 0c2de1e8849c to 31799868224d (1 revision) (flutter/flutter#138840) 2023-11-21 [email protected] Roll Flutter Engine from 746697c27569 to 0c2de1e8849c (1 revision) (flutter/flutter#138834) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This updates the implementation to use the stopwatch from the Clock object and pipes it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands #138843 attempted to reland #137381 which attempted to reland #132291 Fixes #97761 1. The original change was reverted due to flakiness it introduced in tests that use fling gestures. * Using a mocked clock through the test binding fixes this now 2. It was reverted a second time because a change at tip of tree broke it, exposing memory leaks, but it was not rebased before landing. * These leaks are now fixed 3. It was reverted a third time, because we were so excellently quick to revert those other times, that we did not notice the broken benchmark that only runs in postsubmit. * The benchmark is now fixed

This updates the implementation to use the stopwatch from the Clock object and piping it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands flutter#137381 which attempted to reland flutter#132291 Fixes flutter#97761 The original change was reverted due to flakiness it introduced in tests that use fling gestures. * flutter#135728 It was reverted again due to a change in the leak tracking tests that broke it.

Reverts flutter#138843 This has broken the Mac_ios microbenchmarks_ios. Example failure: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20microbenchmarks_ios/9339/overview

This updates the implementation to use the stopwatch from the Clock object and pipes it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync. Relands flutter#138843 attempted to reland flutter#137381 which attempted to reland flutter#132291 Fixes flutter#97761 1. The original change was reverted due to flakiness it introduced in tests that use fling gestures. * Using a mocked clock through the test binding fixes this now 2. It was reverted a second time because a change at tip of tree broke it, exposing memory leaks, but it was not rebased before landing. * These leaks are now fixed 3. It was reverted a third time, because we were so excellently quick to revert those other times, that we did not notice the broken benchmark that only runs in postsubmit. * The benchmark is now fixed

Once more with feeling

3e492fe

github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. f: gestures flutter/packages/flutter/gestures repository. labels Nov 21, 2023

Piinks requested a review from yjbanov November 21, 2023 21:11

chunhtai approved these changes Nov 21, 2023

View reviewed changes

yjbanov approved these changes Nov 21, 2023

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2023

auto-submit bot merged commit 3e4e280 into flutter:master Nov 21, 2023

eyebrowsoffire added a commit that referenced this pull request Nov 22, 2023

Revert "Reland VelocityTracker update (again) (#138843)"

98a96d1

This reverts commit 3e4e280.

eyebrowsoffire mentioned this pull request Nov 22, 2023

Revert "Reland VelocityTracker update (again)" #138863

Merged

engine-flutter-autoroll mentioned this pull request Nov 22, 2023

Roll Flutter from ab721f935ffc to 48187028c11c (14 revisions) flutter/packages#5462

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2023

3e4e280 Reland VelocityTracker update (again) (flutter/flutter#138843)

b5ccc0a

engine-flutter-autoroll mentioned this pull request Nov 22, 2023

Roll Flutter from ab721f935ffc to 35a53b0b1adb (15 revisions) flutter/packages#5463

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2023

3e4e280 Reland VelocityTracker update (again) (flutter/flutter#138843)

f720259

engine-flutter-autoroll mentioned this pull request Nov 22, 2023

Roll Flutter from ab721f935ffc to 721ee5749211 (17 revisions) flutter/packages#5464

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2023

3e4e280 Reland VelocityTracker update (again) (flutter/flutter#138843)

3fe5026

engine-flutter-autoroll mentioned this pull request Nov 22, 2023

Roll Flutter from ab721f935ffc to 843074c5bddd (18 revisions) flutter/packages#5465

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2023

3e4e280 Reland VelocityTracker update (again) (flutter/flutter#138843)

e018404

engine-flutter-autoroll mentioned this pull request Nov 22, 2023

Manual roll Flutter from ab721f935ffc to 14549b38891f (20 revisions) flutter/packages#5466

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2023

3e4e280 Reland VelocityTracker update (again) (flutter/flutter#138843)

289eadb

engine-flutter-autoroll mentioned this pull request Nov 22, 2023

Manual roll Flutter from ab721f935ffc to 106667eb4bc1 (23 revisions) flutter/packages#5467

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2023

3e4e280 Reland VelocityTracker update (again) (flutter/flutter#138843)

acdc50f

Piinks mentioned this pull request Nov 28, 2023

Update VelocityTracker (4) #139166

Merged

8 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024

3e4e280 Reland VelocityTracker update (again) (flutter/flutter#138843)

1378c26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reland VelocityTracker update (again) #138843

Reland VelocityTracker update (again) #138843

Uh oh!

Piinks commented Nov 21, 2023 •

edited

Loading

Uh oh!

chunhtai left a comment

Uh oh!

chunhtai Nov 21, 2023

Uh oh!

chunhtai Nov 21, 2023

Uh oh!

Piinks Nov 21, 2023

Uh oh!

chunhtai Nov 21, 2023

Uh oh!

yjbanov Nov 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Reland VelocityTracker update (again) #138843

Reland VelocityTracker update (again) #138843

Uh oh!

Conversation

Piinks commented Nov 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai Nov 21, 2023

Choose a reason for hiding this comment

Uh oh!

chunhtai Nov 21, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks Nov 21, 2023

Choose a reason for hiding this comment

Uh oh!

chunhtai Nov 21, 2023

Choose a reason for hiding this comment

Uh oh!

yjbanov Nov 21, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Piinks commented Nov 21, 2023 •

edited

Loading