Re-Land of HighlightMode change with benchmark improvements. (#39589)

* Automatic focus highlight mode for FocusManager

This re-lands the highlight mode change.

* Review Changes

packages/flutter/lib/src/foundation/observer_list.dart

@@ -4,10 +4,24 @@
 
 import 'dart:collection';
 
-/// A list optimized for containment queries.
+/// A list optimized for the observer pattern when there are small numbers of
+/// observers.
 ///
 /// Consider using an [ObserverList] instead of a [List] when the number of
 /// [contains] calls dominates the number of [add] and [remove] calls.
+///
+/// This class will include in the [iterator] each added item in the order it
+/// was added, as many times as it was added.
+///
+/// If there will be a large number of observers, consider using
+/// [HashedObserverList] instead. It has slightly different iteration semantics,
+/// but serves a similar purpose, while being more efficient for large numbers
+/// of observers.
+///
+/// See also:
+///
+///  * [HashedObserverList] for a list that is optimized for larger numbers of
+///    observers.
 // TODO(ianh): Use DelegatingIterable, possibly moving it from the collection
 // package to foundation, or to dart:collection.
 class ObserverList<T> extends Iterable<T> {
@@ -16,6 +30,8 @@ class ObserverList<T> extends Iterable<T> {
   HashSet<T> _set;
 
   /// Adds an item to the end of this list.
+  ///
+  /// This operation has constant time complexity.
   void add(T item) {
     _isDirty = true;
     _list.add(item);
@@ -28,6 +44,7 @@ class ObserverList<T> extends Iterable<T> {
   /// Returns whether the item was present in the list.
   bool remove(T item) {
     _isDirty = true;
+    _set?.clear(); // Clear the set so that we don't leak items.
     return _list.remove(item);
   }
 
@@ -40,7 +57,6 @@ class ObserverList<T> extends Iterable<T> {
       if (_set == null) {
         _set = HashSet<T>.from(_list);
       } else {
-        _set.clear();
         _set.addAll(_list);
       }
       _isDirty = false;
@@ -58,3 +74,58 @@ class ObserverList<T> extends Iterable<T> {
   @override
   bool get isNotEmpty => _list.isNotEmpty;
 }
+
+/// A list optimized for the observer pattern, but for larger numbers of observers.
+///
+/// For small numbers of observers (e.g. less than 10), use [ObserverList] instead.
+///
+/// The iteration semantics of the this class are slightly different from
+/// [ObserverList]. This class will only return an item once in the [iterator],
+/// no matter how many times it was added, although it does require that an item
+/// be removed as many times as it was added for it to stop appearing in the
+/// [iterator]. It will return them in the order the first instance of an item
+/// was originally added.
+///
+/// See also:
+///
+///  * [ObserverList] for a list that is fast for small numbers of observers.
+class HashedObserverList<T> extends Iterable<T> {
+  final LinkedHashMap<T, int> _map = LinkedHashMap<T, int>();
+
+  /// Adds an item to the end of this list.
+  ///
+  /// This has constant time complexity.
+  void add(T item) {
+    _map[item] = (_map[item] ?? 0) + 1;
+  }
+
+  /// Removes an item from the list.
+  ///
+  /// This operation has constant time complexity.
+  ///
+  /// Returns whether the item was present in the list.
+  bool remove(T item) {
+    final int value = _map[item];
+    if (value == null) {
+      return false;
+    }
+    if (value == 1) {
+      _map.remove(item);
+    } else {
+      _map[item] = value - 1;
+    }
+    return true;
+  }
+
+  @override
+  bool contains(Object element) => _map.containsKey(element);
+
+  @override
+  Iterator<T> get iterator => _map.keys.iterator;
+
+  @override
+  bool get isEmpty => _map.isEmpty;
+
+  @override
+  bool get isNotEmpty => _map.isNotEmpty;
+}

packages/flutter/lib/src/material/ink_well.dart

@@ -472,6 +472,12 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
 
   bool get highlightsExist => _highlights.values.where((InkHighlight highlight) => highlight != null).isNotEmpty;
 
+  @override
+  void initState() {
+    super.initState();
+    WidgetsBinding.instance.focusManager.addHighlightModeListener(_handleFocusHighlightModeChange);
+  }
+
   @override
   void didChangeDependencies() {
     super.didChangeDependencies();
@@ -491,6 +497,7 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
 
   @override
   void dispose() {
+    WidgetsBinding.instance.focusManager.removeHighlightModeListener(_handleFocusHighlightModeChange);
     _focusNode?.removeListener(_handleFocusUpdate);
     super.dispose();
   }
@@ -608,8 +615,25 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
     return splash;
   }
 
+  void _handleFocusHighlightModeChange(FocusHighlightMode mode) {
+    if (!mounted) {
+      return;
+    }
+    setState(() {
+      _handleFocusUpdate();
+    });
+  }
+
   void _handleFocusUpdate() {
-    final bool showFocus = enabled && (Focus.of(context, nullOk: true)?.hasPrimaryFocus ?? false);
+    bool showFocus;
+    switch (WidgetsBinding.instance.focusManager.highlightMode) {
+      case FocusHighlightMode.touch:
+        showFocus = false;
+        break;
+      case FocusHighlightMode.traditional:
+        showFocus = enabled && (Focus.of(context, nullOk: true)?.hasPrimaryFocus ?? false);
+        break;
+    }
     updateHighlight(_HighlightType.focus, value: showFocus);
   }
 

packages/flutter/lib/src/widgets/focus_manager.dart

@@ -3,9 +3,11 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:io';
 import 'dart:ui';
 
 import 'package:flutter/foundation.dart';
+import 'package:flutter/gestures.dart';
 import 'package:flutter/painting.dart';
 import 'package:flutter/services.dart';
 
@@ -552,6 +554,9 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
   /// This object notifies its listeners whenever this value changes.
   bool get hasPrimaryFocus => _manager?.primaryFocus == this;
 
+  /// Returns the [FocusHighlightMode] that is currently in effect for this node.
+  FocusHighlightMode get highlightMode => WidgetsBinding.instance.focusManager.highlightMode;
+
   /// Returns the nearest enclosing scope node above this node, including
   /// this node, if it's a scope.
   ///
@@ -994,6 +999,40 @@ class FocusScopeNode extends FocusNode {
   }
 }
 
+/// An enum to describe which kind of focus highlight behavior to use when
+/// displaying focus information.
+enum FocusHighlightMode {
+  /// Touch interfaces will not show the focus highlight except for controls
+  /// which bring up the soft keyboard.
+  ///
+  /// If a device that uses a traditional mouse and keyboard has a touch screen
+  /// attached, it can also enter `touch` mode if the user is using the touch
+  /// screen.
+  touch,
+
+  /// Traditional interfaces (keyboard and mouse) will show the currently
+  /// focused control via a focus highlight of some sort.
+  ///
+  /// If a touch device (like a mobile phone) has a keyboard and/or mouse
+  /// attached, it also can enter `traditional` mode if the user is using these
+  /// input devices.
+  traditional,
+}
+
+/// An enum to describe how the current value of [FocusManager.highlightMode] is
+/// determined. The strategy is set on [FocusManager.highlightStrategy].
+enum FocusHighlightStrategy {
+  /// Automatic switches between the various highlight modes based on the last
+  /// kind of input that was received. This is the default.
+  automatic,
+
+  /// [FocusManager.highlightMode] always returns [FocusHighlightMode.touch].
+  alwaysTouch,
+
+  /// [FocusManager.highlightMode] always returns [FocusHighlightMode.traditional].
+  alwaysTraditional,
+}
+
 /// Manages the focus tree.
 ///
 /// The focus tree keeps track of which [FocusNode] is the user's current
@@ -1032,6 +1071,125 @@ class FocusManager with DiagnosticableTreeMixin {
   FocusManager() {
     rootScope._manager = this;
     RawKeyboard.instance.addListener(_handleRawKeyEvent);
+    GestureBinding.instance.pointerRouter.addGlobalRoute(_handlePointerEvent);
+  }
+
+  bool _lastInteractionWasTouch = true;
+
+  /// Sets the strategy by which [highlightMode] is determined.
+  ///
+  /// If set to [FocusHighlightStrategy.automatic], then the highlight mode will
+  /// change depending upon the interaction mode used last. For instance, if the
+  /// last interaction was a touch interaction, then [highlightMode] will return
+  /// [FocusHighlightMode.touch], and focus highlights will only appear on
+  /// widgets that bring up a soft keyboard. If the last interaction was a
+  /// non-touch interaction (hardware keyboard press, mouse click, etc.), then
+  /// [highlightMode] will return [FocusHighlightMode.traditional], and focus
+  /// highlights will appear on all widgets.
+  ///
+  /// If set to [FocusHighlightStrategy.alwaysTouch] or
+  /// [FocusHighlightStrategy.alwaysTraditional], then [highlightMode] will
+  /// always return [FocusHighlightMode.touch] or
+  /// [FocusHighlightMode.traditional], respectively, regardless of the last UI
+  /// interaction type.
+  ///
+  /// The initial value of [highlightMode] depends upon the value of
+  /// [defaultTargetPlatform] and
+  /// [RendererBinding.instance.mouseTracker.mouseIsConnected], making a guess
+  /// about which interaction is most appropriate for the initial interaction
+  /// mode.
+  ///
+  /// Defaults to [FocusHighlightStrategy.automatic].
+  FocusHighlightStrategy get highlightStrategy => _highlightStrategy;
+  FocusHighlightStrategy _highlightStrategy = FocusHighlightStrategy.automatic;
+  set highlightStrategy(FocusHighlightStrategy highlightStrategy) {
+    _highlightStrategy = highlightStrategy;
+    _updateHighlightMode();
+  }
+
+  /// Indicates the current interaction mode for focus highlights.
+  ///
+  /// The value returned depends upon the [highlightStrategy] used, and possibly
+  /// (depending on the value of [highlightStrategy]) the most recent
+  /// interaction mode that they user used.
+  ///
+  /// If [highlightMode] returns [FocusHighlightMode.touch], then widgets should
+  /// not draw their focus highlight unless they perform text entry.
+  ///
+  /// If [highlightMode] returns [FocusHighlightMode.traditional], then widgets should
+  /// draw their focus highlight whenever they are focused.
+  FocusHighlightMode get highlightMode => _highlightMode;
+  FocusHighlightMode _highlightMode = FocusHighlightMode.touch;
+
+  // Update function to be called whenever the state relating to highlightMode
+  // changes.
+  void _updateHighlightMode() {
+    // Assume that if we're on one of these mobile platforms, or if there's no
+    // mouse connected, that the initial interaction will be touch-based, and
+    // that it's traditional mouse and keyboard on all others.
+    //
+    // This only affects the initial value: the ongoing value is updated as soon
+    // as any input events are received.
+    _lastInteractionWasTouch ??= Platform.isAndroid || Platform.isIOS || !WidgetsBinding.instance.mouseTracker.mouseIsConnected;
+    FocusHighlightMode newMode;
+    switch (highlightStrategy) {
+      case FocusHighlightStrategy.automatic:
+        if (_lastInteractionWasTouch) {
+          newMode = FocusHighlightMode.touch;
+        } else {
+          newMode = FocusHighlightMode.traditional;
+        }
+        break;
+      case FocusHighlightStrategy.alwaysTouch:
+        newMode = FocusHighlightMode.touch;
+        break;
+      case FocusHighlightStrategy.alwaysTraditional:
+        newMode = FocusHighlightMode.traditional;
+        break;
+    }
+    if (newMode != _highlightMode) {
+      _highlightMode = newMode;
+      _notifyHighlightModeListeners();
+    }
+  }
+
+  // The list of listeners for [highlightMode] state changes.
+  final HashedObserverList<ValueChanged<FocusHighlightMode>> _listeners = HashedObserverList<ValueChanged<FocusHighlightMode>>();
+
+  /// Register a closure to be called when the [FocusManager] notifies its listeners
+  /// that the value of [highlightMode] has changed.
+  void addHighlightModeListener(ValueChanged<FocusHighlightMode> listener) => _listeners.add(listener);
+
+  /// Remove a previously registered closure from the list of closures that the
+  /// [FocusManager] notifies.
+  void removeHighlightModeListener(ValueChanged<FocusHighlightMode> listener) => _listeners?.remove(listener);
+
+  void _notifyHighlightModeListeners() {
+    if (_listeners.isEmpty) {
+      return;
+    }
+    final List<ValueChanged<FocusHighlightMode>> localListeners = List<ValueChanged<FocusHighlightMode>>.from(_listeners);
+    for (ValueChanged<FocusHighlightMode> listener in localListeners) {
+      try {
+        if (_listeners.contains(listener)) {
+          listener(_highlightMode);
+        }
+      } catch (exception, stack) {
+        FlutterError.reportError(FlutterErrorDetails(
+          exception: exception,
+          stack: stack,
+          library: 'widgets library',
+          context: ErrorDescription('while dispatching notifications for $runtimeType'),
+          informationCollector: () sync* {
+            yield DiagnosticsProperty<FocusManager>(
+              'The $runtimeType sending notification was',
+              this,
+              style: DiagnosticsTreeStyle.errorProperty,
+            );
+          },
+        ));
+      }
+    }
   }
 
   /// The root [FocusScopeNode] in the focus tree.
@@ -1040,7 +1198,33 @@ class FocusManager with DiagnosticableTreeMixin {
   /// for a given [FocusNode], call [FocusNode.nearestScope].
   final FocusScopeNode rootScope = FocusScopeNode(debugLabel: 'Root Focus Scope');
 
+  void _handlePointerEvent(PointerEvent event) {
+    bool currentInteractionIsTouch;
+    switch (event.kind) {
+      case PointerDeviceKind.touch:
+      case PointerDeviceKind.stylus:
+      case PointerDeviceKind.invertedStylus:
+        currentInteractionIsTouch = true;
+        break;
+      case PointerDeviceKind.mouse:
+      case PointerDeviceKind.unknown:
+        currentInteractionIsTouch = false;
+        break;
+    }
+    if (_lastInteractionWasTouch != currentInteractionIsTouch) {
+      _lastInteractionWasTouch = currentInteractionIsTouch;
+      _updateHighlightMode();
+    }
+  }
+
   void _handleRawKeyEvent(RawKeyEvent event) {
+    // Update highlightMode first, since things responding to the keys might
+    // look at the highlight mode, and it should be accurate.
+    if (_lastInteractionWasTouch) {
+      _lastInteractionWasTouch = false;
+      _updateHighlightMode();
+    }
+
     // Walk the current focus from the leaf to the root, calling each one's
     // onKey on the way up, and if one responds that they handled it, stop.
     if (_primaryFocus == null) {

packages/flutter/test/foundation/observer_list_test.dart

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/foundation.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  test('ObserverList', () async {
+    final ObserverList<int> list = ObserverList<int>();
+    for (int i = 0; i < 10; ++i) {
+      list.add(i);
+    }
+    final Iterator<int> iterator = list.iterator;
+    for (int i = 0; i < 10 && iterator.moveNext(); ++i) {
+      expect(iterator.current, equals(i));
+    }
+    for (int i = 9; i >= 0; --i) {
+      expect(list.remove(i), isTrue);
+      final Iterator<int> iterator = list.iterator;
+      for (int j = 0; j < i && iterator.moveNext(); ++j) {
+        expect(iterator.current, equals(j));
+      }
+    }
+  });
+  test('HashedObserverList', () async {
+    final HashedObserverList<int> list = HashedObserverList<int>();
+    for (int i = 0; i < 10; ++i) {
+      list.add(i);
+    }
+    Iterator<int> iterator = list.iterator;
+    for (int i = 0; i < 10 && iterator.moveNext(); ++i) {
+      expect(iterator.current, equals(i));
+    }
+    for (int i = 9; i >= 0; --i) {
+      expect(list.remove(i), isTrue);
+      iterator = list.iterator;
+      for (int j = 0; j < i && iterator.moveNext(); ++j) {
+        expect(iterator.current, equals(j));
+      }
+    }
+    list.add(0);
+    for (int i = 0; i < 10; ++i) {
+      list.add(1);
+    }
+    list.add(2);
+    iterator = list.iterator;
+    for (int i = 0; iterator.moveNext(); ++i) {
+      expect(iterator.current, equals(i));
+      expect(i, lessThan(3));
+    }
+    for (int i = 2; i >= 0; --i) {
+      expect(list.remove(i), isTrue);
+      iterator = list.iterator;
+      for (int j = 0; iterator.moveNext(); ++j) {
+        expect(iterator.current, equals(i != 0 ? j : 1));
+        expect(j, lessThan(3));
+      }
+    }
+    iterator = list.iterator;
+    for (int j = 0; iterator.moveNext(); ++j) {
+      expect(iterator.current, equals(1));
+      expect(j, equals(0));
+    }
+    expect(list.isEmpty, isFalse);
+    iterator = list.iterator;
+    iterator.moveNext();
+    expect(iterator.current, equals(1));
+    for (int i = 0; i < 9; ++i) {
+      expect(list.remove(1), isTrue);
+    }
+    expect(list.isEmpty, isTrue);
+  });
+}

packages/flutter/test/material/buttons_test.dart

@@ -406,6 +406,8 @@ void main() {
         ),
       ),
     );
+
+    WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
     focusNode.requestFocus();
     await tester.pumpAndSettle();
 
@@ -497,6 +499,7 @@ void main() {
       ),
     );
     await tester.pumpAndSettle();
+    WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
 
     // Base elevation
     Material material = tester.widget<Material>(rawButtonMaterial);

packages/flutter/test/material/ink_well_test.dart

@@ -121,6 +121,7 @@ void main() {
   });
 
   testWidgets('ink response changes color on focus', (WidgetTester tester) async {
+    WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
     final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus');
     await tester.pumpWidget(Material(
       child: Directionality(
@@ -154,6 +155,40 @@ void main() {
       ..rect(rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0), color: const Color(0xff0000ff)));
   });
 
+  testWidgets("ink response doesn't change color on focus when on touch device", (WidgetTester tester) async {
+    WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTouch;
+    final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus');
+    await tester.pumpWidget(Material(
+      child: Directionality(
+        textDirection: TextDirection.ltr,
+        child: Center(
+          child: Focus(
+            focusNode: focusNode,
+            child: Container(
+              width: 100,
+              height: 100,
+              child: InkWell(
+                  hoverColor: const Color(0xff00ff00),
+                  splashColor: const Color(0xffff0000),
+                  focusColor: const Color(0xff0000ff),
+                  highlightColor: const Color(0xf00fffff),
+                  onTap: () {},
+                  onLongPress: () {},
+                  onHover: (bool hover) {}
+              ),
+            ),
+          ),
+        ),
+      ),
+    ));
+    await tester.pumpAndSettle();
+    final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
+    expect(inkFeatures, paintsExactlyCountTimes(#rect, 0));
+    focusNode.requestFocus();
+    await tester.pumpAndSettle();
+    expect(inkFeatures, paintsExactlyCountTimes(#rect, 0));
+  });
+
   group('feedback', () {
     FeedbackTester feedback;
 

packages/flutter/test/material/raw_material_button_test.dart

@@ -215,6 +215,7 @@ void main() {
     const Key key = Key('test');
     const Color focusColor = Color(0xff00ff00);
 
+    WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
     await tester.pumpWidget(
       MaterialApp(
         home: Center(
@@ -313,6 +314,7 @@ void main() {
     const Key key = Key('test');
     const Color hoverColor = Color(0xff00ff00);
 
+    WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
     await tester.pumpWidget(
       MaterialApp(
         home: Center(

packages/flutter/test/widgets/focus_manager_test.dart

@@ -5,6 +5,7 @@
 import 'dart:typed_data';
 
 import 'package:flutter/foundation.dart';
+import 'package:flutter/gestures.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter/widgets.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -483,6 +484,55 @@ void main() {
       // receive it.
       expect(receivedAnEvent, isEmpty);
     });
+    testWidgets('Events change focus highlight mode.', (WidgetTester tester) async {
+      await setupWidget(tester);
+      int callCount = 0;
+      FocusHighlightMode lastMode;
+      void handleModeChange(FocusHighlightMode mode) {
+        lastMode = mode;
+        callCount++;
+      }
+
+      final FocusManager focusManager = WidgetsBinding.instance.focusManager;
+      focusManager.addHighlightModeListener(handleModeChange);
+      addTearDown(() => focusManager.removeHighlightModeListener(handleModeChange));
+      expect(callCount, equals(0));
+      expect(lastMode, isNull);
+      focusManager.highlightStrategy = FocusHighlightStrategy.automatic;
+      expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
+      sendFakeKeyEvent(<String, dynamic>{
+        'type': 'keydown',
+        'keymap': 'fuchsia',
+        'hidUsage': 0x04,
+        'codePoint': 0x64,
+        'modifiers': RawKeyEventDataFuchsia.modifierLeftMeta,
+      });
+      expect(callCount, equals(1));
+      expect(lastMode, FocusHighlightMode.traditional);
+      expect(focusManager.highlightMode, equals(FocusHighlightMode.traditional));
+      await tester.tap(find.byType(Container));
+      expect(callCount, equals(2));
+      expect(lastMode, FocusHighlightMode.touch);
+      expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
+      final TestGesture gesture = await tester.startGesture(Offset.zero, kind: PointerDeviceKind.mouse);
+      addTearDown(gesture.removePointer);
+      await gesture.up();
+      expect(callCount, equals(3));
+      expect(lastMode, FocusHighlightMode.traditional);
+      expect(focusManager.highlightMode, equals(FocusHighlightMode.traditional));
+      await tester.tap(find.byType(Container));
+      expect(callCount, equals(4));
+      expect(lastMode, FocusHighlightMode.touch);
+      expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
+      focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
+      expect(callCount, equals(5));
+      expect(lastMode, FocusHighlightMode.traditional);
+      expect(focusManager.highlightMode, equals(FocusHighlightMode.traditional));
+      focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTouch;
+      expect(callCount, equals(6));
+      expect(lastMode, FocusHighlightMode.touch);
+      expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
+    });
     testWidgets('implements debugFillProperties', (WidgetTester tester) async {
       final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder();
       FocusScopeNode(