Make cursor blinking more efficient (#9142)

Rather than rebuilding to blink the cursor, we now pass a
ValueNotifier<bool> to the RenderEditable so that it can simply repaint.

This patch also contains some refactoring towards being able to do the
same thing with the text being edited, but I didn't quite get it
working.

packages/flutter/lib/src/rendering/editable.dart

@@ -49,7 +49,7 @@ class RenderEditable extends RenderBox {
     TextSpan text,
     TextAlign textAlign,
     Color cursorColor,
-    bool showCursor: false,
+    ValueNotifier<bool> showCursor,
     int maxLines: 1,
     Color selectionColor,
     double textScaleFactor: 1.0,
@@ -58,15 +58,15 @@ class RenderEditable extends RenderBox {
     this.onSelectionChanged,
   }) : _textPainter = new TextPainter(text: text, textAlign: textAlign, textScaleFactor: textScaleFactor),
        _cursorColor = cursorColor,
-       _showCursor = showCursor,
+       _showCursor = showCursor ?? new ValueNotifier<bool>(false),
        _maxLines = maxLines,
        _selection = selection,
        _offset = offset {
-    assert(showCursor != null);
+    assert(_showCursor != null);
     assert(maxLines != null);
     assert(textScaleFactor != null);
     assert(offset != null);
-    assert(!showCursor || cursorColor != null);
+    assert(!_showCursor.value || cursorColor != null);
     _tap = new TapGestureRecognizer()
       ..onTapDown = _handleTapDown
       ..onTap = _handleTap
@@ -108,13 +108,17 @@ class RenderEditable extends RenderBox {
   }
 
   /// Whether to paint the cursor.
-  bool get showCursor => _showCursor;
-  bool _showCursor;
-  set showCursor(bool value) {
+  ValueNotifier<bool> get showCursor => _showCursor;
+  ValueNotifier<bool> _showCursor;
+  set showCursor(ValueNotifier<bool> value) {
     assert(value != null);
     if (_showCursor == value)
       return;
+    if (attached)
+      _showCursor.removeListener(markNeedsPaint);
     _showCursor = value;
+    if (attached)
+      _showCursor.addListener(markNeedsPaint);
     markNeedsPaint();
   }
 
@@ -186,6 +190,20 @@ class RenderEditable extends RenderBox {
     markNeedsLayout();
   }
 
+  @override
+  void attach(PipelineOwner owner) {
+    super.attach(owner);
+    _offset.addListener(markNeedsPaint);
+    _showCursor.addListener(markNeedsPaint);
+  }
+
+  @override
+  void detach() {
+    _offset.removeListener(markNeedsPaint);
+    _showCursor.removeListener(markNeedsPaint);
+    super.detach();
+  }
+
   bool get _isMultiline => maxLines > 1;
 
   Axis get _viewportAxis => _isMultiline ? Axis.vertical : Axis.horizontal;
@@ -377,7 +395,7 @@ class RenderEditable extends RenderBox {
     final Offset effectiveOffset = offset + _paintOffset;
 
     if (_selection != null) {
-      if (_selection.isCollapsed && _showCursor && cursorColor != null) {
+      if (_selection.isCollapsed && _showCursor.value && cursorColor != null) {
         _paintCaret(context.canvas, effectiveOffset);
       } else if (!_selection.isCollapsed && _selectionColor != null) {
         _selectionRects ??= _textPainter.getBoxesForSelection(_selection);

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

@@ -23,29 +23,19 @@ export 'package:flutter/services.dart' show TextEditingValue, TextSelection, Tex
 
 const Duration _kCursorBlinkHalfPeriod = const Duration(milliseconds: 500);
 
-class TextEditingController extends ChangeNotifier {
+class TextEditingController extends ValueNotifier<TextEditingValue> {
   TextEditingController({ String text })
-    : _value = text == null ? TextEditingValue.empty : new TextEditingValue(text: text);
+    : super(text == null ? TextEditingValue.empty : new TextEditingValue(text: text));
 
   TextEditingController.fromValue(TextEditingValue value)
-    : _value = value ?? TextEditingValue.empty;
+    : super(value ?? TextEditingValue.empty);
 
-  TextEditingValue get value => _value;
-  TextEditingValue _value;
-  set value(TextEditingValue newValue) {
-    assert(newValue != null);
-    if (_value == newValue)
-      return;
-    _value = newValue;
-    notifyListeners();
-  }
-
-  String get text => _value.text;
+  String get text => value.text;
   set text(String newText) {
     value = value.copyWith(text: newText, composing: TextRange.empty);
   }
 
-  TextSelection get selection => _value.selection;
+  TextSelection get selection => value.selection;
   set selection(TextSelection newSelection) {
     value = value.copyWith(selection: newSelection, composing: TextRange.empty);
   }
@@ -174,7 +164,7 @@ class EditableText extends StatefulWidget {
 /// State for a [EditableText].
 class EditableTextState extends State<EditableText> implements TextInputClient {
   Timer _cursorTimer;
-  bool _showCursor = false;
+  final ValueNotifier<bool> _showCursor = new ValueNotifier<bool>(false);
 
   TextInputConnection _textInputConnection;
   TextSelectionOverlay _selectionOverlay;
@@ -206,7 +196,7 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
     if (config.controller != oldConfig.controller) {
       oldConfig.controller.removeListener(_didChangeTextEditingValue);
       config.controller.addListener(_didChangeTextEditingValue);
-      if (_isAttachedToKeyboard && config.controller.value != oldConfig.controller.value)
+      if (_hasInputConnection && config.controller.value != oldConfig.controller.value)
         _textInputConnection.setEditingState(config.controller.value);
      }
     if (config.focusNode != oldConfig.focusNode) {
@@ -218,13 +208,9 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
   @override
   void dispose() {
     config.controller.removeListener(_didChangeTextEditingValue);
-    if (_isAttachedToKeyboard) {
-      _textInputConnection.close();
-      _textInputConnection = null;
-    }
-    assert(!_isAttachedToKeyboard);
-    if (_cursorTimer != null)
-      _stopCursorTimer();
+    _closeInputConnectionIfNeeded();
+    assert(!_hasInputConnection);
+    _stopCursorTimer();
     assert(_cursorTimer == null);
     _selectionOverlay?.dispose();
     _selectionOverlay = null;
@@ -256,12 +242,7 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
     config.controller.value = value;
   }
 
-  void _didChangeTextEditingValue() {
-    setState(() { /* We use config.controller.value in build(). */ });
-  }
-
-  bool get _isAttachedToKeyboard => _textInputConnection != null && _textInputConnection.attached;
-
+  bool get _hasFocus => config.focusNode.hasFocus;
   bool get _isMultiline => config.maxLines > 1;
 
   // Calculate the new scroll offset so the cursor remains visible.
@@ -278,17 +259,28 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
   }
 
   bool _didRequestKeyboard = false;
+  bool get _hasInputConnection => _textInputConnection != null && _textInputConnection.attached;
 
-  void _attachOrDetachKeyboard(bool focused) {
-    if (focused && !_isAttachedToKeyboard && _didRequestKeyboard) {
+  void _openInputConnectionIfNeeded() {
+    if (!_hasInputConnection) {
       _textInputConnection = TextInput.attach(this, new TextInputConfiguration(inputType: config.keyboardType))
         ..setEditingState(_value)
         ..show();
-    } else if (!focused) {
-      if (_isAttachedToKeyboard) {
-        _textInputConnection.close();
-        _textInputConnection = null;
-      }
+    }
+  }
+
+  void _closeInputConnectionIfNeeded() {
+    if (_hasInputConnection) {
+      _textInputConnection.close();
+      _textInputConnection = null;
+    }
+  }
+
+  void _openOrCloseInputConnectionIfNeeded() {
+    if (_hasFocus && _didRequestKeyboard) {
+      _openInputConnectionIfNeeded();
+    } else if (!_hasFocus) {
+      _closeInputConnectionIfNeeded();
       config.controller.clearComposing();
     }
     _didRequestKeyboard = false;
@@ -302,14 +294,15 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
   /// focus, the control will then attach to the keyboard and request that the
   /// keyboard become visible.
   void requestKeyboard() {
-    if (_isAttachedToKeyboard) {
+    if (_hasInputConnection) {
       _textInputConnection.show();
     } else {
-      _didRequestKeyboard = true;
-      if (config.focusNode.hasFocus)
-        _attachOrDetachKeyboard(true);
-      else
+      if (_hasFocus) {
+        _openInputConnectionIfNeeded();
+      } else {
+        _didRequestKeyboard = true;
         FocusScope.of(context).requestFocus(config.focusNode);
+      }
     }
   }
 
@@ -318,6 +311,17 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
     _selectionOverlay = null;
   }
 
+  void _updateOrDisposeSelectionOverlayIfNeeded() {
+    if (_selectionOverlay != null) {
+      if (_hasFocus) {
+        _selectionOverlay.update(_value);
+      } else {
+        _selectionOverlay.dispose();
+        _selectionOverlay = null;
+      }
+    }
+  }
+
   void _handleSelectionChanged(TextSelection selection, RenderEditable renderObject, bool longPress) {
     // Note that this will show the keyboard for all selection changes on the
     // EditableWidget, not just changes triggered by user gestures.
@@ -351,7 +355,7 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
   /// Whether the blinking cursor is actually visible at this precise moment
   /// (it's hidden half the time, since it blinks).
   @visibleForTesting
-  bool get cursorCurrentlyVisible => _showCursor;
+  bool get cursorCurrentlyVisible => _showCursor.value;
 
   /// The cursor blink interval (the amount of time the cursor is in the "on"
   /// state or the "off" state). A complete cursor blink period is twice this
@@ -360,39 +364,39 @@ class EditableTextState extends State<EditableText> implements TextInputClient {
   Duration get cursorBlinkInterval => _kCursorBlinkHalfPeriod;
 
   void _cursorTick(Timer timer) {
-    setState(() {
-      _showCursor = !_showCursor;
-    });
+    _showCursor.value = !_showCursor.value;
   }
 
   void _startCursorTimer() {
-    _showCursor = true;
+    _showCursor.value = true;
     _cursorTimer = new Timer.periodic(_kCursorBlinkHalfPeriod, _cursorTick);
   }
 
-  void _handleFocusChanged() {
-    final bool focused = config.focusNode.hasFocus;
-    _attachOrDetachKeyboard(focused);
+  void _stopCursorTimer() {
+    _cursorTimer?.cancel();
+    _cursorTimer = null;
+    _showCursor.value = false;
+  }
 
-    if (_cursorTimer == null && focused && _value.selection.isCollapsed)
+  void _startOrStopCursorTimerIfNeeded() {
+    if (_cursorTimer == null && _hasFocus && _value.selection.isCollapsed)
       _startCursorTimer();
-    else if (_cursorTimer != null && (!focused || !_value.selection.isCollapsed))
+    else if (_cursorTimer != null && (!_hasFocus || !_value.selection.isCollapsed))
       _stopCursorTimer();
+  }
 
-    if (_selectionOverlay != null) {
-      if (focused) {
-        _selectionOverlay.update(_value);
-      } else {
-        _selectionOverlay.dispose();
-        _selectionOverlay = null;
-      }
-    }
+  void _didChangeTextEditingValue() {
+    _startOrStopCursorTimerIfNeeded();
+    _updateOrDisposeSelectionOverlayIfNeeded();
+    // TODO(abarth): Teach RenderEditable about ValueNotifier<TextEditingValue>
+    // to avoid this setState().
+    setState(() { /* We use config.controller.value in build(). */ });
   }
 
-  void _stopCursorTimer() {
-    _cursorTimer.cancel();
-    _cursorTimer = null;
-    _showCursor = false;
+  void _handleFocusChanged() {
+    _openOrCloseInputConnectionIfNeeded();
+    _startOrStopCursorTimerIfNeeded();
+    _updateOrDisposeSelectionOverlayIfNeeded();
   }
 
   @override
@@ -440,7 +444,7 @@ class _Editable extends LeafRenderObjectWidget {
   final TextEditingValue value;
   final TextStyle style;
   final Color cursorColor;
-  final bool showCursor;
+  final ValueNotifier<bool> showCursor;
   final int maxLines;
   final Color selectionColor;
   final double textScaleFactor;