Fix timer keeps active when resampling disabled in some cases by wangying3426 · Pull Request #97197 · flutter/flutter

wangying3426 · 2022-01-25T09:03:28Z

Fix timer still keeps active when resampling disabled.

The problem will occur like this:

A developer set resamplingEnabled to true.
When events received, a periodic timer for resampling will be started until all resamplers are finished. But if one event's timestamp is larger than current simple time, its resampler will keep resampling.
Then, the developer set resamplingEnabled to false, at this moment the periodic time still keeps active .

For example, flutter/engine#30422 shows a case that the timestamp of a fake event is much more than current sample time, so its resampler and the timer will keeps active for a very long time.

For the tests, an assert exception will occur and tests will failed if _timer?.cancel not be called.

Pending timers:
Timer (duration: 0:00:00.016667, periodic: true), created:
#0      new FakeTimer._ (package:fake_async/fake_async.dart:284:41)
#1      FakeAsync._createTimer (package:fake_async/fake_async.dart:248:27)
#2      FakeAsync.run.<anonymous closure> (package:fake_async/fake_async.dart:183:19)
#5      _Resampler.sample.<anonymous closure> (package:flutter/src/gestures/binding.dart:168:24)
#6      SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1145:15)
#7      SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1090:9)
#8      AutomatedTestWidgetsFlutterBinding.scheduleWarmUpFrame (package:flutter_test/src/binding.dart:1074:5)
#9      runApp (package:flutter/src/widgets/binding.dart:1037:7)
#10     TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:813:7)
<asynchronous suspension>
(elided 2 frames from dart:async)

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test:
A Timer is still pending even after the widget tree was disposed.
'package:flutter_test/src/binding.dart':
Failed assertion: line 1236 pos 12: '!timersPending'

When the exception was thrown, this was the stack:
#2      AutomatedTestWidgetsFlutterBinding._verifyInvariants (package:flutter_test/src/binding.dart:1236:12)
#3      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:821:7)
<asynchronous suspension>
(elided 2 frames from class _AssertionError)

The test description was:
  Timer should be canceled when resampling stopped
════════════════════════════════════════════════════════════════════════════════════════════════════
00:03 +1 -1: Timer should be canceled when resampling stopped [E]                                                                                                                                        
  Test failed. See exception logs above.
  The test description was: Timer should be canceled when resampling stopped
  
00:03 +1 -1: Some tests failed.

skia-gold · 2022-01-25T09:07:25Z

Gold has detected about 6 new digest(s) on patchset 1.
View them at https://flutter-gold.skia.org/cl/github/97197

skia-gold · 2022-01-25T11:57:25Z

Gold has detected about 41 new digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/97197

skia-gold · 2022-01-25T12:17:24Z

Gold has detected about 30 new digest(s) on patchset 3.
View them at https://flutter-gold.skia.org/cl/github/97197

skia-gold · 2022-01-25T12:47:24Z

Gold has detected about 49 new digest(s) on patchset 4.
View them at https://flutter-gold.skia.org/cl/github/97197

skia-gold · 2022-01-26T03:42:24Z

Gold has detected about 50 new digest(s) on patchset 5.
View them at https://flutter-gold.skia.org/cl/github/97197

skia-gold · 2022-01-26T04:02:24Z

Gold has detected about 37 new digest(s) on patchset 6.
View them at https://flutter-gold.skia.org/cl/github/97197

skia-gold · 2022-01-26T06:52:24Z

Gold has detected about 12 new digest(s) on patchset 7.
View them at https://flutter-gold.skia.org/cl/github/97197

skia-gold · 2022-01-26T07:57:24Z

Gold has detected about 49 new digest(s) on patchset 8.
View them at https://flutter-gold.skia.org/cl/github/97197

goderbauer · 2022-01-26T22:35:17Z

@wangying3426 Looks like this PR is addressing two different issues? Can you please split it in two so we can evaluate (and land) them separately?

skia-gold · 2022-01-27T11:02:25Z

Gold has detected about 24 new digest(s) on patchset 9.
View them at https://flutter-gold.skia.org/cl/github/97197

wangying3426 · 2022-01-27T11:27:42Z

@goderbauer I have modified this PR to focus only one issue and updated the first comments.

goderbauer

LGTM

goderbauer · 2022-02-02T23:28:09Z

@wangying3426 Can you rebase this to the latest master to make the checks pass, please?

…cel" This reverts commit ef44ba0, reversing changes made to 633dad3.

fluttergithubbot · 2022-02-09T08:40:18Z

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

Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

…flutter/flutter#97197)

wangying3426 · 2022-02-09T13:38:00Z

@goderbauer Could you please also review this related PR flutter/engine#30422 ？ thanks.

…flutter/flutter#97197)

goderbauer · 2022-02-09T17:25:43Z

@wangying3426 It would be best to have that one reviewed by an engine expert.

…flutter/flutter#97197)

…r#97197)

Fix timer may not be canceled when all resamplers stopped

c5332b2

flutter-dashboard bot added f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels. labels Jan 25, 2022

wangying3426 mentioned this pull request Jan 25, 2022

Fix timestamp of touch events should use system startup time flutter/engine#30422

Merged

8 tasks

set resamplingEnabled to false

ebab929

wangying3426 changed the title ~~Fix timer may not be canceled when all resamplers stopped~~ Fix timer keeps active when all resamplers stopped in some cases Jan 25, 2022

revert timer cancel for test

767f90f

wangying3426 changed the title ~~Fix timer keeps active when all resamplers stopped in some cases~~ Fix timer keeps active when resampling disabled in some cases Jan 25, 2022

fix timer keep active when resampling disabled sometimes.

58e348a

fix test exception

46b75a6

modify test format

93274f7

add test

5317682

fix

6292f92

goderbauer self-requested a review January 26, 2022 22:35

remove timestamp test

633dad3

goderbauer approved these changes Feb 2, 2022

View reviewed changes

wangying3426 and others added 2 commits February 7, 2022 11:29

Merge branch 'flutter:master' into fix_resamper_timer_not_cancel

ef44ba0

Revert "Merge branch 'flutter:master' into fix_resamper_timer_not_can…

7f6a28e

…cel" This reverts commit ef44ba0, reversing changes made to 633dad3.

christopherfujino removed their request for review February 7, 2022 18:18

wangying3426 force-pushed the fix_resamper_timer_not_cancel branch 2 times, most recently from 553618c to 2fbebfb Compare February 8, 2022 01:27

fix unnecessary_non_null_assertion

3c155a3

wangying3426 force-pushed the fix_resamper_timer_not_cancel branch from 2fbebfb to 3c155a3 Compare February 8, 2022 01:47

eggfly added the waiting for tree to go green label Feb 9, 2022

fluttergithubbot removed the waiting for tree to go green label Feb 9, 2022

eggfly approved these changes Feb 9, 2022

View reviewed changes

eggfly added the waiting for tree to go green label Feb 9, 2022

fluttergithubbot merged commit 499e900 into flutter:master Feb 9, 2022

engine-flutter-autoroll mentioned this pull request Feb 9, 2022

Roll Flutter from d2853dce6c14 to 499e900e3952 (2 revisions) flutter/packages#754

Closed

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

499e900 Fix timer keeps active when resampling disabled in some cases (…

3631cf9

…flutter/flutter#97197)

engine-flutter-autoroll mentioned this pull request Feb 9, 2022

Roll Flutter from f483b7d80831 to 499e900e3952 (45 revisions) flutter/plugins#4773

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 9, 2022

499e900 Fix timer keeps active when resampling disabled in some cases (…

5f79218

…flutter/flutter#97197)

engine-flutter-autoroll mentioned this pull request Feb 9, 2022

Roll Flutter from d2853dce6c14 to c54a27dc8790 (3 revisions) flutter/packages#755

Closed

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

499e900 Fix timer keeps active when resampling disabled in some cases (…

2c07d44

…flutter/flutter#97197)

engine-flutter-autoroll mentioned this pull request Feb 9, 2022

Roll Flutter from f483b7d80831 to c54a27dc8790 (46 revisions) flutter/plugins#4774

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 9, 2022

499e900 Fix timer keeps active when resampling disabled in some cases (…

86d0ee3

…flutter/flutter#97197)

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

499e900 Fix timer keeps active when resampling disabled in some cases (…

24c79f7

…flutter/flutter#97197)

This was referenced Feb 9, 2022

Roll Flutter from d2853dce6c14 to c54a27dc8790 (3 revisions) flutter/packages#756

Merged

Roll Flutter from f483b7d80831 to 312085a19a0f (47 revisions) flutter/plugins#4775

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 9, 2022

499e900 Fix timer keeps active when resampling disabled in some cases (…

7dc6917

…flutter/flutter#97197)

engine-flutter-autoroll mentioned this pull request Feb 9, 2022

Roll Flutter from f483b7d80831 to ca647d1e9a5d (48 revisions) flutter/plugins#4776

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 9, 2022

499e900 Fix timer keeps active when resampling disabled in some cases (…

9e88f3c

…flutter/flutter#97197)

engine-flutter-autoroll mentioned this pull request Feb 9, 2022

Roll Flutter from f483b7d80831 to bb3275b9340f (51 revisions) flutter/plugins#4778

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 9, 2022

499e900 Fix timer keeps active when resampling disabled in some cases (…

011b514

…flutter/flutter#97197)

clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Mar 8, 2022

Fix timer keeps active when resampling disabled in some cases (flutte…

611a249

…r#97197)

Conversation

wangying3426 commented Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skia-gold commented Jan 25, 2022

Uh oh!

skia-gold commented Jan 25, 2022

Uh oh!

skia-gold commented Jan 25, 2022

Uh oh!

skia-gold commented Jan 25, 2022

Uh oh!

skia-gold commented Jan 26, 2022

Uh oh!

skia-gold commented Jan 26, 2022

Uh oh!

skia-gold commented Jan 26, 2022

Uh oh!

skia-gold commented Jan 26, 2022

Uh oh!

goderbauer commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skia-gold commented Jan 27, 2022

Uh oh!

wangying3426 commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Feb 2, 2022

Uh oh!

fluttergithubbot commented Feb 9, 2022

Uh oh!

wangying3426 commented Feb 9, 2022

Uh oh!

goderbauer commented Feb 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wangying3426 commented Jan 25, 2022 •

edited

Loading

goderbauer commented Jan 26, 2022 •

edited

Loading

wangying3426 commented Jan 27, 2022 •

edited

Loading