Fix Focus

Focus.at() and company should be on Focus, not FocusState.

_notifyDescendants() was using the wrong runtimeType.

Let InheritedWidget update the descendants during build.

When you setState() during build, assert that you're not
markNeedsBuild()ing someone who isn't a descendant.

Typo in Widget.toString().

examples/address_book/lib/main.dart

@@ -53,14 +53,12 @@ class AddressBookHome extends StatelessComponent {
     );
   }
 
-  static final GlobalKey nameKey = new GlobalKey();
-  static final GlobalKey phoneKey = new GlobalKey();
-  static final GlobalKey emailKey = new GlobalKey();
-  static final GlobalKey addressKey = new GlobalKey();
-  static final GlobalKey ringtoneKey = new GlobalKey();
-  static final GlobalKey noteKey = new GlobalKey();
-  static final GlobalKey fillKey = new GlobalKey();
-  static final GlobalKey emoticonKey = new GlobalKey();
+  static final GlobalKey nameKey = new GlobalKey(label: 'name field');
+  static final GlobalKey phoneKey = new GlobalKey(label: 'phone field');
+  static final GlobalKey emailKey = new GlobalKey(label: 'email field');
+  static final GlobalKey addressKey = new GlobalKey(label: 'address field');
+  static final GlobalKey ringtoneKey = new GlobalKey(label: 'ringtone field');
+  static final GlobalKey noteKey = new GlobalKey(label: 'note field');
 
   Widget buildBody(BuildContext context) {
     return new Material(

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

@@ -41,7 +41,7 @@ class WidgetFlutterBinding extends FlutterBinding {
     Element.finalizeTree();
   }
 
-  List<BuildableElement> _dirtyElements = new List<BuildableElement>();
+  List<BuildableElement> _dirtyElements = <BuildableElement>[];
 
   /// Adds an element to the dirty elements list so that it will be rebuilt
   /// when buildDirtyElements is called.
@@ -62,10 +62,19 @@ class WidgetFlutterBinding extends FlutterBinding {
       return;
     BuildableElement.lockState(() {
       _dirtyElements.sort((BuildableElement a, BuildableElement b) => a.depth - b.depth);
-      for (BuildableElement element in _dirtyElements)
-        element.rebuild();
+      int dirtyCount = _dirtyElements.length;
+      int index = 0;
+      while (index < dirtyCount) {
+        _dirtyElements[index].rebuild();
+        index += 1;
+        if (dirtyCount < _dirtyElements.length) {
+          _dirtyElements.sort((BuildableElement a, BuildableElement b) => a.depth - b.depth);
+          dirtyCount = _dirtyElements.length;
+        }
+      }
+      assert(!_dirtyElements.any((BuildableElement element) => element.dirty));
       _dirtyElements.clear();
-    });
+    }, building: true);
     assert(_dirtyElements.isEmpty);
   }
 }
@@ -76,7 +85,7 @@ void runApp(Widget app) {
     WidgetFlutterBinding.instance.renderViewElement.update(
       WidgetFlutterBinding.instance.describeApp(app)
     );
-  });
+  }, building: true);
 }
 
 void debugDumpApp() {

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

@@ -75,6 +75,52 @@ class Focus extends StatefulComponent {
   final bool autofocus;
   final Widget child;
 
+  static bool at(BuildContext context, Widget widget, { bool autofocus: true }) {
+    assert(widget != null);
+    assert(widget.key is GlobalKey);
+    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
+    if (focusScope != null) {
+      if (autofocus)
+        focusScope._setFocusedWidgetIfUnset(widget.key);
+      return focusScope.scopeFocused &&
+             focusScope.focusedScope == null &&
+             focusScope.focusedWidget == widget.key;
+    }
+    return true;
+  }
+
+  static bool _atScope(BuildContext context, Widget widget, { bool autofocus: true }) {
+    assert(widget != null);
+    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
+    if (focusScope != null) {
+      if (autofocus)
+        focusScope._setFocusedScopeIfUnset(widget.key);
+      assert(widget.key != null);
+      return focusScope.scopeFocused &&
+             focusScope.focusedScope == widget.key;
+    }
+    return true;
+  }
+
+  // Don't call moveTo() from your build() function, it's intended to be called
+  // from event listeners, e.g. in response to a finger tap or tab key.
+
+  static void moveTo(BuildContext context, Widget widget) {
+    assert(widget != null);
+    assert(widget.key is GlobalKey);
+    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
+    if (focusScope != null)
+      focusScope.focusState._setFocusedWidget(widget.key);
+  }
+
+  static void _moveScopeTo(BuildContext context, Focus component) {
+    assert(component != null);
+    assert(component.key != null);
+    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
+    if (focusScope != null)
+      focusScope.focusState._setFocusedScope(component.key);
+  }
+
   FocusState createState() => new FocusState();
 }
 
@@ -155,7 +201,7 @@ class FocusState extends State<Focus> {
   void initState() {
     super.initState();
     if (config.autofocus)
-      FocusState._moveScopeTo(context, config);
+      Focus._moveScopeTo(context, config);
     _updateWidgetRemovalListener(_focusedWidget);
     _updateScopeRemovalListener(_focusedScope);
   }
@@ -169,56 +215,10 @@ class FocusState extends State<Focus> {
   Widget build(BuildContext context) {
     return new _FocusScope(
       focusState: this,
-      scopeFocused: FocusState._atScope(context, config),
+      scopeFocused: Focus._atScope(context, config),
       focusedScope: _focusedScope == _noFocusedScope ? null : _focusedScope,
       focusedWidget: _focusedWidget,
       child: config.child
     );
   }
-
-  static bool at(BuildContext context, Widget widget, { bool autofocus: true }) {
-    assert(widget != null);
-    assert(widget.key is GlobalKey);
-    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
-    if (focusScope != null) {
-      if (autofocus)
-        focusScope._setFocusedWidgetIfUnset(widget.key);
-      return focusScope.scopeFocused &&
-             focusScope.focusedScope == null &&
-             focusScope.focusedWidget == widget.key;
-    }
-    return true;
-  }
-
-  static bool _atScope(BuildContext context, Widget widget, { bool autofocus: true }) {
-    assert(widget != null);
-    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
-    if (focusScope != null) {
-      if (autofocus)
-        focusScope._setFocusedScopeIfUnset(widget.key);
-      assert(widget.key != null);
-      return focusScope.scopeFocused &&
-             focusScope.focusedScope == widget.key;
-    }
-    return true;
-  }
-
-  // Don't call moveTo() from your build() function, it's intended to be called
-  // from event listeners, e.g. in response to a finger tap or tab key.
-
-  static void moveTo(BuildContext context, Widget widget) {
-    assert(widget != null);
-    assert(widget.key is GlobalKey);
-    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
-    if (focusScope != null)
-      focusScope.focusState._setFocusedWidget(widget.key);
-  }
-
-  static void _moveScopeTo(BuildContext context, Focus component) {
-    assert(component != null);
-    assert(component.key != null);
-    _FocusScope focusScope = context.inheritedWidgetOfType(_FocusScope);
-    if (focusScope != null)
-      focusScope.focusState._setFocusedScope(component.key);
-  }
 }

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

@@ -186,8 +186,8 @@ abstract class Widget {
     final List<String> data = <String>[];
     debugFillDescription(data);
     if (data.isEmpty)
-      return 'name';
-    return 'name(${data.join("; ")})';
+      return '$name';
+    return '$name(${data.join("; ")})';
    }
 
   void debugFillDescription(List<String> description) { }
@@ -550,7 +550,7 @@ abstract class Element<T extends Widget> implements BuildContext {
   /// Wrapper around visitChildren for BuildContext.
   void visitChildElements(void visitor(Element element)) {
     // don't allow visitChildElements() during build, since children aren't necessarily built yet
-    assert(BuildableElement._debugStateLockLevel == 0);
+    assert(!BuildableElement._debugStateLocked);
     visitChildren(visitor);
   }
 
@@ -858,6 +858,8 @@ abstract class BuildableElement<T extends Widget> extends Element<T> {
     assert(_child != null);
   }
 
+  static BuildableElement _debugCurrentBuildTarget;
+
   /// Reinvokes the build() method of the StatelessComponent object (for
   /// stateless components) or the State object (for stateful components) and
   /// then updates the widget tree.
@@ -874,6 +876,12 @@ abstract class BuildableElement<T extends Widget> extends Element<T> {
     assert(_debugLifecycleState == _ElementLifecycle.active);
     assert(_debugStateLocked);
     assert(_debugSetAllowIgnoredCallsToMarkNeedsBuild(true));
+    BuildableElement debugPreviousBuildTarget;
+    assert(() {
+      debugPreviousBuildTarget = _debugCurrentBuildTarget;
+      _debugCurrentBuildTarget = this;
+     return true;
+    });
     Widget built;
     try {
       built = _builder(this);
@@ -896,12 +904,19 @@ abstract class BuildableElement<T extends Widget> extends Element<T> {
       built = new ErrorWidget();
       _child = updateChild(null, built, slot);
     }
+
+    assert(() {
+      assert(_debugCurrentBuildTarget == this);
+      _debugCurrentBuildTarget = debugPreviousBuildTarget;
+      return true;
+    });
   }
 
   static BuildScheduler scheduleBuildFor;
 
   static int _debugStateLockLevel = 0;
   static bool get _debugStateLocked => _debugStateLockLevel > 0;
+  static bool _debugBuilding = false;
 
   /// Establishes a scope in which component build functions can run.
   ///
@@ -913,13 +928,31 @@ abstract class BuildableElement<T extends Widget> extends Element<T> {
   /// After unwinding the last build scope on the stack, the framework verifies
   /// that each global key is used at most once and notifies listeners about
   /// changes to global keys.
-  static void lockState(void callback()) {
+  static void lockState(void callback(), { bool building: false }) {
+    assert(_debugStateLockLevel >= 0);
+    assert(() {
+      if (building) {
+        assert(!_debugBuilding);
+        assert(_debugCurrentBuildTarget == null);
+        _debugBuilding = true;
+      }
       _debugStateLockLevel += 1;
+      return true;
+    });
     try {
       callback();
     } finally {
+      assert(() {
         _debugStateLockLevel -= 1;
+        if (building) {
+          assert(_debugBuilding);
+          assert(_debugCurrentBuildTarget == null);
+          _debugBuilding = false;
+        }
+        return true;
+      });
     }
+    assert(_debugStateLockLevel >= 0);
   }
 
   /// Marks the element as dirty and adds it to the global list of widgets to
@@ -934,10 +967,23 @@ abstract class BuildableElement<T extends Widget> extends Element<T> {
     if (!_active)
       return;
     assert(_debugLifecycleState == _ElementLifecycle.active);
-    assert(!_debugStateLocked || (_debugAllowIgnoredCallsToMarkNeedsBuild && dirty));
+    assert(() {
+      if (_debugBuilding) {
+        bool foundTarget = false;
+        visitAncestorElements((Element element) {
+          if (element == _debugCurrentBuildTarget) {
+            foundTarget = true;
+            return false;
+          }
+          return true;
+        });
+        if (foundTarget)
+          return true;
+      }
+      return !_debugStateLocked || (_debugAllowIgnoredCallsToMarkNeedsBuild && dirty);
+    });
     if (dirty)
       return;
-    assert(!_debugStateLocked);
     _dirty = true;
     assert(scheduleBuildFor != null);
     scheduleBuildFor(this);
@@ -1101,11 +1147,12 @@ class InheritedElement extends StatelessComponentElement<InheritedWidget> {
   }
 
   void _notifyDescendants() {
-    final Type ourRuntimeType = runtimeType;
+    final Type ourRuntimeType = widget.runtimeType;
     void notifyChildren(Element child) {
       if (child._dependencies != null &&
-          child._dependencies.contains(ourRuntimeType))
+          child._dependencies.contains(ourRuntimeType)) {
         child.dependenciesChanged();
+      }
       if (child.runtimeType != ourRuntimeType)
         child.visitChildren(notifyChildren);
     }

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

@@ -71,7 +71,7 @@ class InputState extends ScrollableState<Input> {
 
   Widget buildContent(BuildContext context) {
     ThemeData themeData = Theme.of(context);
-    bool focused = FocusState.at(context, config);
+    bool focused = Focus.at(context, config);
 
     if (focused && !_keyboardHandle.attached) {
       _keyboardHandle = keyboard.show(_editableValue.stub, config.keyboardType);
@@ -122,11 +122,11 @@ class InputState extends ScrollableState<Input> {
         )
       ),
       onPointerDown: (_) {
-        if (FocusState.at(context, config)) {
+        if (Focus.at(context, config)) {
           assert(_keyboardHandle.attached);
           _keyboardHandle.showByRequest();
         } else {
-          FocusState.moveTo(context, config);
+          Focus.moveTo(context, config);
           // we'll get told to rebuild and we'll take care of the keyboard then
         }
       }

packages/unit/test/widget/build_scope_test.dart

@@ -66,6 +66,10 @@ class BadDisposeWidgetState extends State<BadDisposeWidget> {
 void main() {
   dynamic cachedException;
 
+  // ** WARNING **
+  // THIS TEST OVERRIDES THE NORMAL EXCEPTION HANDLING
+  // AND DOES NOT REPORT EXCEPTIONS FROM THE FRAMEWORK
+
   setUp(() {
     assert(cachedException == null);
     debugWidgetsExceptionHandler = (String context, dynamic exception, StackTrace stack) {

packages/unit/test/widget/focus_test.dart

+import 'package:sky/widgets.dart';
+import 'package:test/test.dart';
+
+import 'widget_tester.dart';
+
+class TestFocusable extends StatelessComponent {
+  TestFocusable(this.no, this.yes, GlobalKey key) : super(key: key);
+  final String no;
+  final String yes;
+  Widget build(BuildContext context) {
+    bool focused = Focus.at(context, this);
+    return new GestureDetector(
+      onTap: () { Focus.moveTo(context, this); },
+      child: new Text(focused ? yes : no)
+    );
+  }
+}
+
+void main() {
+  test('Can have multiple focused children and they update accordingly', () {
+    testWidgets((WidgetTester tester) {
+      GlobalKey keyA = new GlobalKey();
+      GlobalKey keyB = new GlobalKey();
+      tester.pumpWidget(
+        new Focus(
+          child: new Column([
+            // reverse these when you fix https://github.com/flutter/engine/issues/1495
+            new TestFocusable('b', 'B FOCUSED', keyB),
+            new TestFocusable('a', 'A FOCUSED', keyA),
+          ])
+        )
+      );
+      expect(tester.findText('a'),         isNull);
+      expect(tester.findText('A FOCUSED'), isNotNull);
+      expect(tester.findText('b'),         isNotNull);
+      expect(tester.findText('B FOCUSED'), isNull);
+      tester.tap(tester.findText('A FOCUSED'));
+      tester.pump();
+      expect(tester.findText('a'),         isNull);
+      expect(tester.findText('A FOCUSED'), isNotNull);
+      expect(tester.findText('b'),         isNotNull);
+      expect(tester.findText('B FOCUSED'), isNull);
+      tester.tap(tester.findText('A FOCUSED'));
+      tester.pump();
+      expect(tester.findText('a'),         isNull);
+      expect(tester.findText('A FOCUSED'), isNotNull);
+      expect(tester.findText('b'),         isNotNull);
+      expect(tester.findText('B FOCUSED'), isNull);
+      tester.tap(tester.findText('b'));
+      tester.pump();
+      expect(tester.findText('a'),         isNotNull);
+      expect(tester.findText('A FOCUSED'), isNull);
+      expect(tester.findText('b'),         isNull);
+      expect(tester.findText('B FOCUSED'), isNotNull);
+      tester.tap(tester.findText('a'));
+      tester.pump();
+      expect(tester.findText('a'),         isNull);
+      expect(tester.findText('A FOCUSED'), isNotNull);
+      expect(tester.findText('b'),         isNotNull);
+      expect(tester.findText('B FOCUSED'), isNull);
+    });
+  });
+}