Unverified Commit 055c5489 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Fix KeySet<T> (and LogicalKeySet, PhysicalKeySet) hashCode calculation (#38936)

This fixes the hashCode calculation for KeySet<T> so that it doesn't depend on the insertion order of the keys in the set.

The fix involves switching from Set<T> to HashSet<T> internally, so that the iteration order is stable around the hash values of the inserted keys, and not the insertion order. This matters when hashList is called in KeySet<T>.hashCode to build the hash value of the contents of the internal set.

Fixes #38919
parent 01a5d112
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:collection';
import 'package:collection/collection.dart'; import 'package:collection/collection.dart';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
...@@ -39,7 +41,7 @@ class KeySet<T extends KeyboardKey> extends Diagnosticable { ...@@ -39,7 +41,7 @@ class KeySet<T extends KeyboardKey> extends Diagnosticable {
T key3, T key3,
T key4, T key4,
]) : assert(key1 != null), ]) : assert(key1 != null),
_keys = <T>{key1} { _keys = HashSet<T>()..add(key1) {
int count = 1; int count = 1;
if (key2 != null) { if (key2 != null) {
_keys.add(key2); _keys.add(key2);
...@@ -74,11 +76,14 @@ class KeySet<T extends KeyboardKey> extends Diagnosticable { ...@@ -74,11 +76,14 @@ class KeySet<T extends KeyboardKey> extends Diagnosticable {
: assert(keys != null), : assert(keys != null),
assert(keys.isNotEmpty), assert(keys.isNotEmpty),
assert(!keys.contains(null)), assert(!keys.contains(null)),
_keys = keys; _keys = HashSet<T>.from(keys);
/// Returns an unmodifiable view of the [KeyboardKey]s in this [KeySet]. /// Returns an unmodifiable view of the [KeyboardKey]s in this [KeySet].
Set<T> get keys => UnmodifiableSetView<T>(_keys); Set<T> get keys => UnmodifiableSetView<T>(_keys);
final Set<T> _keys; // This needs to be a hash set to be sure that the hashCode accessor returns
// consistent results. LinkedHashSet (the default Set implementation) depends
// upon insertion order, and HashSet does not.
final HashSet<T> _keys;
@override @override
bool operator ==(Object other) { bool operator ==(Object other) {
......
...@@ -167,8 +167,26 @@ void main() { ...@@ -167,8 +167,26 @@ void main() {
final LogicalKeySet set2 = LogicalKeySet( final LogicalKeySet set2 = LogicalKeySet(
LogicalKeyboardKey.keyA, LogicalKeyboardKey.keyA,
LogicalKeyboardKey.keyB, LogicalKeyboardKey.keyB,
LogicalKeyboardKey.keyC,
LogicalKeyboardKey.keyD,
);
final LogicalKeySet set3 = LogicalKeySet(
LogicalKeyboardKey.keyD,
LogicalKeyboardKey.keyC,
LogicalKeyboardKey.keyB,
LogicalKeyboardKey.keyA,
);
final LogicalKeySet set4 = LogicalKeySet.fromSet(<LogicalKeyboardKey>{
LogicalKeyboardKey.keyD,
LogicalKeyboardKey.keyC,
LogicalKeyboardKey.keyB,
LogicalKeyboardKey.keyA,}
); );
final Map<LogicalKeySet, String> map = <LogicalKeySet, String>{set1: 'one'}; final Map<LogicalKeySet, String> map = <LogicalKeySet, String>{set1: 'one'};
expect(set2 == set3, isTrue);
expect(set2 == set4, isTrue);
expect(set2.hashCode == set3.hashCode, isTrue);
expect(set2.hashCode == set4.hashCode, isTrue);
expect(map.containsKey(set1), isTrue); expect(map.containsKey(set1), isTrue);
expect(map.containsKey(LogicalKeySet(LogicalKeyboardKey.keyA)), isTrue); expect(map.containsKey(LogicalKeySet(LogicalKeyboardKey.keyA)), isTrue);
expect( expect(
...@@ -176,6 +194,8 @@ void main() { ...@@ -176,6 +194,8 @@ void main() {
equals(LogicalKeySet.fromSet(<LogicalKeyboardKey>{ equals(LogicalKeySet.fromSet(<LogicalKeyboardKey>{
LogicalKeyboardKey.keyA, LogicalKeyboardKey.keyA,
LogicalKeyboardKey.keyB, LogicalKeyboardKey.keyB,
LogicalKeyboardKey.keyC,
LogicalKeyboardKey.keyD,
}))); })));
}); });
test('$KeySet diagnostics work.', () { test('$KeySet diagnostics work.', () {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment