Skip to content

Commit 2603055

Browse files
goderbauergoderbauer
authored
Fix semantic debugger (#147953)
This was broken in flutter/flutter#122452. The culprit is that `PipelineOwner.ensureSemantics` doesn't turn on semantics for the entire app, it pretends to only turn it on for the local `PipelineOwner`. Unfortunately, that local `PipelineOwner` is never informed that it should produce semantics when semantics are not turned on globally. So, `PipelineOwner.ensureSemantics` is essentially without effect if semantics are not already turned on globally. I can't think of a use case where it would be useful to only turn on semantics for a particular pipeline owner and fixing `PipelineOwner.ensureSemantics` would get pretty messy with performance implications even if semantics are turned off. So, this PR deprecates that functionality and moves the `SemanticsDebugger` to the global semantics API. Fixes flutter/flutter#147665.
1 parent 9e2d945 commit 2603055

File tree

8 files changed

+82
-117
lines changed

8 files changed

+82
-117
lines changed

‎packages/flutter/lib/src/rendering/object.dart‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,30 +1219,29 @@ class PipelineOwner with DiagnosticableTreeMixin {
12191219
SemanticsOwner? get semanticsOwner => _semanticsOwner;
12201220
SemanticsOwner? _semanticsOwner;
12211221

1222-
/// The number of clients registered to listen for semantics.
1223-
///
1224-
/// The number is increased whenever [ensureSemantics] is called and decreased
1225-
/// when [SemanticsHandle.dispose] is called.
1222+
/// Deprecated.
1223+
///
1224+
/// Use [SemanticsBinding.debugOutstandingSemanticsHandles] instead. This
1225+
/// API is broken because an outstanding semantics handle on a given pipeline
1226+
/// owner doesn't mean that semantics are actually produced.
1227+
@Deprecated(
1228+
'Use SemanticsBinding.debugOutstandingSemanticsHandles instead. '
1229+
'This API is broken (see ensureSemantics). '
1230+
'This feature was deprecated after v3.22.0-23.0.pre.'
1231+
)
12261232
int get debugOutstandingSemanticsHandles => _outstandingSemanticsHandles;
12271233
int _outstandingSemanticsHandles = 0;
12281234

1229-
/// Opens a [SemanticsHandle] and calls [listener] whenever the semantics tree
1230-
/// generated from the render tree owned by this [PipelineOwner] updates.
1231-
///
1232-
/// Calling this method only ensures that this particular [PipelineOwner] will
1233-
/// generate a semantics tree. Consider calling
1234-
/// [SemanticsBinding.ensureSemantics] instead to turn on semantics globally
1235-
/// for the entire app.
1236-
///
1237-
/// The [PipelineOwner] updates the semantics tree only when there are clients
1238-
/// that wish to use the semantics tree. These clients express their interest
1239-
/// by holding [SemanticsHandle] objects that notify them whenever the
1240-
/// semantics tree updates.
1235+
/// Deprecated.
12411236
///
1242-
/// Clients can close their [SemanticsHandle] by calling
1243-
/// [SemanticsHandle.dispose]. Once all the outstanding [SemanticsHandle]
1244-
/// objects for a given [PipelineOwner] are closed, the [PipelineOwner] stops
1245-
/// maintaining the semantics tree.
1237+
/// Call [SemanticsBinding.ensureSemantics] instead and optionally add a
1238+
/// listener to [PipelineOwner.semanticsOwner]. This API is broken as calling
1239+
/// it does not guarantee that semantics are produced.
1240+
@Deprecated(
1241+
'Call SemanticsBinding.ensureSemantics instead and optionally add a listener to PipelineOwner.semanticsOwner. '
1242+
'This API is broken; it does not guarantee that semantics are actually produced. '
1243+
'This feature was deprecated after v3.22.0-23.0.pre.'
1244+
)
12461245
SemanticsHandle ensureSemantics({ VoidCallback? listener }) {
12471246
_outstandingSemanticsHandles += 1;
12481247
_updateSemanticsOwner();

‎packages/flutter/lib/src/widgets/semantics_debugger.dart‎

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -45,31 +45,33 @@ class SemanticsDebugger extends StatefulWidget {
4545
}
4646

4747
class _SemanticsDebuggerState extends State<SemanticsDebugger> with WidgetsBindingObserver {
48-
_SemanticsClient? _client;
4948
PipelineOwner? _pipelineOwner;
49+
SemanticsHandle? _semanticsHandle;
50+
int _generation = 0;
5051

5152
@override
5253
void initState() {
5354
super.initState();
55+
_semanticsHandle = SemanticsBinding.instance.ensureSemantics();
5456
WidgetsBinding.instance.addObserver(this);
5557
}
5658

57-
@override
5859
@override
5960
void didChangeDependencies() {
6061
super.didChangeDependencies();
6162
final PipelineOwner newOwner = View.pipelineOwnerOf(context);
63+
assert(newOwner.semanticsOwner != null);
6264
if (newOwner != _pipelineOwner) {
63-
_client?.dispose();
64-
_client = _SemanticsClient(newOwner)
65-
..addListener(_update);
65+
_pipelineOwner?.semanticsOwner?.removeListener(_update);
66+
newOwner.semanticsOwner!.addListener(_update);
6667
_pipelineOwner = newOwner;
6768
}
6869
}
6970

7071
@override
7172
void dispose() {
72-
_client?.dispose();
73+
_pipelineOwner?.semanticsOwner?.removeListener(_update);
74+
_semanticsHandle?.dispose();
7375
WidgetsBinding.instance.removeObserver(this);
7476
super.dispose();
7577
}
@@ -82,6 +84,7 @@ class _SemanticsDebuggerState extends State<SemanticsDebugger> with WidgetsBindi
8284
}
8385

8486
void _update() {
87+
_generation++;
8588
SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) {
8689
// Semantic information are only available at the end of a frame and our
8790
// only chance to paint them on the screen is the next frame. To achieve
@@ -157,7 +160,7 @@ class _SemanticsDebuggerState extends State<SemanticsDebugger> with WidgetsBindi
157160
return CustomPaint(
158161
foregroundPainter: _SemanticsDebuggerPainter(
159162
_pipelineOwner!,
160-
_client!.generation,
163+
_generation,
161164
_lastPointerDownLocation, // in physical pixels
162165
View.of(context).devicePixelRatio,
163166
widget.labelStyle,
@@ -180,42 +183,6 @@ class _SemanticsDebuggerState extends State<SemanticsDebugger> with WidgetsBindi
180183
}
181184
}
182185

183-
class _SemanticsClient extends ChangeNotifier {
184-
_SemanticsClient(PipelineOwner pipelineOwner) {
185-
// TODO(polina-c): stop duplicating code across disposables
186-
// https://github.com/flutter/flutter/issues/137435
187-
if (kFlutterMemoryAllocationsEnabled) {
188-
FlutterMemoryAllocations.instance.dispatchObjectCreated(
189-
library: 'package:flutter/widgets.dart',
190-
className: '$_SemanticsClient',
191-
object: this,
192-
);
193-
}
194-
_semanticsHandle = pipelineOwner.ensureSemantics(
195-
listener: _didUpdateSemantics,
196-
);
197-
}
198-
199-
SemanticsHandle? _semanticsHandle;
200-
201-
@override
202-
void dispose() {
203-
if (kFlutterMemoryAllocationsEnabled) {
204-
FlutterMemoryAllocations.instance.dispatchObjectDisposed(object: this);
205-
}
206-
_semanticsHandle!.dispose();
207-
_semanticsHandle = null;
208-
super.dispose();
209-
}
210-
211-
int generation = 0;
212-
213-
void _didUpdateSemantics() {
214-
generation += 1;
215-
notifyListeners();
216-
}
217-
}
218-
219186
class _SemanticsDebuggerPainter extends CustomPainter {
220187
const _SemanticsDebuggerPainter(this.owner, this.generation, this.pointerPosition, this.devicePixelRatio, this.labelStyle);
221188

‎packages/flutter/test/rendering/paragraph_test.dart‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,11 +1415,10 @@ void main() {
14151415
);
14161416

14171417
int semanticsUpdateCount = 0;
1418-
TestRenderingFlutterBinding.instance.pipelineOwner.ensureSemantics(
1419-
listener: () {
1420-
++semanticsUpdateCount;
1421-
},
1422-
);
1418+
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.ensureSemantics();
1419+
TestRenderingFlutterBinding.instance.pipelineOwner.semanticsOwner!.addListener(() {
1420+
++semanticsUpdateCount;
1421+
});
14231422

14241423
layout(paragraph);
14251424

@@ -1453,6 +1452,8 @@ void main() {
14531452
data = children.single.getSemanticsData();
14541453
expect(data.hasAction(SemanticsAction.longPress), true);
14551454
expect(data.hasAction(SemanticsAction.tap), false);
1455+
1456+
semanticsHandle.dispose();
14561457
});
14571458
}
14581459

‎packages/flutter/test/rendering/platform_view_test.dart‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,10 @@ void main() {
4747
child: platformViewRenderBox,
4848
);
4949
int semanticsUpdateCount = 0;
50-
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.rootPipelineOwner.ensureSemantics(
51-
listener: () {
52-
++semanticsUpdateCount;
53-
},
54-
);
50+
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.ensureSemantics();
51+
TestRenderingFlutterBinding.instance.pipelineOwner.semanticsOwner!.addListener(() {
52+
++semanticsUpdateCount;
53+
});
5554
layout(tree, phase: EnginePhase.flushSemantics);
5655
// Initial semantics update
5756
expect(semanticsUpdateCount, 1);

‎packages/flutter/test/rendering/reattach_test.dart‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,10 @@ void main() {
135135
test('objects can be detached and re-attached: semantics (no change)', () {
136136
final TestTree testTree = TestTree();
137137
int semanticsUpdateCount = 0;
138-
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.pipelineOwner.ensureSemantics(
139-
listener: () {
140-
++semanticsUpdateCount;
141-
},
142-
);
138+
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.ensureSemantics();
139+
TestRenderingFlutterBinding.instance.pipelineOwner.semanticsOwner!.addListener(() {
140+
++semanticsUpdateCount;
141+
});
143142
// Lay out, composite, paint, and update semantics
144143
layout(testTree.root, phase: EnginePhase.flushSemantics);
145144
expect(semanticsUpdateCount, 1);
@@ -158,11 +157,10 @@ void main() {
158157
test('objects can be detached and re-attached: semantics (with change)', () {
159158
final TestTree testTree = TestTree();
160159
int semanticsUpdateCount = 0;
161-
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.pipelineOwner.ensureSemantics(
162-
listener: () {
163-
++semanticsUpdateCount;
164-
},
165-
);
160+
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.ensureSemantics();
161+
TestRenderingFlutterBinding.instance.pipelineOwner.semanticsOwner!.addListener(() {
162+
++semanticsUpdateCount;
163+
});
166164
// Lay out, composite, paint, and update semantics
167165
layout(testTree.root, phase: EnginePhase.flushSemantics);
168166
expect(semanticsUpdateCount, 1);

‎packages/flutter/test/rendering/simple_semantics_test.dart‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ void main() {
2020
child: testRender,
2121
);
2222
int semanticsUpdateCount = 0;
23-
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.pipelineOwner.ensureSemantics(
24-
listener: () {
23+
final SemanticsHandle semanticsHandle = TestRenderingFlutterBinding.instance.ensureSemantics();
24+
TestRenderingFlutterBinding.instance.pipelineOwner.semanticsOwner!.addListener(
25+
() {
2526
++semanticsUpdateCount;
2627
},
2728
);

‎packages/flutter/test/widgets/semantics_debugger_test.dart‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,22 @@ void main() {
545545

546546
expect(_getMessageShownInSemanticsDebugger(widgetKey: label, debuggerKey: debugger, tester: tester), '\u2067ملصق\u2069');
547547
});
548+
549+
testWidgets('SemanticsDebugger turns on semantics.', (WidgetTester tester) async {
550+
// Regression test for https://github.com/flutter/flutter/issues/147665.
551+
expect(tester.binding.semanticsEnabled, isFalse);
552+
await tester.pumpWidget(
553+
Directionality(
554+
textDirection: TextDirection.rtl,
555+
child: SemanticsDebugger(
556+
child: Semantics(
557+
label: 'Hello World',
558+
),
559+
),
560+
),
561+
);
562+
expect(tester.binding.semanticsEnabled, isTrue);
563+
}, semanticsEnabled: false);
548564
}
549565

550566
String _getMessageShownInSemanticsDebugger({

‎packages/flutter/test/widgets/semantics_test.dart‎

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -713,11 +713,9 @@ void main() {
713713
testWidgets('Actions can be replaced without triggering semantics update', (WidgetTester tester) async {
714714
final SemanticsTester semantics = SemanticsTester(tester);
715715
int semanticsUpdateCount = 0;
716-
final SemanticsHandle handle = tester.binding.pipelineOwner.ensureSemantics(
717-
listener: () {
718-
semanticsUpdateCount += 1;
719-
},
720-
);
716+
tester.binding.pipelineOwner.semanticsOwner!.addListener(() {
717+
semanticsUpdateCount += 1;
718+
});
721719

722720
final List<String> performedActions = <String>[];
723721

@@ -803,7 +801,6 @@ void main() {
803801
expect(semantics, hasSemantics(expectedSemantics));
804802
expect(semanticsUpdateCount, 1);
805803

806-
handle.dispose();
807804
semantics.dispose();
808805
});
809806

@@ -887,11 +884,9 @@ void main() {
887884
testWidgets('Semantics widgets built in a widget tree are sorted properly', (WidgetTester tester) async {
888885
final SemanticsTester semantics = SemanticsTester(tester);
889886
int semanticsUpdateCount = 0;
890-
final SemanticsHandle handle = tester.binding.pipelineOwner.ensureSemantics(
891-
listener: () {
892-
semanticsUpdateCount += 1;
893-
},
894-
);
887+
tester.binding.pipelineOwner.semanticsOwner!.addListener(() {
888+
semanticsUpdateCount += 1;
889+
});
895890
await tester.pumpWidget(
896891
Directionality(
897892
textDirection: TextDirection.ltr,
@@ -963,18 +958,15 @@ void main() {
963958
ignoreRect: true,
964959
));
965960

966-
handle.dispose();
967961
semantics.dispose();
968962
});
969963

970964
testWidgets('Semantics widgets built with explicit sort orders are sorted properly', (WidgetTester tester) async {
971965
final SemanticsTester semantics = SemanticsTester(tester);
972966
int semanticsUpdateCount = 0;
973-
final SemanticsHandle handle = tester.binding.pipelineOwner.ensureSemantics(
974-
listener: () {
975-
semanticsUpdateCount += 1;
976-
},
977-
);
967+
tester.binding.pipelineOwner.semanticsOwner!.addListener(() {
968+
semanticsUpdateCount += 1;
969+
});
978970
await tester.pumpWidget(
979971
Directionality(
980972
textDirection: TextDirection.ltr,
@@ -1021,18 +1013,15 @@ void main() {
10211013
ignoreRect: true,
10221014
));
10231015

1024-
handle.dispose();
10251016
semantics.dispose();
10261017
});
10271018

10281019
testWidgets('Semantics widgets without sort orders are sorted properly', (WidgetTester tester) async {
10291020
final SemanticsTester semantics = SemanticsTester(tester);
10301021
int semanticsUpdateCount = 0;
1031-
final SemanticsHandle handle = tester.binding.pipelineOwner.ensureSemantics(
1032-
listener: () {
1033-
semanticsUpdateCount += 1;
1034-
},
1035-
);
1022+
tester.binding.pipelineOwner.semanticsOwner!.addListener(() {
1023+
semanticsUpdateCount += 1;
1024+
});
10361025
await tester.pumpWidget(
10371026
const Directionality(
10381027
textDirection: TextDirection.ltr,
@@ -1082,18 +1071,15 @@ void main() {
10821071
ignoreId: true,
10831072
));
10841073

1085-
handle.dispose();
10861074
semantics.dispose();
10871075
});
10881076

10891077
testWidgets('Semantics widgets that are transformed are sorted properly', (WidgetTester tester) async {
10901078
final SemanticsTester semantics = SemanticsTester(tester);
10911079
int semanticsUpdateCount = 0;
1092-
final SemanticsHandle handle = tester.binding.pipelineOwner.ensureSemantics(
1093-
listener: () {
1094-
semanticsUpdateCount += 1;
1095-
},
1096-
);
1080+
tester.binding.pipelineOwner.semanticsOwner!.addListener(() {
1081+
semanticsUpdateCount += 1;
1082+
});
10971083
await tester.pumpWidget(
10981084
Directionality(
10991085
textDirection: TextDirection.ltr,
@@ -1146,14 +1132,13 @@ void main() {
11461132
ignoreId: true,
11471133
));
11481134

1149-
handle.dispose();
11501135
semantics.dispose();
11511136
});
11521137

11531138
testWidgets('Semantics widgets without sort orders are sorted properly when no Directionality is present', (WidgetTester tester) async {
11541139
final SemanticsTester semantics = SemanticsTester(tester);
11551140
int semanticsUpdateCount = 0;
1156-
final SemanticsHandle handle = tester.binding.pipelineOwner.ensureSemantics(listener: () {
1141+
tester.binding.pipelineOwner.semanticsOwner!.addListener(() {
11571142
semanticsUpdateCount += 1;
11581143
});
11591144
await tester.pumpWidget(
@@ -1247,7 +1232,6 @@ void main() {
12471232
),
12481233
);
12491234

1250-
handle.dispose();
12511235
semantics.dispose();
12521236
});
12531237

0 commit comments

Comments
 (0)