Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions lib/features/workspace/result_grid_view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,20 @@ class _VirtualResultGridState extends material.State<VirtualResultGrid> {
ResultGridColumnWindow? _currentWindow;
double _currentRowsViewportHeight = 0.0;

/// Handlers that live in `extension _Grid* on _VirtualResultGridState`. Tearing
/// off an extension method makes a new closure each time and two of them are
/// never equal, so `removeListener(_onPiiMaskingChanged)` would not find what
/// `addListener(_onPiiMaskingChanged)` stored (#1555). Keep one closure and
/// pass that to both.
late final void Function() _piiMaskingListener = _onPiiMaskingChanged;
late final void Function() _horizontalScrollListener = _onHorizontalScroll;

@override
void initState() {
super.initState();
_horizontalController.addListener(_onHorizontalScroll);
_horizontalController.addListener(_horizontalScrollListener);
_verticalController.addListener(_onVerticalScroll);
PiiMaskingController.instance.addListener(_onPiiMaskingChanged);
PiiMaskingController.instance.addListener(_piiMaskingListener);
widget.stagingBuffer?.addListener(_onStagingBufferChanged);
_lastKnownStagedRowCount = _baseRows.length;
_updateSortedRows();
Expand Down Expand Up @@ -389,11 +397,11 @@ class _VirtualResultGridState extends material.State<VirtualResultGrid> {
void dispose() {
_hideCellTooltip();
_selectionValuesDebounce?.cancel();
PiiMaskingController.instance.removeListener(_onPiiMaskingChanged);
PiiMaskingController.instance.removeListener(_piiMaskingListener);
_stopAutoScroll();
_rowWidgets.clear();
widget.stagingBuffer?.removeListener(_onStagingBufferChanged);
_horizontalController.removeListener(_onHorizontalScroll);
_horizontalController.removeListener(_horizontalScrollListener);
_horizontalController.dispose();
_verticalController.removeListener(_onVerticalScroll);
_verticalController.dispose();
Expand Down
44 changes: 44 additions & 0 deletions test/features/workspace/result_grid_listener_leak_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import 'package:flutter/material.dart' as material;
import 'package:flutter_test/flutter_test.dart';
import 'package:querya_desktop/core/security/pii_masking_controller.dart';
import 'package:querya_desktop/features/workspace/result_grid_view.dart';

import '../../support/querya_theme_test_shell.dart';

material.Widget _shell({required material.Widget child}) {
return queryaThemeTestShell(child: material.Scaffold(body: child));
}

void main() {
group('VirtualResultGrid listeners (#1555)', () {
testWidgets('disposing a grid removes its listener from PiiMaskingController',
(tester) async {
// ignore: invalid_use_of_protected_member
final before = PiiMaskingController.instance.hasListeners;

await tester.pumpWidget(
_shell(
child: const material.SizedBox(
width: 800,
height: 400,
child: VirtualResultGrid(
columns: ['id', 'name'],
rows: [
['1', 'Alice'],
],
),
),
),
);
await tester.pumpAndSettle();
// ignore: invalid_use_of_protected_member
expect(PiiMaskingController.instance.hasListeners, isTrue);

await tester.pumpWidget(_shell(child: const material.SizedBox()));
await tester.pumpAndSettle();

// ignore: invalid_use_of_protected_member
expect(PiiMaskingController.instance.hasListeners, before);
});
});
}
58 changes: 58 additions & 0 deletions test/guards/no_extension_method_listeners_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import 'dart:io';

import 'package:flutter_test/flutter_test.dart';

/// #1555: an extension method torn off twice gives two different closures, so
/// `removeListener(_onX)` never removes what `addListener(_onX)` stored and the
/// listener keeps its owner alive. Handlers passed to `addListener` /
/// `removeListener` must be class members or a stored closure.
void main() {
test('addListener / removeListener never take an extension method', () {
final extensionBody = RegExp(
r'^extension\s+\w+(?:<[^>]*>)?\s+on\s+[^{]+\{',
multiLine: true,
);
final methodDecl = RegExp(
r'^ (?:[\w<>?,\s]+\s)?(_?\w+)\s*\([^)]*\)\s*(?:async\s*)?(?:\{|=>)',
multiLine: true,
);
final listenerArg = RegExp(r'\b(?:add|remove)Listener\(\s*(_?\w+)\s*[,)]');

final extensionMethods = <String>{};
final files = <File>[];
for (final f in Directory('lib').listSync(recursive: true)) {
if (f is! File || !f.path.endsWith('.dart')) continue;
files.add(f);
final src = f.readAsStringSync();
for (final m in extensionBody.allMatches(src)) {
// Extensions are declared at the top level; their members are indented
// by two spaces up to the first closing brace in column zero.
final end = src.indexOf('\n}', m.end);
final body = src.substring(m.end, end < 0 ? src.length : end);
for (final d in methodDecl.allMatches(body)) {
extensionMethods.add(d.group(1)!);
}
}
}

// Comments may quote the pattern they warn about.
final commentLine = RegExp(r'^\s*//.*$', multiLine: true);

final offenders = <String>[];
for (final f in files) {
final code = f.readAsStringSync().replaceAll(commentLine, '');
for (final m in listenerArg.allMatches(code)) {
final name = m.group(1)!;
if (extensionMethods.contains(name)) offenders.add('${f.path}: $name');
}
}

expect(
offenders,
isEmpty,
reason: 'Extension methods passed to addListener / removeListener: '
'$offenders. Declare them in the class or keep one closure in a '
'field.',
);
});
}
Loading