Commit 6b93efdd authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Make dependenciesChanged fire after initState (#5810)

This allows us to simplify the logic around inherited widgets e.g. in
the Image widget.
parent b298bdc8
...@@ -779,17 +779,14 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect ...@@ -779,17 +779,14 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect
_lastSelectedIndex = _selection.index; _lastSelectedIndex = _selection.index;
} }
@override
void initState() {
super.initState();
scrollBehavior.isScrollable = config.isScrollable;
}
@override @override
void didUpdateConfig(TabBar<T> oldConfig) { void didUpdateConfig(TabBar<T> oldConfig) {
super.didUpdateConfig(oldConfig); super.didUpdateConfig(oldConfig);
if (!config.isScrollable) if (config.isScrollable != oldConfig.isScrollable) {
scrollTo(0.0); scrollBehavior.isScrollable = config.isScrollable;
if (!config.isScrollable)
scrollTo(0.0);
}
} }
@override @override
...@@ -921,7 +918,10 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect ...@@ -921,7 +918,10 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect
} }
@override @override
ScrollBehavior<double, double> createScrollBehavior() => new _TabsScrollBehavior(); ScrollBehavior<double, double> createScrollBehavior() {
return new _TabsScrollBehavior()
..isScrollable = config.isScrollable;
}
@override @override
_TabsScrollBehavior get scrollBehavior => super.scrollBehavior; _TabsScrollBehavior get scrollBehavior => super.scrollBehavior;
......
...@@ -545,12 +545,12 @@ abstract class StatefulWidget extends Widget { ...@@ -545,12 +545,12 @@ abstract class StatefulWidget extends Widget {
/// Tracks the lifecycle of [State] objects when asserts are enabled. /// Tracks the lifecycle of [State] objects when asserts are enabled.
enum _StateLifecycle { enum _StateLifecycle {
/// The [State] object has been created but [State.initState] has not yet been /// The [State] object has been created. [State.initState] is called at this
/// called. /// time.
created, created,
/// The [State.initState] method has been called but the [State] object is /// The [State.initState] method has been called but the [State] object is
/// not yet ready to build. /// not yet ready to build. [State.dependenciesChanged] is called at this time.
initialized, initialized,
/// The [State] object is ready to build and [State.dispose] has not yet been /// The [State] object is ready to build and [State.dispose] has not yet been
...@@ -594,6 +594,11 @@ typedef void StateSetter(VoidCallback fn); ...@@ -594,6 +594,11 @@ typedef void StateSetter(VoidCallback fn);
/// [BuildContext] or the widget, which are available as the [context] and /// [BuildContext] or the widget, which are available as the [context] and
/// [config] properties, respectively, when the [initState] method is /// [config] properties, respectively, when the [initState] method is
/// called. /// called.
/// * The framework calls [dependenciesChanged]. Subclasses of [State] should
/// override [dependenciesChanged] to perform initialization involving
/// [InheritedWidget]s. If [BuildContext.inheritFromWidgetOfExactType] is
/// called, the [dependenciesChanged] method will be called again if the
/// inherited widgets subsequently change or if the widget moves in the tree.
/// * At this point, the [State] object is fully initialized and the framework /// * At this point, the [State] object is fully initialized and the framework
/// might call its [build] method any number of times to obtain a /// might call its [build] method any number of times to obtain a
/// description of the user interface for this subtree. [State] objects can /// description of the user interface for this subtree. [State] objects can
...@@ -704,13 +709,17 @@ abstract class State<T extends StatefulWidget> { ...@@ -704,13 +709,17 @@ abstract class State<T extends StatefulWidget> {
/// The framework will call this method exactly once for each [State] object /// The framework will call this method exactly once for each [State] object
/// it creates. /// it creates.
/// ///
/// You cannot use [BuildContext.inheritFromWidgetOfExactType] from this
/// method. However, [dependenciesChanged] will be called immediately
/// following this method, and [BuildContext.inheritFromWidgetOfExactType] can
/// be used there.
///
/// If you override this, make sure your method starts with a call to /// If you override this, make sure your method starts with a call to
/// super.initState(). /// super.initState().
@protected @protected
@mustCallSuper @mustCallSuper
void initState() { void initState() {
assert(_debugLifecycleState == _StateLifecycle.created); assert(_debugLifecycleState == _StateLifecycle.created);
assert(() { _debugLifecycleState = _StateLifecycle.initialized; return true; });
} }
/// Called whenever the configuration changes. /// Called whenever the configuration changes.
...@@ -804,7 +813,7 @@ abstract class State<T extends StatefulWidget> { ...@@ -804,7 +813,7 @@ abstract class State<T extends StatefulWidget> {
if (_debugLifecycleState == _StateLifecycle.defunct) { if (_debugLifecycleState == _StateLifecycle.defunct) {
throw new FlutterError( throw new FlutterError(
'setState() called after dispose(): $this\n' 'setState() called after dispose(): $this\n'
'This error happens if you call setState() on State object for a widget that ' 'This error happens if you call setState() on a State object for a widget that '
'no longer appears in the widget tree (e.g., whose parent widget no longer ' 'no longer appears in the widget tree (e.g., whose parent widget no longer '
'includes the widget in its build). This error can occur when code calls ' 'includes the widget in its build). This error can occur when code calls '
'setState() from a timer or an animation callback. The preferred solution is ' 'setState() from a timer or an animation callback. The preferred solution is '
...@@ -812,11 +821,10 @@ abstract class State<T extends StatefulWidget> { ...@@ -812,11 +821,10 @@ abstract class State<T extends StatefulWidget> {
'callback. Another solution is to check the "mounted" property of this ' 'callback. Another solution is to check the "mounted" property of this '
'object before calling setState() to ensure the object is still in the ' 'object before calling setState() to ensure the object is still in the '
'tree.\n' 'tree.\n'
'\n'
'This error might indicate a memory leak if setState() is being called ' 'This error might indicate a memory leak if setState() is being called '
'because another object is retaining a reference to this State object ' 'because another object is retaining a reference to this State object '
'after it has been removed from the tree. To avoid memory leaks, ' 'after it has been removed from the tree. To avoid memory leaks, '
'consider breaking the reference to this object during dipose().' 'consider breaking the reference to this object during dispose().'
); );
} }
return true; return true;
...@@ -921,6 +929,9 @@ abstract class State<T extends StatefulWidget> { ...@@ -921,6 +929,9 @@ abstract class State<T extends StatefulWidget> {
/// [InheritedWidget] that later changed, the framework would call this /// [InheritedWidget] that later changed, the framework would call this
/// method to notify this object about the change. /// method to notify this object about the change.
/// ///
/// This method is also called immediately after [initState]. It is safe to
/// call [BuildContext.inheritFromWidgetOfExactType] from this method.
///
/// Subclasses rarely override this method because the framework always /// Subclasses rarely override this method because the framework always
/// calls [build] after a dependency changes. Some subclasses do override /// calls [build] after a dependency changes. Some subclasses do override
/// this method because they need to do some expensive work (e.g., network /// this method because they need to do some expensive work (e.g., network
...@@ -2278,20 +2289,18 @@ abstract class BuildableElement extends Element { ...@@ -2278,20 +2289,18 @@ abstract class BuildableElement extends Element {
@override @override
void dependenciesChanged() { void dependenciesChanged() {
super.dependenciesChanged(); super.dependenciesChanged();
assert(_active); assert(_active); // otherwise markNeedsBuild is a no-op
markNeedsBuild(); markNeedsBuild();
} }
@override @override
void activate() { void activate() {
final bool shouldRebuild = ((_dependencies != null && _dependencies.length > 0) || _hadUnsatisfiedDependencies); final bool hadDependencies = ((_dependencies != null && _dependencies.length > 0) || _hadUnsatisfiedDependencies);
super.activate(); // clears _dependencies, and sets active to true super.activate(); // clears _dependencies, and sets active to true
if (_dirty && !_inDirtyList) { if (_dirty && !_inDirtyList)
owner.scheduleBuildFor(this); owner.scheduleBuildFor(this);
} else if (shouldRebuild) { if (hadDependencies)
assert(_active); // otherwise markNeedsBuild is a no-op dependenciesChanged();
markNeedsBuild();
}
} }
@override @override
...@@ -2444,15 +2453,8 @@ class StatefulElement extends ComponentElement { ...@@ -2444,15 +2453,8 @@ class StatefulElement extends ComponentElement {
} finally { } finally {
_debugSetAllowIgnoredCallsToMarkNeedsBuild(false); _debugSetAllowIgnoredCallsToMarkNeedsBuild(false);
} }
assert(() { assert(() { _state._debugLifecycleState = _StateLifecycle.initialized; return true; });
if (_state._debugLifecycleState == _StateLifecycle.initialized) _state.dependenciesChanged();
return true;
throw new FlutterError(
'${_state.runtimeType}.initState failed to call super.initState.\n'
'initState() implementations must always call their superclass initState() method, to ensure '
'that the entire widget is initialized correctly.'
);
});
assert(() { _state._debugLifecycleState = _StateLifecycle.ready; return true; }); assert(() { _state._debugLifecycleState = _StateLifecycle.ready; return true; });
super._firstBuild(); super._firstBuild();
} }
...@@ -2512,17 +2514,40 @@ class StatefulElement extends ComponentElement { ...@@ -2512,17 +2514,40 @@ class StatefulElement extends ComponentElement {
@override @override
InheritedWidget inheritFromWidgetOfExactType(Type targetType) { InheritedWidget inheritFromWidgetOfExactType(Type targetType) {
assert(() { assert(() {
if (state._debugLifecycleState == _StateLifecycle.ready) if (state._debugLifecycleState == _StateLifecycle.created) {
return true; throw new FlutterError(
throw new FlutterError( 'inheritFromWidgetOfExactType($targetType) was called before ${_state.runtimeType}.initState() completed.\n'
"inheritFromWidgetOfExactType($targetType) was called before ${_state.runtimeType}.initState() completed.\n" 'When an inherited widget changes, for example if the value of Theme.of() changes, '
"When an inherited widget changes, for example if the value of Theme.of() changes, " 'its dependent widgets are rebuilt. If the dependent widget\'s reference to '
"its dependent widgets are rebuilt. If the dependent widget's reference to " 'the inherited widget is in a constructor or an initState() method, '
"the inherited widget is in a constructor or an initState() method, " 'then the rebuilt dependent widget will not reflect the changes in the '
"then the rebuilt dependent widget will not reflect the changes in the " 'inherited widget.\n'
"inherited widget.\n" 'Typically references to to inherited widgets should occur in widget build() methods. Alternatively, '
"Typically references to to inherited widgets should occur in widget build() methods.\n" 'initialization based on inherited widgets can be placed in the dependenciesChanged method, which '
); 'is called after initState and whenever the dependencies change thereafter.'
);
}
if (state._debugLifecycleState == _StateLifecycle.defunct) {
throw new FlutterError(
'inheritFromWidgetOfExactType($targetType) called after dispose(): $this\n'
'This error happens if you call inheritFromWidgetOfExactType() on the '
'BuildContext for a widget that no longer appears in the widget tree '
'(e.g., whose parent widget no longer includes the widget in its '
'build). This error can occur when code calls '
'inheritFromWidgetOfExactType() from a timer or an animation callback. '
'The preferred solution is to cancel the timer or stop listening to the '
'animation in the dispose() callback. Another solution is to check the '
'"mounted" property of this object before calling '
'inheritFromWidgetOfExactType() to ensure the object is still in the '
'tree.\n'
'This error might indicate a memory leak if '
'inheritFromWidgetOfExactType() is being called because another object '
'is retaining a reference to this State object after it has been '
'removed from the tree. To avoid memory leaks, consider breaking the '
'reference to this object during dispose().'
);
}
return true;
}); });
return super.inheritFromWidgetOfExactType(targetType); return super.inheritFromWidgetOfExactType(targetType);
} }
......
...@@ -186,30 +186,21 @@ class _ImageState extends State<Image> { ...@@ -186,30 +186,21 @@ class _ImageState extends State<Image> {
ImageStream _imageStream; ImageStream _imageStream;
ImageInfo _imageInfo; ImageInfo _imageInfo;
@override
void didUpdateConfig(Image oldConfig) {
if (config.image != oldConfig.image)
_resolveImage();
}
@override
void deactivate() {
// If this image is activated again then force _imageStream to be recreated,
// in case the InheritedWidget ancestors it depends on have changed.
_imageStream?.removeListener(_handleImageChanged);
_imageStream = null;
super.deactivate();
}
@override @override
void dependenciesChanged() { void dependenciesChanged() {
_resolveImage(); _resolveImage();
super.dependenciesChanged(); super.dependenciesChanged();
} }
@override
void didUpdateConfig(Image oldConfig) {
if (config.image != oldConfig.image)
_resolveImage();
}
@override @override
void reassemble() { void reassemble() {
_resolveImage(); _resolveImage(); // in case the image cache was flushed
super.reassemble(); super.reassemble();
} }
...@@ -235,16 +226,14 @@ class _ImageState extends State<Image> { ...@@ -235,16 +226,14 @@ class _ImageState extends State<Image> {
} }
@override @override
Widget build(BuildContext context) { void dispose() {
// This one-time initialization could have been done in initState() since assert(_imageStream != null);
// changes to the inherited widgets that _resolveImage depends on, notably _imageStream.removeListener(_handleImageChanged);
// DefaultAssetBundle, are handle by the dependenciesChanged() method. We're super.dispose();
// doing it here instead to avoid the assert that disallows references to }
// inherited widgets at initState() time. We've found that assert to be a
// reliable source of real bugs, and that it is worth this minor inconvenience.
if (_imageStream == null)
_resolveImage();
@override
Widget build(BuildContext context) {
return new RawImage( return new RawImage(
image: _imageInfo?.image, image: _imageInfo?.image,
width: config.width, width: config.width,
...@@ -257,4 +246,11 @@ class _ImageState extends State<Image> { ...@@ -257,4 +246,11 @@ class _ImageState extends State<Image> {
centerSlice: config.centerSlice centerSlice: config.centerSlice
); );
} }
@override
void debugFillDescription(List<String> description) {
super.debugFillDescription(description);
description.add('stream: $_imageStream');
description.add('pixels: $_imageInfo');
}
} }
...@@ -348,15 +348,14 @@ class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -348,15 +348,14 @@ class ScrollableState<T extends Scrollable> extends State<T> {
/// Scroll behaviors control where the boundaries of the scrollable are placed /// Scroll behaviors control where the boundaries of the scrollable are placed
/// and how the scrolling physics should behave near those boundaries and /// and how the scrolling physics should behave near those boundaries and
/// after the user stops directly manipulating the scrollable. /// after the user stops directly manipulating the scrollable.
ExtentScrollBehavior get scrollBehavior { ExtentScrollBehavior get scrollBehavior => _scrollBehavior;
return _scrollBehavior ??= createScrollBehavior();
}
ExtentScrollBehavior _scrollBehavior; ExtentScrollBehavior _scrollBehavior;
/// Use the value returned by [ScrollConfiguration.createScrollBehavior]. /// Use the value returned by [ScrollConfiguration.createScrollBehavior].
/// If this widget doesn't have a ScrollConfiguration ancestor, /// If this widget doesn't have a ScrollConfiguration ancestor,
/// or its createScrollBehavior callback is null, then return a new instance /// or its createScrollBehavior callback is null, then return a new instance
/// of [OverscrollWhenScrollableBehavior]. /// of [OverscrollWhenScrollableBehavior].
@protected
ExtentScrollBehavior createScrollBehavior() { ExtentScrollBehavior createScrollBehavior() {
return ScrollConfiguration.of(context)?.createScrollBehavior(); return ScrollConfiguration.of(context)?.createScrollBehavior();
} }
......
...@@ -286,6 +286,18 @@ void main() { ...@@ -286,6 +286,18 @@ void main() {
expect(imageProvider._configuration.devicePixelRatio, 10.0); expect(imageProvider._configuration.devicePixelRatio, 10.0);
}); });
testWidgets('Verify Image stops listening to ImageStream', (WidgetTester tester) async {
TestImageProvider imageProvider = new TestImageProvider();
await tester.pumpWidget(new Image(image: imageProvider));
State<Image> image = tester.state/*State<Image>*/(find.byType(Image));
expect(image.toString(), matches(new RegExp(r'_ImageState\([0-9]+; stream: ImageStream\(OneFrameImageStreamCompleter; unresolved; 1 listener\); pixels: null\)')));
imageProvider.complete();
await tester.pump();
expect(image.toString(), matches(new RegExp(r'_ImageState\([0-9]+; stream: ImageStream\(OneFrameImageStreamCompleter; \[100×100\] @ 1\.0x; 1 listener\); pixels: \[100×100\] @ 1\.0x\)')));
await tester.pumpWidget(new Container());
expect(image.toString(), matches(new RegExp(r'_ImageState\([0-9]+; _StateLifecycle.defunct; not mounted; stream: ImageStream\(OneFrameImageStreamCompleter; \[100×100\] @ 1\.0x; 0 listeners\); pixels: \[100×100\] @ 1\.0x\)')));
});
} }
class TestImageProvider extends ImageProvider<TestImageProvider> { class TestImageProvider extends ImageProvider<TestImageProvider> {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment