Skip to content

Commit

Permalink
Avoid accessing bounds of widget that has never been laid out (#424)
Browse files Browse the repository at this point in the history
Fixes a bug in visibility_detector.
  • Loading branch information
dnfield authored Sep 13, 2022
1 parent d30d18e commit 6bba637
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 5 deletions.
4 changes: 4 additions & 0 deletions packages/visibility_detector/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# CHANGELOG

## 0.4.0+2

* Fix a bug for updates to render objects that have not been laid out yet.

## 0.4.0+1

* Correct Flutter SDK version dependency to 3.1.0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,14 @@ mixin RenderVisibilityDetectorBase on RenderObject {
}
bool isFirstUpdate = _updates.isEmpty;
_updates[key] = () {
_fireCallback(layer, bounds);
if (bounds == null) {
// This can happen if set onVisibilityChanged was called with a non-null
// value but this render object has not been laid out. In that case,
// it has no size or geometry, and we should not worry about firing
// an update since it never has been visible.
return;
}
_fireCallback(layer, bounds!);
};
final updateInterval = VisibilityDetectorController.instance.updateInterval;
if (updateInterval == Duration.zero) {
Expand Down Expand Up @@ -228,7 +235,9 @@ mixin RenderVisibilityDetectorBase on RenderObject {

/// Used to get the bounds of the render object when it is time to update
/// clients about visibility.
Rect get bounds;
///
/// A null value means bounds are not available.
Rect? get bounds;

Matrix4? _lastPaintTransform;
Rect? _lastPaintClipBounds;
Expand Down Expand Up @@ -280,7 +289,7 @@ class RenderVisibilityDetector extends RenderProxyBox
final Key key;

@override
Rect get bounds => semanticBounds;
Rect? get bounds => hasSize ? semanticBounds : null;
}

/// The [RenderObject] corresponding to the [SliverVisibilityDetector] widget.
Expand All @@ -302,7 +311,11 @@ class RenderSliverVisibilityDetector extends RenderProxySliver
final Key key;

@override
Rect get bounds {
Rect? get bounds {
if (geometry == null) {
return null;
}

Size widgetSize;
Offset widgetOffset;
switch (applyGrowthDirectionToAxisDirection(
Expand Down
2 changes: 1 addition & 1 deletion packages/visibility_detector/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: visibility_detector
version: 0.4.0+1
version: 0.4.0+2
description: >
A widget that detects the visibility of its child and notifies a callback.
repository: https://github.com/google/flutter.widgets/tree/master/packages/visibility_detector
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,44 @@ void main() {

expect(detector.debugScheduleUpdateCount, 0);
});

testWidgets('RVS can schedule an update for a RO that is not laid out',
(WidgetTester tester) async {
final RenderVisibilityDetector detector = RenderVisibilityDetector(
key: Key('test'),
onVisibilityChanged: (_) {
fail('should not get called');
},
);

// Force an out of band update to get scheduled without laying out.
detector.onVisibilityChanged = (_) {
fail('This should also not get called');
};

expect(detector.debugScheduleUpdateCount, 1);

detector.dispose();
});

testWidgets(
'RVS (Sliver) can schedule an update for a RO that is not laid out',
(WidgetTester tester) async {
final RenderSliverVisibilityDetector detector =
RenderSliverVisibilityDetector(
key: Key('test'),
onVisibilityChanged: (_) {
fail('should not get called');
},
);

// Force an out of band update to get scheduled without laying out.
detector.onVisibilityChanged = (_) {
fail('This should also not get called');
};

expect(detector.debugScheduleUpdateCount, 1);

detector.dispose();
});
}

0 comments on commit 6bba637

Please sign in to comment.