Mark State.setState as protected (#4295)

This required refactoring some cases where we weren't following the rules for
the protected annotation.

dev/benchmarks/complex_layout/lib/main.dart

@@ -35,6 +35,12 @@ class ComplexLayoutAppState extends State<ComplexLayoutApp> {
       _lightTheme = value;
     });
   }
+
+  void toggleAnimationSpeed() {
+    setState(() {
+      timeDilation = (timeDilation != 1.0) ? 1.0 : 5.0;
+    });
+  }
 }
 
 class ComplexLayout extends StatefulWidget {
@@ -566,12 +572,6 @@ class GalleryDrawer extends StatelessWidget {
     ComplexLayoutApp.of(context).lightTheme = value;
   }
 
-  void _toggleAnimationSpeed(BuildContext context) {
-    ComplexLayoutApp.of(context).setState(() {
-      timeDilation = (timeDilation != 1.0) ? 1.0 : 5.0;
-    });
-  }
-
   @override
   Widget build(BuildContext context) {
     return new Drawer(
@@ -612,13 +612,13 @@ class GalleryDrawer extends StatelessWidget {
           new DrawerItem(
             icon: Icons.hourglass_empty,
             selected: timeDilation != 1.0,
-            onPressed: () { _toggleAnimationSpeed(context); },
+            onPressed: () { ComplexLayoutApp.of(context).toggleAnimationSpeed(); },
             child: new Row(
               children: <Widget>[
                 new Flexible(child: new Text('Animate Slowly')),
                 new Checkbox(
                   value: timeDilation != 1.0,
-                  onChanged: (bool value) { _toggleAnimationSpeed(context); }
+                  onChanged: (bool value) { ComplexLayoutApp.of(context).toggleAnimationSpeed(); }
                 )
               ]
             )

dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart

@@ -8,8 +8,6 @@ import 'package:stocks/stock_data.dart' as stock_data;
 
 const Duration kBenchmarkTime = const Duration(seconds: 15);
 
-void _doNothing() { }
-
 Future<Null> main() async {
   assert(false); // don't run this in checked mode! Use --release.
   stock_data.StockDataFetcher.actuallyFetchData = false;
@@ -26,12 +24,12 @@ Future<Null> main() async {
     await tester.pump(const Duration(seconds: 1)); // Complete drawer animation
 
 
-    final stocks.StocksAppState appState = tester.state(find.byType(stocks.StocksApp));
+    final BuildableElement appState = tester.element(find.byType(stocks.StocksApp));
     final BuildOwner buildOwner = WidgetsBinding.instance.buildOwner;
 
     watch.start();
     while (watch.elapsed < kBenchmarkTime) {
-      appState.setState(_doNothing);
+      appState.markNeedsBuild();
       buildOwner.buildDirtyElements();
       iterations += 1;
     }

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

@@ -405,6 +405,7 @@ abstract class State<T extends StatefulWidget> {
   /// If you just change the state directly without calling setState(), then the
   /// widget will not be scheduled for rebuilding, meaning that its rendering
   /// will not be updated.
+  @protected
   void setState(VoidCallback fn) {
     assert(() {
       if (_debugLifecycleState == _StateLifecycle.defunct) {

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

@@ -196,15 +196,13 @@ class LazyBlock extends StatelessWidget {
       double contentExtent,
       double containerExtent,
       double minScrollOffset) {
-    state.setState(() {
-      final BoundedBehavior scrollBehavior = state.scrollBehavior;
-      state.didUpdateScrollBehavior(scrollBehavior.updateExtents(
-        contentExtent: contentExtent,
-        containerExtent: containerExtent,
-        minScrollOffset: minScrollOffset,
-        scrollOffset: state.scrollOffset
-      ));
-    });
+    final BoundedBehavior scrollBehavior = state.scrollBehavior;
+    state.didUpdateScrollBehavior(scrollBehavior.updateExtents(
+      contentExtent: contentExtent,
+      containerExtent: containerExtent,
+      minScrollOffset: minScrollOffset,
+      scrollOffset: state.scrollOffset
+    ));
   }
 
   Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {

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

@@ -63,14 +63,13 @@ class OverlayEntry {
   set opaque (bool value) {
     if (_opaque == value)
       return;
+    _opaque = value;
     assert(_overlay != null);
-    _overlay.setState(() {
-      _opaque = value;
-    });
+    _overlay._didChangeEntryOpacity();
   }
 
   OverlayState _overlay;
-  final GlobalKey _key = new GlobalKey();
+  final GlobalKey<_OverlayEntryState> _key = new GlobalKey<_OverlayEntryState>();
 
   /// Remove this entry from the overlay.
   void remove() {
@@ -82,7 +81,7 @@ class OverlayEntry {
   ///
   /// You need to call this function if the output of [builder] has changed.
   void markNeedsBuild() {
-    _key.currentState?.setState(() { /* the state that changed is in the builder */ });
+    _key.currentState?._markNeedsBuild();
   }
 
   @override
@@ -101,6 +100,10 @@ class _OverlayEntry extends StatefulWidget {
 class _OverlayEntryState extends State<_OverlayEntry> {
   @override
   Widget build(BuildContext context) => config.entry.builder(context);
+
+  void _markNeedsBuild() {
+    setState(() { /* the state that changed is in the builder */ });
+  }
 }
 
 /// A [Stack] of entries that can be managed independently.
@@ -245,6 +248,13 @@ class OverlayState extends State<Overlay> {
     return result;
   }
 
+  void _didChangeEntryOpacity() {
+    setState(() {
+      // We use the opacity of the entry in our build function, which means we
+      // our state has changed.
+    });
+  }
+
   @override
   Widget build(BuildContext context) {
     List<Widget> backwardsChildren = <Widget>[];

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

@@ -394,6 +394,13 @@ class _ModalScopeState extends State<_ModalScope> {
     });
   }
 
+  void _didChangeRouteOffStage() {
+    setState(() {
+      // We use the route's offstage bool in our build function, which means our
+      // state has changed.
+    });
+  }
+
   @override
   Widget build(BuildContext context) {
     Widget contents = new PageStorage(
@@ -515,14 +522,7 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
     if (_offstage == value)
       return;
     _offstage = value;
-    _scopeKey.currentState?.setState(() {
-      // _offstage is the value we're setting, but since there might not be a
-      // state, we set it outside of this callback (which will only be called if
-      // there's a state currently built).
-      // _scopeKey is the key for the _ModalScope built in _buildModalScope().
-      // When we mark that state dirty, it'll rebuild itself, and use our
-      // offstage (via their config.route.offstage) when building.
-    });
+    _scopeKey.currentState?._didChangeRouteOffStage();
   }
 
   /// The build context for the subtree containing the primary content of this route.

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

@@ -402,6 +402,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
   /// If there are no in-progress scrolling physics, this function scrolls to
   /// the given offset instead.
   void didUpdateScrollBehavior(double newScrollOffset) {
+    setState(() { /* The scroll behavior is part of our build state. */ });
     assert(_controller.isAnimating || _simulation == null);
     if (_numberOfInProgressScrolls > 0) {
       if (_simulation != null) {

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

@@ -67,13 +67,11 @@ class ScrollableGrid extends StatelessWidget {
   final Iterable<Widget> children;
 
   void _handleExtentsChanged(ScrollableState state, double contentExtent, double containerExtent) {
-    state.setState(() {
-      state.didUpdateScrollBehavior(state.scrollBehavior.updateExtents(
-        contentExtent: contentExtent,
-        containerExtent: containerExtent,
-        scrollOffset: state.scrollOffset
-      ));
-    });
+    state.didUpdateScrollBehavior(state.scrollBehavior.updateExtents(
+      contentExtent: contentExtent,
+      containerExtent: containerExtent,
+      scrollOffset: state.scrollOffset
+    ));
   }
 
   Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {

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

@@ -102,13 +102,11 @@ class ScrollableList extends StatelessWidget {
   final Iterable<Widget> children;
 
   void _handleExtentsChanged(ScrollableState state, double contentExtent, double containerExtent) {
-    state.setState(() {
-      state.didUpdateScrollBehavior(state.scrollBehavior.updateExtents(
-        contentExtent: itemsWrap ? double.INFINITY : contentExtent,
-        containerExtent: containerExtent,
-        scrollOffset: state.scrollOffset
-      ));
-    });
+    state.didUpdateScrollBehavior(state.scrollBehavior.updateExtents(
+      contentExtent: itemsWrap ? double.INFINITY : contentExtent,
+      containerExtent: containerExtent,
+      scrollOffset: state.scrollOffset
+    ));
   }
 
   Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {
@@ -428,13 +426,11 @@ class ScrollableLazyList extends StatelessWidget {
   final EdgeInsets padding;
 
   void _handleExtentsChanged(ScrollableState state, double contentExtent, double containerExtent) {
-    state.setState(() {
-      state.didUpdateScrollBehavior(state.scrollBehavior.updateExtents(
-        contentExtent: contentExtent,
-        containerExtent: containerExtent,
-        scrollOffset: state.scrollOffset
-      ));
-    });
+    state.didUpdateScrollBehavior(state.scrollBehavior.updateExtents(
+      contentExtent: contentExtent,
+      containerExtent: containerExtent,
+      scrollOffset: state.scrollOffset
+    ));
   }
 
   Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {

packages/flutter/test/widget/build_scope_test.dart

@@ -37,11 +37,11 @@ class ProbeWidgetState extends State<ProbeWidget> {
 class BadWidget extends StatelessWidget {
   BadWidget(this.parentState);
 
-  final State parentState;
+  final BadWidgetParentState parentState;
 
   @override
   Widget build(BuildContext context) {
-    parentState.setState(() {});
+    parentState._markNeedsBuild();
     return new Container();
   }
 }
@@ -52,6 +52,13 @@ class BadWidgetParent extends StatefulWidget {
 }
 
 class BadWidgetParentState extends State<BadWidgetParent> {
+  void _markNeedsBuild() {
+    setState(() {
+      // Our state didn't really change, but we're doing something pathological
+      // here to trigger an interesting scenario to test.
+    });
+  }
+
   @override
   Widget build(BuildContext context) {
     return new BadWidget(this);

packages/flutter/test/widget/stateful_components_test.dart

@@ -75,7 +75,7 @@ void main() {
 
     StatefulElement outerElement = tester.element(find.byKey(outerKey));
     expect(outerElement.state.config, equals(outer2));
-    outerElement.state.setState(() {});
+    outerElement.markNeedsBuild();
     await tester.pump();
 
     expect(tester.element(find.byKey(innerKey)), equals(innerElement));