Update Semantics for SingleChildScrollViews (#12376)

* Update Semantics for SingleChildScrollViews

* refactor

* review feedback

* added assert and comments

* doc

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

@@ -2145,6 +2145,11 @@ class BuildOwner {
 
   int _debugStateLockLevel = 0;
   bool get _debugStateLocked => _debugStateLockLevel > 0;
+
+  /// Whether this widget tree is in the build phase.
+  ///
+  /// Only valid when asserts are enabled.
+  bool get debugBuilding => _debugBuilding;
   bool _debugBuilding = false;
   Element _debugCurrentBuildTarget;
 

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

@@ -535,29 +535,31 @@ class RawGestureDetectorState extends State<RawGestureDetector> {
     }
   }
 
-  /// This method can be called after the build phase, during the layout of the
+  /// This method can be called outside of the build phase to filter the list of
-  /// nearest descendant [RenderObjectWidget] of the gesture detector, to filter
+  /// available semantic actions.
-  /// the list of available semantic actions.
+  ///
+  /// The actual filtering is happening in the next frame and a frame will be
+  /// scheduled if non is pending.
   ///
   /// This is used by [Scrollable] to configure system accessibility tools so
   /// that they know in which direction a particular list can be scrolled.
   ///
   /// If this is never called, then the actions are not filtered. If the list of
-  /// actions to filter changes, it must be called again (during the layout of
+  /// actions to filter changes, it must be called again.
-  /// the nearest descendant [RenderObjectWidget] of the gesture detector).
   void replaceSemanticsActions(Set<SemanticsAction> actions) {
     assert(() {
-      if (!context.findRenderObject().owner.debugDoingLayout) {
+      final Element element = context;
+      if (element.owner.debugBuilding) {
         throw new FlutterError(
           'Unexpected call to replaceSemanticsActions() method of RawGestureDetectorState.\n'
-          'The replaceSemanticsActions() method can only be called during the layout phase.'
+          'The replaceSemanticsActions() method can only be called outside of the build phase.'
         );
       }
       return true;
     }());
     if (!widget.excludeFromSemantics) {
       final RenderSemanticsGestureHandler semanticsGestureHandler = context.findRenderObject();
-      semanticsGestureHandler.validActions = actions;
+      semanticsGestureHandler.validActions = actions;  // will call _markNeedsSemanticsUpdate(), if required.
     }
   }
 

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

@@ -371,6 +371,18 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
 
   Set<SemanticsAction> _semanticActions;
 
+  /// Called whenever the scroll position or the dimensions of the scroll view
+  /// change to schedule an update of the available semantics actions. The
+  /// actual update will be performed in the nxt frame. If non is pending
+  /// a frame will be scheduled.
+  ///
+  /// For example: If the scroll view has been scrolled all the way to the top,
+  /// the action to scroll further up needs to be removed as the scroll view
+  /// cannot be scrolled in that direction anymore.
+  ///
+  /// This method is potentially called twice per frame (if scroll position and
+  /// scroll view dimensions both change) and therefore shouldn't do anything
+  /// expensive.
   void _updateSemanticActions() {
     SemanticsAction forward;
     SemanticsAction backward;
@@ -409,7 +421,6 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
       applyNewDimensions();
       _didChangeViewportDimension = false;
     }
-    _updateSemanticActions();
     return true;
   }
 
@@ -437,6 +448,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   void applyNewDimensions() {
     assert(pixels != null);
     activity.applyNewDimensions();
+    _updateSemanticActions();  // will potentially request a semantics update.
   }
 
   /// Animates the position such that the given object is as visible as possible
@@ -613,6 +625,12 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
     super.dispose();
   }
 
+  @override
+  void notifyListeners() {
+    _updateSemanticActions();  // will potentially request a semantics update.
+    super.notifyListeners();
+  }
+
   @override
   void debugFillDescription(List<String> description) {
     if (debugLabel != null)

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

@@ -205,13 +205,18 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix
     if (value == _offset)
       return;
     if (attached)
-      _offset.removeListener(markNeedsPaint);
+      _offset.removeListener(_hasScrolled);
     _offset = value;
     if (attached)
-      _offset.addListener(markNeedsPaint);
+      _offset.addListener(_hasScrolled);
     markNeedsLayout();
   }
 
+  void _hasScrolled() {
+    markNeedsPaint();
+    markNeedsSemanticsUpdate();
+  }
+
   @override
   void setupParentData(RenderObject child) {
     // We don't actually use the offset argument in BoxParentData, so let's
@@ -223,12 +228,12 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix
   @override
   void attach(PipelineOwner owner) {
     super.attach(owner);
-    _offset.addListener(markNeedsPaint);
+    _offset.addListener(_hasScrolled);
   }
 
   @override
   void detach() {
-    _offset.removeListener(markNeedsPaint);
+    _offset.removeListener(_hasScrolled);
     super.detach();
   }
 

packages/flutter/test/material/tabs_test.dart

@@ -1038,6 +1038,56 @@ void main() {
     semantics.dispose();
   });
 
+  testWidgets('correct scrolling semantics', (WidgetTester tester) async {
+    final SemanticsTester semantics = new SemanticsTester(tester);
+
+    final List<Tab> tabs = new List<Tab>.generate(20, (int index) {
+      return new Tab(text: 'This is a very wide tab #$index');
+    });
+
+    final TabController controller = new TabController(
+      vsync: const TestVSync(),
+      length: tabs.length,
+      initialIndex: 0,
+    );
+
+    await tester.pumpWidget(
+      boilerplate(
+        child: new Semantics(
+          container: true,
+          child: new TabBar(
+            isScrollable: true,
+            controller: controller,
+            tabs: tabs,
+          ),
+        ),
+      ),
+    );
+
+    expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollLeft]));
+    expect(semantics, isNot(includesNodeWith(label: 'This is a very wide tab #10')));
+
+    controller.index = 10;
+    await tester.pumpAndSettle();
+
+    expect(semantics, isNot(includesNodeWith(label: 'This is a very wide tab #0')));
+    expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollLeft, SemanticsAction.scrollRight]));
+    expect(semantics, includesNodeWith(label: 'This is a very wide tab #10'));
+
+    controller.index = 19;
+    await tester.pumpAndSettle();
+
+    expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollRight]));
+
+    controller.index = 0;
+    await tester.pumpAndSettle();
+
+    expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollLeft]));
+    expect(semantics, includesNodeWith(label: 'This is a very wide tab #0'));
+
+    semantics.dispose();
+  });
+
   testWidgets('TabBar etc with zero tabs', (WidgetTester tester) async {
     final TabController controller = new TabController(
       vsync: const TestVSync(),