From 055c548902c61de8152717029a384af283220b09 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 23 Aug 2019 07:51:35 -0700 Subject: [PATCH] Fix KeySet (and LogicalKeySet, PhysicalKeySet) hashCode calculation (#38936) This fixes the hashCode calculation for KeySet so that it doesn't depend on the insertion order of the keys in the set. The fix involves switching from Set to HashSet 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.hashCode to build the hash value of the contents of the internal set. Fixes #38919 --- .../flutter/lib/src/widgets/shortcuts.dart | 11 +++++++--- .../flutter/test/widgets/shortcuts_test.dart | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/flutter/lib/src/widgets/shortcuts.dart b/packages/flutter/lib/src/widgets/shortcuts.dart index 549f79bc2f..d8c3b35246 100644 --- a/packages/flutter/lib/src/widgets/shortcuts.dart +++ b/packages/flutter/lib/src/widgets/shortcuts.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:collection'; + import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; @@ -39,7 +41,7 @@ class KeySet extends Diagnosticable { T key3, T key4, ]) : assert(key1 != null), - _keys = {key1} { + _keys = HashSet()..add(key1) { int count = 1; if (key2 != null) { _keys.add(key2); @@ -74,11 +76,14 @@ class KeySet extends Diagnosticable { : assert(keys != null), assert(keys.isNotEmpty), assert(!keys.contains(null)), - _keys = keys; + _keys = HashSet.from(keys); /// Returns an unmodifiable view of the [KeyboardKey]s in this [KeySet]. Set get keys => UnmodifiableSetView(_keys); - final Set _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 _keys; @override bool operator ==(Object other) { diff --git a/packages/flutter/test/widgets/shortcuts_test.dart b/packages/flutter/test/widgets/shortcuts_test.dart index 930212f82c..075421c6eb 100644 --- a/packages/flutter/test/widgets/shortcuts_test.dart +++ b/packages/flutter/test/widgets/shortcuts_test.dart @@ -167,8 +167,26 @@ void main() { final LogicalKeySet set2 = LogicalKeySet( LogicalKeyboardKey.keyA, LogicalKeyboardKey.keyB, + LogicalKeyboardKey.keyC, + LogicalKeyboardKey.keyD, + ); + final LogicalKeySet set3 = LogicalKeySet( + LogicalKeyboardKey.keyD, + LogicalKeyboardKey.keyC, + LogicalKeyboardKey.keyB, + LogicalKeyboardKey.keyA, + ); + final LogicalKeySet set4 = LogicalKeySet.fromSet({ + LogicalKeyboardKey.keyD, + LogicalKeyboardKey.keyC, + LogicalKeyboardKey.keyB, + LogicalKeyboardKey.keyA,} ); final Map map = {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(LogicalKeySet(LogicalKeyboardKey.keyA)), isTrue); expect( @@ -176,6 +194,8 @@ void main() { equals(LogicalKeySet.fromSet({ LogicalKeyboardKey.keyA, LogicalKeyboardKey.keyB, + LogicalKeyboardKey.keyC, + LogicalKeyboardKey.keyD, }))); }); test('$KeySet diagnostics work.', () {