Skip to content

Commit 01c2866

Browse files
authored
Return more eagerly when toggling service extensions (#162774)
Fixes flutter/devtools#8497 When handling requests to enable or disable service extensions, we currently wait until the next frame is rendered before responding to the request. If the app is hidden this will not happen until the app is unhidden, or a frame is scheduled for some other reason. This fix does not await the next frame before responding - which I think is reasonable - but feel free to push back if there is a reason to not do it that way. Updated tests to expect the response before actually rendering the next frame. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent c484d14 commit 01c2866

File tree

2 files changed

+40
-27
lines changed

2 files changed

+40
-27
lines changed

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

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
/// @docImport 'layer.dart';
1010
library;
1111

12+
import 'dart:async';
1213
import 'dart:ui' as ui show PictureRecorder, SceneBuilder, SemanticsUpdate;
1314

1415
import 'package:flutter/foundation.dart';
@@ -85,43 +86,49 @@ mixin RendererBinding
8586
setter: (bool value) async {
8687
if (debugInvertOversizedImages != value) {
8788
debugInvertOversizedImages = value;
88-
return _forceRepaint();
89+
// We don't want to block the vm service response on the frame
90+
// actually rendering, just schedule it and return;
91+
unawaited(_forceRepaint());
8992
}
90-
return Future<void>.value();
9193
},
9294
);
9395
registerBoolServiceExtension(
9496
name: RenderingServiceExtensions.debugPaint.name,
9597
getter: () async => debugPaintSizeEnabled,
96-
setter: (bool value) {
98+
setter: (bool value) async {
9799
if (debugPaintSizeEnabled == value) {
98-
return Future<void>.value();
100+
return;
99101
}
100102
debugPaintSizeEnabled = value;
101-
return _forceRepaint();
103+
// We don't want to block the vm service response on the frame
104+
// actually rendering, just schedule it and return;
105+
unawaited(_forceRepaint());
102106
},
103107
);
104108
registerBoolServiceExtension(
105109
name: RenderingServiceExtensions.debugPaintBaselinesEnabled.name,
106110
getter: () async => debugPaintBaselinesEnabled,
107-
setter: (bool value) {
111+
setter: (bool value) async {
108112
if (debugPaintBaselinesEnabled == value) {
109-
return Future<void>.value();
113+
return;
110114
}
111115
debugPaintBaselinesEnabled = value;
112-
return _forceRepaint();
116+
// We don't want to block the vm service response on the frame
117+
// actually rendering, just schedule it and return;
118+
unawaited(_forceRepaint());
113119
},
114120
);
115121
registerBoolServiceExtension(
116122
name: RenderingServiceExtensions.repaintRainbow.name,
117123
getter: () async => debugRepaintRainbowEnabled,
118-
setter: (bool value) {
124+
setter: (bool value) async {
119125
final bool repaint = debugRepaintRainbowEnabled && !value;
120126
debugRepaintRainbowEnabled = value;
121127
if (repaint) {
122-
return _forceRepaint();
128+
// We don't want to block the vm service response on the frame
129+
// actually rendering, just schedule it and return;
130+
unawaited(_forceRepaint());
123131
}
124-
return Future<void>.value();
125132
},
126133
);
127134
registerServiceExtension(
@@ -133,34 +140,40 @@ mixin RendererBinding
133140
registerBoolServiceExtension(
134141
name: RenderingServiceExtensions.debugDisableClipLayers.name,
135142
getter: () async => debugDisableClipLayers,
136-
setter: (bool value) {
143+
setter: (bool value) async {
137144
if (debugDisableClipLayers == value) {
138-
return Future<void>.value();
145+
return;
139146
}
140147
debugDisableClipLayers = value;
141-
return _forceRepaint();
148+
// We don't want to block the vm service response on the frame
149+
// actually rendering, just schedule it and return;
150+
unawaited(_forceRepaint());
142151
},
143152
);
144153
registerBoolServiceExtension(
145154
name: RenderingServiceExtensions.debugDisablePhysicalShapeLayers.name,
146155
getter: () async => debugDisablePhysicalShapeLayers,
147-
setter: (bool value) {
156+
setter: (bool value) async {
148157
if (debugDisablePhysicalShapeLayers == value) {
149-
return Future<void>.value();
158+
return;
150159
}
151160
debugDisablePhysicalShapeLayers = value;
152-
return _forceRepaint();
161+
// We don't want to block the vm service response on the frame
162+
// actually rendering, just schedule it and return;
163+
unawaited(_forceRepaint());
153164
},
154165
);
155166
registerBoolServiceExtension(
156167
name: RenderingServiceExtensions.debugDisableOpacityLayers.name,
157168
getter: () async => debugDisableOpacityLayers,
158-
setter: (bool value) {
169+
setter: (bool value) async {
159170
if (debugDisableOpacityLayers == value) {
160-
return Future<void>.value();
171+
return;
161172
}
162173
debugDisableOpacityLayers = value;
163-
return _forceRepaint();
174+
// We don't want to block the vm service response on the frame
175+
// actually rendering, just schedule it and return;
176+
unawaited(_forceRepaint());
164177
},
165178
);
166179
return true;

packages/flutter/test/foundation/service_extensions_test.dart

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ void main() {
413413
});
414414
await binding.flushMicrotasks();
415415
expect(binding.frameScheduled, isTrue);
416-
expect(completed, isFalse);
416+
expect(completed, isTrue);
417417
await binding.doFrame();
418418
await binding.flushMicrotasks();
419419
expect(completed, isTrue);
@@ -484,7 +484,7 @@ void main() {
484484
});
485485
await binding.flushMicrotasks();
486486
expect(binding.frameScheduled, isTrue);
487-
expect(completed, isFalse);
487+
expect(completed, isTrue);
488488
await binding.doFrame();
489489
await binding.flushMicrotasks();
490490
expect(completed, isTrue);
@@ -546,7 +546,7 @@ void main() {
546546
});
547547
await binding.flushMicrotasks();
548548
expect(binding.frameScheduled, isTrue);
549-
expect(completed, isFalse);
549+
expect(completed, isTrue);
550550
await binding.doFrame();
551551
await binding.flushMicrotasks();
552552
expect(completed, isTrue);
@@ -1038,7 +1038,7 @@ void main() {
10381038
completed = true;
10391039
});
10401040
await binding.flushMicrotasks();
1041-
expect(completed, false);
1041+
expect(completed, isTrue);
10421042
expect(binding.frameScheduled, isTrue);
10431043
await binding.doFrame();
10441044
await binding.flushMicrotasks();
@@ -1082,7 +1082,7 @@ void main() {
10821082
});
10831083
await binding.flushMicrotasks();
10841084
expect(binding.frameScheduled, isTrue);
1085-
expect(completed, isFalse);
1085+
expect(completed, isTrue);
10861086
await binding.doFrame();
10871087
await binding.flushMicrotasks();
10881088
expect(completed, isTrue);
@@ -1143,7 +1143,7 @@ void main() {
11431143
});
11441144
await binding.flushMicrotasks();
11451145
expect(binding.frameScheduled, isTrue);
1146-
expect(completed, isFalse);
1146+
expect(completed, isTrue);
11471147
await binding.doFrame();
11481148
await binding.flushMicrotasks();
11491149
expect(completed, isTrue);
@@ -1204,7 +1204,7 @@ void main() {
12041204
});
12051205
await binding.flushMicrotasks();
12061206
expect(binding.frameScheduled, isTrue);
1207-
expect(completed, isFalse);
1207+
expect(completed, isTrue);
12081208
await binding.doFrame();
12091209
await binding.flushMicrotasks();
12101210
expect(completed, isTrue);

0 commit comments

Comments
 (0)