brandondiamond/is_fixed_extent_list_borked.dart

## is_fixed_extent_list_borked.dart
/////////////// Background ///////////////
/*

(The test case that appears below may do a better job of explaning than this wall of text :)

I recently noticed this calculation while investigating some framework code:
https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart#L235

There's an associated test case, and though it makes sense, I think it misses a potential issue:
https://github.com/flutter/flutter/blob/master/packages/flutter/test/widgets/slivers_evil_test.dart#L255

In particular, when we are attempting to reveal the children that no longer exist (i.e., via the second drag),
I believe there will be a failure if instead of shifting up by 400px, we shifted
the full 500px (i.e., revealing the item with index zero).

In theory, this would position the zero-index child at the top of the viewport (if it still existed).

Of course, since it doesn't exist, we compute an offset correction. But I can't figure out how that correction
currently takes into account the pixels that the zeroth item had previously consumed.

More specifically, since the offset correction is always <first missing index * extent>, and the first missing index
will be 3 [in the test, as written] regardless of whether the zeroth item is visible, I don't see how the correction
will be sufficient to push the first valid child to the top of the viewport.

(I'm also not sure how this correction accounts for items that are partially visible...)

Is this a bug? Or am I just confused about how the scroll offset correction behaves :)

The rest of this gist contains a simplified test case that illustrates my concern.
*/


/////////////// Actual test case /////////////////////

// Copyright 2016 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_test/flutter_test.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/physics.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

import 'dart:io';
import 'dart:typed_data';

void main() async {
    final fontData = File('assets/fonts/Roboto-Regular.ttf')
      .readAsBytes()
      .then((bytes) => ByteData.view(Uint8List.fromList(bytes).buffer));

  final fontLoader = FontLoader('Roboto')..addFont(fontData);
  await fontLoader.load();

  testWidgets('Removing offscreen items above and rescrolling does not crash', (WidgetTester tester) async {
    await tester.pumpWidget(MaterialApp(
      home: Scaffold(body: RepaintBoundary(child: CustomScrollView(
        cacheExtent: 0.0,
        slivers: <Widget>[
          SliverFixedExtentList(
            itemExtent: 100.0,
            delegate: SliverChildBuilderDelegate(
              (BuildContext context, int index) {
                return Container(
                  color: Colors.blue,
                  child: Text(index.toString()),
                );
              },
              childCount: 30,
            ),
          ),
        ],
      ))),
    ));

    // Screen is 600px high. Let's see how things look to start.
    await tester.pump();
    // (IMAGE) Initial state: https://imgur.com/c3ud4Ky
    await expectLater(find.byType(CustomScrollView),
                  matchesGoldenFile('mainInit.png'));

    // Move item with index=1 100px up screen (by scrolling down). It's now at the top. Let's confirm visually.
    await tester.drag(find.text('1'), const Offset(0.0, -100.0));
    await tester.pump();

    // (IMAGE) Scrolled-down: https://i.imgur.com/OJT2hHu.png
    await expectLater(find.byType(CustomScrollView),
                  matchesGoldenFile('mainScrollDown.png'));


    // Stop returning index=0 item.

    await tester.pumpWidget(MaterialApp(
      home: Scaffold(body: RepaintBoundary(child: CustomScrollView(
        cacheExtent: 0.0,
        slivers: <Widget>[
          SliverFixedExtentList(
            itemExtent: 100.0,
            delegate: SliverChildBuilderDelegate(
              (BuildContext context, int index) {
                if (index > 0) {
                  return Container(
                    color: Colors.blue,
                    child: Text(index.toString(), style: TextStyle(fontFamily: 'Roboto')),
                  );
                }
                return null;
              },
              childCount: 30,
            ),
          ),
        ],
      ))),
    ));

    // Nothing should look different at this point.
    // (IMAGE) Scrolled-down + index=0 removed: https://i.imgur.com/cO54fkZ.png
    await tester.pump();
    await expectLater(find.byType(CustomScrollView),
                  matchesGoldenFile('mainDeleteInitial.png'));

    // Now, move item with index=1 100px down screen (by scrolling up).
    // Previously, item with index=0 would have become visible. Because it has been removed, a correction should
    // be applied to avoid gaps in the viewport. However, things fall apart here (the correction is computed as 0px).
    await tester.drag(find.text('1'), const Offset(0.0, 100.0));
    await tester.pump();

    // (IMAGE) Scrolled up + index=0 removed: https://i.imgur.com/7h86PTQ.png
    await expectLater(find.byType(CustomScrollView),
                  matchesGoldenFile('mainScrollUp.png'));

/*
Note:
The assertion triggers in sliver_fixed_extent_list.dart; this calculation seems inadequate to handle the zeroth item
(and partially visible items).

235         geometry = SliverGeometry(scrollOffsetCorrection: index * itemExtent);

    ══╡ EXCEPTION CAUGHT BY RENDERING LIBRARY ╞═════════════════════════════════════════════════════════
The following assertion was thrown during performLayout():
'package:flutter/src/rendering/sliver.dart': Failed assertion: line 533 pos 15:
'scrollOffsetCorrection != 0.0': is not true.

Either the assertion indicates an error in the framework itself, or we should provide substantially
more information in this error message to help you determine and fix the underlying cause.
In either case, please report this assertion by filing a bug on GitHub:
  https://github.com/flutter/flutter/issues/new?template=BUG.md

Widget creation tracking is currently disabled. Enabling it enables improved error messages. It can
be enabled by passing `--track-widget-creation` to `flutter run` or `flutter test`.

When the exception was thrown, this was the stack:
#2      new SliverGeometry (package:flutter/src/rendering/sliver.dart:533:15)
#3      RenderSliverFixedExtentBoxAdaptor.performLayout (package:flutter/src/rendering/sliver_fixed_extent_list.dart:235:20)
#4      RenderObject.layout (package:flutter/src/rendering/object.dart:1716:7)
#5      RenderViewportBase.layoutChildSequence (package:flutter/src/rendering/viewport.dart:406:13)
#6      RenderViewport._attemptLayout (package:flutter/src/rendering/viewport.dart:1355:12)
#7      RenderViewport.performLayout (package:flutter/src/rendering/viewport.dart:1273:20)
#8      RenderObject._layoutWithoutResize (package:flutter/src/rendering/object.dart:1584:7)
#9      PipelineOwner.flushLayout (package:flutter/src/rendering/object.dart:844:18)
#10     AutomatedTestWidgetsFlutterBinding.drawFrame (package:flutter_test/src/binding.dart:974:23)
#11     RendererBinding._handlePersistentFrameCallback (package:flutter/src/rendering/binding.dart:279:5)
#12     SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1043:15)
#13     SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:982:9)
#14     AutomatedTestWidgetsFlutterBinding.pump.<anonymous closure> (package:flutter_test/src/binding.dart:869:9)
#17     TestAsyncUtils.guard (package:flutter_test/src/test_async_utils.dart:69:41)
#18     AutomatedTestWidgetsFlutterBinding.pump (package:flutter_test/src/binding.dart:856:27)
#19     WidgetTester.pump.<anonymous closure> (package:flutter_test/src/widget_tester.dart:340:53)
#22     TestAsyncUtils.guard (package:flutter_test/src/test_async_utils.dart:69:41)
#23     WidgetTester.pump (package:flutter_test/src/widget_tester.dart:340:27)
#24     main.<anonymous closure> (file:///usr/local/google/home/bdiamond/github/Junk/flutter/packages/flutter/test/widgets/slivers_evil_test.dart:280:18)
<asynchronous suspension>
#25     testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:121:25)
<asynchronous suspension>
#26     TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:695:19)
<asynchronous suspension>
#29     TestWidgetsFlutterBinding._runTest (package:flutter_test/src/binding.dart:678:14)
#30     AutomatedTestWidgetsFlutterBinding.runTest.<anonymous closure> (package:flutter_test/src/binding.dart:1047:24)
#36     AutomatedTestWidgetsFlutterBinding.runTest (package:flutter_test/src/binding.dart:1044:15)
#37     testWidgets.<anonymous closure> (package:flutter_test/src/widget_tester.dart:118:22)
#38     Declarer.test.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/declarer.dart:168:27)
<asynchronous suspension>
#39     Invoker.waitForOutstandingCallbacks.<anonymous closure> (package:test_api/src/backend/invoker.dart:250:15)
<asynchronous suspension>
#44     Invoker.waitForOutstandingCallbacks (package:test_api/src/backend/invoker.dart:247:5)
#45     Declarer.test.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/declarer.dart:166:33)
#50     Declarer.test.<anonymous closure> (package:test_api/src/backend/declarer.dart:165:13)
<asynchronous suspension>
#51     Invoker._onRun.<anonymous closure>.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/invoker.dart:400:25)
<asynchronous suspension>
#65     _Timer._runTimers (dart:isolate-patch/timer_impl.dart:382:19)
#66     _Timer._handleMessage (dart:isolate-patch/timer_impl.dart:416:5)
#67     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:172:12)
(elided 34 frames from class _AssertionError, class _FakeAsync, package dart:async, package dart:async-patch, and package stack_trace)

The following RenderObject was being processed when the exception was fired: RenderSliverFixedExtentList#253f3 relayoutBoundary=up1:
  needs compositing
  creator: SliverFixedExtentList ← Viewport ← IgnorePointer-[GlobalKey#c9840] ← Semantics ←
    _PointerListener ← Listener ← _GestureSemantics ←
    RawGestureDetector-[LabeledGlobalKey<RawGestureDetectorState>#84e49] ← _PointerListener ← Listener
    ← _ScrollableScope ← _ScrollSemantics-[GlobalKey#a820a] ← ⋯
  parentData: paintOffset=Offset(0.0, 0.0) (can use size)
  constraints: SliverConstraints(AxisDirection.down, GrowthDirection.forward, ScrollDirection.idle,
    scrollOffset: 0.0, remainingPaintExtent: 600.0, crossAxisExtent: 800.0, crossAxisDirection:
    AxisDirection.right, viewportMainAxisExtent: 600.0, remainingCacheExtent: 600.0 cacheOrigin: 0.0 )
  geometry: SliverGeometry(scrollExtent: 3000.0, paintExtent: 600.0, maxPaintExtent: 3000.0,
    hasVisualOverflow: true, cacheExtent: 600.0)
  currently live children: 1 to 5
This RenderObject had the following descendants (showing up to depth 5):
    child with index 1: RenderIndexedSemantics#e3c35
      child: RenderRepaintBoundary#bf97d
        child: RenderDecoratedBox#5bd96
          child: RenderParagraph#28521
            text: TextSpan
    child with index 2: RenderIndexedSemantics#b88b7
      child: RenderRepaintBoundary#51643
        child: RenderDecoratedBox#7f706
          child: RenderParagraph#16006
            text: TextSpan
    child with index 3: RenderIndexedSemantics#51913
      child: RenderRepaintBoundary#c9ce8
        child: RenderDecoratedBox#6a904
          child: RenderParagraph#41189
            text: TextSpan
    child with index 4: RenderIndexedSemantics#0fe5d
      child: RenderRepaintBoundary#4aee5
        child: RenderDecoratedBox#b3e18
          child: RenderParagraph#57806
            text: TextSpan
    child with index 5: RenderIndexedSemantics#9e189
      child: RenderRepaintBoundary#fd8fd
        child: RenderDecoratedBox#9232f
          child: RenderParagraph#76da4
            text: TextSpan
════════════════════════════════════════════════════════════════════════════════════════════════════
*/
  });
}
	/////////////// Background ///////////////
	/*

	(The test case that appears below may do a better job of explaning than this wall of text :)

	I recently noticed this calculation while investigating some framework code:
	https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart#L235

	There's an associated test case, and though it makes sense, I think it misses a potential issue:
	https://github.com/flutter/flutter/blob/master/packages/flutter/test/widgets/slivers_evil_test.dart#L255

	In particular, when we are attempting to reveal the children that no longer exist (i.e., via the second drag),
	I believe there will be a failure if instead of shifting up by 400px, we shifted
	the full 500px (i.e., revealing the item with index zero).

	In theory, this would position the zero-index child at the top of the viewport (if it still existed).

	Of course, since it doesn't exist, we compute an offset correction. But I can't figure out how that correction
	currently takes into account the pixels that the zeroth item had previously consumed.

	More specifically, since the offset correction is always <first missing index * extent>, and the first missing index
	will be 3 [in the test, as written] regardless of whether the zeroth item is visible, I don't see how the correction
	will be sufficient to push the first valid child to the top of the viewport.

	(I'm also not sure how this correction accounts for items that are partially visible...)

	Is this a bug? Or am I just confused about how the scroll offset correction behaves :)

	The rest of this gist contains a simplified test case that illustrates my concern.
	*/


	/////////////// Actual test case /////////////////////

	// Copyright 2016 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_test/flutter_test.dart';
	import 'package:flutter/rendering.dart';
	import 'package:flutter/physics.dart';
	import 'package:flutter/material.dart';
	import 'package:flutter/services.dart';

	import 'dart:io';
	import 'dart:typed_data';

	void main() async {
	final fontData = File('assets/fonts/Roboto-Regular.ttf')
	.readAsBytes()
	.then((bytes) => ByteData.view(Uint8List.fromList(bytes).buffer));

	final fontLoader = FontLoader('Roboto')..addFont(fontData);
	await fontLoader.load();

	testWidgets('Removing offscreen items above and rescrolling does not crash', (WidgetTester tester) async {
	await tester.pumpWidget(MaterialApp(
	home: Scaffold(body: RepaintBoundary(child: CustomScrollView(
	cacheExtent: 0.0,
	slivers: <Widget>[
	SliverFixedExtentList(
	itemExtent: 100.0,
	delegate: SliverChildBuilderDelegate(
	(BuildContext context, int index) {
	return Container(
	color: Colors.blue,
	child: Text(index.toString()),
	);
	},
	childCount: 30,
	),
	),
	],
	))),
	));

	// Screen is 600px high. Let's see how things look to start.
	await tester.pump();
	// (IMAGE) Initial state: https://imgur.com/c3ud4Ky
	await expectLater(find.byType(CustomScrollView),
	matchesGoldenFile('mainInit.png'));

	// Move item with index=1 100px up screen (by scrolling down). It's now at the top. Let's confirm visually.
	await tester.drag(find.text('1'), const Offset(0.0, -100.0));
	await tester.pump();

	// (IMAGE) Scrolled-down: https://i.imgur.com/OJT2hHu.png
	await expectLater(find.byType(CustomScrollView),
	matchesGoldenFile('mainScrollDown.png'));


	// Stop returning index=0 item.

	await tester.pumpWidget(MaterialApp(
	home: Scaffold(body: RepaintBoundary(child: CustomScrollView(
	cacheExtent: 0.0,
	slivers: <Widget>[
	SliverFixedExtentList(
	itemExtent: 100.0,
	delegate: SliverChildBuilderDelegate(
	(BuildContext context, int index) {
	if (index > 0) {
	return Container(
	color: Colors.blue,
	child: Text(index.toString(), style: TextStyle(fontFamily: 'Roboto')),
	);
	}
	return null;
	},
	childCount: 30,
	),
	),
	],
	))),
	));

	// Nothing should look different at this point.
	// (IMAGE) Scrolled-down + index=0 removed: https://i.imgur.com/cO54fkZ.png
	await tester.pump();
	await expectLater(find.byType(CustomScrollView),
	matchesGoldenFile('mainDeleteInitial.png'));

	// Now, move item with index=1 100px down screen (by scrolling up).
	// Previously, item with index=0 would have become visible. Because it has been removed, a correction should
	// be applied to avoid gaps in the viewport. However, things fall apart here (the correction is computed as 0px).
	await tester.drag(find.text('1'), const Offset(0.0, 100.0));
	await tester.pump();

	// (IMAGE) Scrolled up + index=0 removed: https://i.imgur.com/7h86PTQ.png
	await expectLater(find.byType(CustomScrollView),
	matchesGoldenFile('mainScrollUp.png'));

	/*
	Note:
	The assertion triggers in sliver_fixed_extent_list.dart; this calculation seems inadequate to handle the zeroth item
	(and partially visible items).

	235 geometry = SliverGeometry(scrollOffsetCorrection: index * itemExtent);

	══╡ EXCEPTION CAUGHT BY RENDERING LIBRARY ╞═════════════════════════════════════════════════════════
	The following assertion was thrown during performLayout():
	'package:flutter/src/rendering/sliver.dart': Failed assertion: line 533 pos 15:
	'scrollOffsetCorrection != 0.0': is not true.

	Either the assertion indicates an error in the framework itself, or we should provide substantially
	more information in this error message to help you determine and fix the underlying cause.
	In either case, please report this assertion by filing a bug on GitHub:
	https://github.com/flutter/flutter/issues/new?template=BUG.md

	Widget creation tracking is currently disabled. Enabling it enables improved error messages. It can
	be enabled by passing `--track-widget-creation` to `flutter run` or `flutter test`.

	When the exception was thrown, this was the stack:
	#2 new SliverGeometry (package:flutter/src/rendering/sliver.dart:533:15)
	#3 RenderSliverFixedExtentBoxAdaptor.performLayout (package:flutter/src/rendering/sliver_fixed_extent_list.dart:235:20)
	#4 RenderObject.layout (package:flutter/src/rendering/object.dart:1716:7)
	#5 RenderViewportBase.layoutChildSequence (package:flutter/src/rendering/viewport.dart:406:13)
	#6 RenderViewport._attemptLayout (package:flutter/src/rendering/viewport.dart:1355:12)
	#7 RenderViewport.performLayout (package:flutter/src/rendering/viewport.dart:1273:20)
	#8 RenderObject._layoutWithoutResize (package:flutter/src/rendering/object.dart:1584:7)
	#9 PipelineOwner.flushLayout (package:flutter/src/rendering/object.dart:844:18)
	#10 AutomatedTestWidgetsFlutterBinding.drawFrame (package:flutter_test/src/binding.dart:974:23)
	#11 RendererBinding._handlePersistentFrameCallback (package:flutter/src/rendering/binding.dart:279:5)
	#12 SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1043:15)
	#13 SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:982:9)
	#14 AutomatedTestWidgetsFlutterBinding.pump.<anonymous closure> (package:flutter_test/src/binding.dart:869:9)
	#17 TestAsyncUtils.guard (package:flutter_test/src/test_async_utils.dart:69:41)
	#18 AutomatedTestWidgetsFlutterBinding.pump (package:flutter_test/src/binding.dart:856:27)
	#19 WidgetTester.pump.<anonymous closure> (package:flutter_test/src/widget_tester.dart:340:53)
	#22 TestAsyncUtils.guard (package:flutter_test/src/test_async_utils.dart:69:41)
	#23 WidgetTester.pump (package:flutter_test/src/widget_tester.dart:340:27)
	#24 main.<anonymous closure> (file:///usr/local/google/home/bdiamond/github/Junk/flutter/packages/flutter/test/widgets/slivers_evil_test.dart:280:18)
	<asynchronous suspension>
	#25 testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:121:25)
	<asynchronous suspension>
	#26 TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:695:19)
	<asynchronous suspension>
	#29 TestWidgetsFlutterBinding._runTest (package:flutter_test/src/binding.dart:678:14)
	#30 AutomatedTestWidgetsFlutterBinding.runTest.<anonymous closure> (package:flutter_test/src/binding.dart:1047:24)
	#36 AutomatedTestWidgetsFlutterBinding.runTest (package:flutter_test/src/binding.dart:1044:15)
	#37 testWidgets.<anonymous closure> (package:flutter_test/src/widget_tester.dart:118:22)
	#38 Declarer.test.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/declarer.dart:168:27)
	<asynchronous suspension>
	#39 Invoker.waitForOutstandingCallbacks.<anonymous closure> (package:test_api/src/backend/invoker.dart:250:15)
	<asynchronous suspension>
	#44 Invoker.waitForOutstandingCallbacks (package:test_api/src/backend/invoker.dart:247:5)
	#45 Declarer.test.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/declarer.dart:166:33)
	#50 Declarer.test.<anonymous closure> (package:test_api/src/backend/declarer.dart:165:13)
	<asynchronous suspension>
	#51 Invoker._onRun.<anonymous closure>.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/invoker.dart:400:25)
	<asynchronous suspension>
	#65 _Timer._runTimers (dart:isolate-patch/timer_impl.dart:382:19)
	#66 _Timer._handleMessage (dart:isolate-patch/timer_impl.dart:416:5)
	#67 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:172:12)
	(elided 34 frames from class _AssertionError, class _FakeAsync, package dart:async, package dart:async-patch, and package stack_trace)

	The following RenderObject was being processed when the exception was fired: RenderSliverFixedExtentList#253f3 relayoutBoundary=up1:
	needs compositing
	creator: SliverFixedExtentList ← Viewport ← IgnorePointer-[GlobalKey#c9840] ← Semantics ←
	_PointerListener ← Listener ← _GestureSemantics ←
	RawGestureDetector-[LabeledGlobalKey<RawGestureDetectorState>#84e49] ← _PointerListener ← Listener
	← _ScrollableScope ← _ScrollSemantics-[GlobalKey#a820a] ← ⋯
	parentData: paintOffset=Offset(0.0, 0.0) (can use size)
	constraints: SliverConstraints(AxisDirection.down, GrowthDirection.forward, ScrollDirection.idle,
	scrollOffset: 0.0, remainingPaintExtent: 600.0, crossAxisExtent: 800.0, crossAxisDirection:
	AxisDirection.right, viewportMainAxisExtent: 600.0, remainingCacheExtent: 600.0 cacheOrigin: 0.0 )
	geometry: SliverGeometry(scrollExtent: 3000.0, paintExtent: 600.0, maxPaintExtent: 3000.0,
	hasVisualOverflow: true, cacheExtent: 600.0)
	currently live children: 1 to 5
	This RenderObject had the following descendants (showing up to depth 5):
	child with index 1: RenderIndexedSemantics#e3c35
	child: RenderRepaintBoundary#bf97d
	child: RenderDecoratedBox#5bd96
	child: RenderParagraph#28521
	text: TextSpan
	child with index 2: RenderIndexedSemantics#b88b7
	child: RenderRepaintBoundary#51643
	child: RenderDecoratedBox#7f706
	child: RenderParagraph#16006
	text: TextSpan
	child with index 3: RenderIndexedSemantics#51913
	child: RenderRepaintBoundary#c9ce8
	child: RenderDecoratedBox#6a904
	child: RenderParagraph#41189
	text: TextSpan
	child with index 4: RenderIndexedSemantics#0fe5d
	child: RenderRepaintBoundary#4aee5
	child: RenderDecoratedBox#b3e18
	child: RenderParagraph#57806
	text: TextSpan
	child with index 5: RenderIndexedSemantics#9e189
	child: RenderRepaintBoundary#fd8fd
	child: RenderDecoratedBox#9232f
	child: RenderParagraph#76da4
	text: TextSpan
	════════════════════════════════════════════════════════════════════════════════════════════════════
	*/
	});
	}