Revert selectable update back to be a postframecallback or microtask #125140

chunhtai · 2023-04-19T16:16:44Z

The regression was caused by the previous pr #124624 changes postframecallback to scheduleframecallback. The reason is that if a new postframecallback was scheduled when running a postframecallback. The newly added postframecallback will be execute on the next frame. However, adding postframecallback will not schedule a new frame. So if there isn't other widget that schedule a new frame, the newly added postframecallback will never gets run.

After changing to scheduleframecallback, it causes an issue that transient callback may be called when rendering tree contains dirty layout information that are waiting to be rebuilt.

Therefore, I use microtask to get around of the postframecallback issue instead of scheduleframecallback.

fixes #125065

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.

justinmc

LGTM 👍

Tricky! Could this sort of bug happen anywhere else that we're using scheduleFrameCallback?

Also there is a test failure, but I think it's unrelated?

justinmc · 2023-04-19T17:04:51Z

packages/flutter/test/widgets/selectable_region_test.dart

+            selectionControls: materialTextSelectionControls,
+            child: SelectionSpy(key: spy),
+          ),
+        ),


Thanks for cleaning this up.

chunhtai · 2023-04-19T17:08:04Z

the tree breakage is unrelated, #125144

justinmc · 2023-04-19T17:08:36Z

Ah thanks, I just saw that same failure on one of my PRs after I reviewed this.

chunhtai · 2023-04-19T17:10:16Z

I didn't find too many instance of shceduleframecallback. and i think they are fine as long as they are not doing widget/rendering tree related operations

justinmc · 2023-04-19T17:11:02Z

Sounds good, I'll keep that in mind in case it comes up again.

Renzo-Olivares

LGTM thanks for the explanation

…microtask (flutter/flutter#125140)

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Apr 19, 2023

chunhtai requested review from justinmc and Renzo-Olivares April 19, 2023 16:18

justinmc approved these changes Apr 19, 2023

View reviewed changes

Renzo-Olivares approved these changes Apr 19, 2023

View reviewed changes

Revert selectable update back to be a postframecallback or microtask

74ab73c

chunhtai force-pushed the issues/125065 branch from e6999fc to 74ab73c Compare April 19, 2023 17:33

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 19, 2023

auto-submit bot merged commit 14e191f into flutter:master Apr 19, 2023
71 checks passed

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter from 3476b966524f to 9f2e7088310d (19 revisions) flutter/packages#3771

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

26ec6d2

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter from 3476b966524f to 64591356b968 (20 revisions) flutter/packages#3773

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

9dcf635

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter from 3476b966524f to 70ca469792ce (21 revisions) flutter/packages#3775

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

8075eb8

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter from 3476b966524f to 0d023144d993 (25 revisions) flutter/packages#3777

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

c63490b

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter from 3476b966524f to 13f4fe409158 (26 revisions) flutter/packages#3778

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

9653339

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter from 3476b966524f to 5764404f5c97 (27 revisions) flutter/packages#3779

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

6d9e862

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter from 3476b966524f to d85e2fb810b6 (28 revisions) flutter/packages#3781

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

cbe039b

…microtask (flutter/flutter#125140)

This was referenced Apr 20, 2023

i123636 android embedding v2 agp gradle kotlin update #125256

Closed

[integration_test] add support for agp8.0 #125310

Closed

engine-flutter-autoroll mentioned this pull request Apr 21, 2023

Roll Flutter from 3476b966524f to 8784eb1d8e30 (46 revisions) flutter/packages#3788

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

0a78e81

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 21, 2023

Roll Flutter from 3476b966524f to c9004ff822cf (47 revisions) flutter/packages#3790

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

c776297

…microtask (flutter/flutter#125140)

reidbaker mentioned this pull request Apr 21, 2023

Test AGP 8.0 using java 17 #125323

Merged

8 tasks

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

14e191fa7 Revert selectable update back to be a postframecallback or …

a8438b8

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

891cb70

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

96bafef

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

ee5a69a

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

b666816

…microtask (flutter/flutter#125140)

engine-flutter-autoroll mentioned this pull request Apr 24, 2023

Manual roll Flutter from 3476b966524f to d85e2fb810b6 (28 revisions) flutter/packages#3807

Closed

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

14e191fa7 Revert selectable update back to be a postframecallback or …

98e0154

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

702571c

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

90493a6

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

1ef439b

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

cc3e715

…microtask (flutter/flutter#125140)

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

14e191fa7 Revert selectable update back to be a postframecallback or …

6b1e258

…microtask (flutter/flutter#125140)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert selectable update back to be a postframecallback or microtask #125140

Revert selectable update back to be a postframecallback or microtask #125140

chunhtai commented Apr 19, 2023 •

edited

justinmc left a comment

justinmc Apr 19, 2023

chunhtai commented Apr 19, 2023

justinmc commented Apr 19, 2023

chunhtai commented Apr 19, 2023

justinmc commented Apr 19, 2023

Renzo-Olivares left a comment

Revert selectable update back to be a postframecallback or microtask #125140

Revert selectable update back to be a postframecallback or microtask #125140

Conversation

chunhtai commented Apr 19, 2023 • edited

Pre-launch Checklist

justinmc left a comment

Choose a reason for hiding this comment

justinmc Apr 19, 2023

Choose a reason for hiding this comment

chunhtai commented Apr 19, 2023

justinmc commented Apr 19, 2023

chunhtai commented Apr 19, 2023

justinmc commented Apr 19, 2023

Renzo-Olivares left a comment

Choose a reason for hiding this comment

chunhtai commented Apr 19, 2023 •

edited