Skip to content

Commit ce6d671

Browse files
committed
Unsubscribe from chunkEvents
1 parent ad4dea3 commit ce6d671

File tree

3 files changed

+92
-26
lines changed

3 files changed

+92
-26
lines changed

cached_network_image/lib/src/image_provider/cached_network_image_provider.dart

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ class CachedNetworkImageProvider
7777
ImageStreamCompleter load(
7878
image_provider.CachedNetworkImageProvider key, DecoderCallback decode) {
7979
final chunkEvents = StreamController<ImageChunkEvent>();
80-
final imageStreamCompleter = MultiImageStreamCompleter(
80+
return MultiImageStreamCompleter(
8181
codec: _loadAsync(key, chunkEvents, decode),
8282
chunkEvents: chunkEvents.stream,
8383
scale: key.scale,
@@ -89,10 +89,6 @@ class CachedNetworkImageProvider
8989
);
9090
},
9191
);
92-
93-
_handleChunkEventsClose(imageStreamCompleter, chunkEvents);
94-
95-
return imageStreamCompleter;
9692
}
9793

9894
@Deprecated(
@@ -122,7 +118,7 @@ class CachedNetworkImageProvider
122118
ImageStreamCompleter loadBuffer(image_provider.CachedNetworkImageProvider key,
123119
DecoderBufferCallback decode) {
124120
final chunkEvents = StreamController<ImageChunkEvent>();
125-
final imageStreamCompleter = MultiImageStreamCompleter(
121+
return MultiImageStreamCompleter(
126122
codec: _loadBufferAsync(key, chunkEvents, decode),
127123
chunkEvents: chunkEvents.stream,
128124
scale: key.scale,
@@ -134,10 +130,6 @@ class CachedNetworkImageProvider
134130
);
135131
},
136132
);
137-
138-
_handleChunkEventsClose(imageStreamCompleter, chunkEvents);
139-
140-
return imageStreamCompleter;
141133
}
142134

143135
Stream<ui.Codec> _loadBufferAsync(
@@ -172,21 +164,6 @@ class CachedNetworkImageProvider
172164
return false;
173165
}
174166

175-
void _handleChunkEventsClose(
176-
ImageStreamCompleter imageStreamCompleter,
177-
StreamController<ImageChunkEvent> chunkEvents,
178-
) {
179-
// Make sure to close the chunk events controller after
180-
// the image stream disposes. Otherwise reporting an image chunk
181-
// event could fail beacause the ImageStream has been disposed.
182-
// (https://github.com/Baseflow/flutter_cached_network_image/issues/785)
183-
imageStreamCompleter.addOnLastListenerRemovedCallback(() {
184-
if (!chunkEvents.isClosed) {
185-
chunkEvents.close();
186-
}
187-
});
188-
}
189-
190167
@override
191168
int get hashCode => Object.hash(cacheKey ?? url, scale, maxHeight, maxWidth);
192169

cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class MultiImageStreamCompleter extends ImageStreamCompleter {
3838
);
3939
});
4040
if (chunkEvents != null) {
41-
chunkEvents.listen(
41+
_chunkSubscription = chunkEvents.listen(
4242
reportImageChunkEvent,
4343
onError: (dynamic error, StackTrace stack) {
4444
reportError(
@@ -65,10 +65,17 @@ class MultiImageStreamCompleter extends ImageStreamCompleter {
6565
// How many frames have been emitted so far.
6666
int _framesEmitted = 0;
6767
Timer? _timer;
68+
StreamSubscription<ImageChunkEvent>? _chunkSubscription;
6869

6970
// Used to guard against registering multiple _handleAppFrame callbacks for the same frame.
7071
bool _frameCallbackScheduled = false;
7172

73+
/// We must avoid disposing a completer if it has never had a listener, even
74+
/// if all [keepAlive] handles get disposed.
75+
bool __hadAtLeastOneListener = false;
76+
77+
bool __disposed = false;
78+
7279
void _switchToNewCodec() {
7380
_framesEmitted = 0;
7481
_timer = null;
@@ -159,6 +166,7 @@ class MultiImageStreamCompleter extends ImageStreamCompleter {
159166

160167
@override
161168
void addListener(ImageStreamListener listener) {
169+
__hadAtLeastOneListener = true;
162170
if (!hasListeners && _codec != null) _decodeNextFrameAndSchedule();
163171
super.addListener(listener);
164172
}
@@ -169,6 +177,56 @@ class MultiImageStreamCompleter extends ImageStreamCompleter {
169177
if (!hasListeners) {
170178
_timer?.cancel();
171179
_timer = null;
180+
__maybeDispose();
172181
}
173182
}
183+
184+
int __keepAliveHandles = 0;
185+
186+
@override
187+
ImageStreamCompleterHandle keepAlive() {
188+
final delegateHandle = super.keepAlive();
189+
return _MultiImageStreamCompleterHandle(this, delegateHandle);
190+
}
191+
192+
void __maybeDispose() {
193+
if (!__hadAtLeastOneListener ||
194+
__disposed ||
195+
hasListeners ||
196+
__keepAliveHandles != 0) {
197+
return;
198+
}
199+
200+
__disposed = true;
201+
202+
_chunkSubscription?.onData(null);
203+
_chunkSubscription?.cancel();
204+
_chunkSubscription = null;
205+
}
206+
}
207+
208+
class _MultiImageStreamCompleterHandle implements ImageStreamCompleterHandle {
209+
_MultiImageStreamCompleterHandle(this._completer, this._delegateHandle) {
210+
_completer!.__keepAliveHandles += 1;
211+
}
212+
213+
MultiImageStreamCompleter? _completer;
214+
final ImageStreamCompleterHandle _delegateHandle;
215+
216+
/// Call this method to signal the [ImageStreamCompleter] that it can now be
217+
/// disposed when its last listener drops.
218+
///
219+
/// This method must only be called once per object.
220+
@override
221+
void dispose() {
222+
assert(_completer != null);
223+
assert(_completer!.__keepAliveHandles > 0);
224+
assert(!_completer!.__disposed);
225+
226+
_delegateHandle.dispose();
227+
228+
_completer!.__keepAliveHandles -= 1;
229+
_completer!.__maybeDispose();
230+
_completer = null;
231+
}
174232
}

cached_network_image/test/image_stream_completer_test.dart

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,37 @@ void main() {
9696
expect(tester.takeException(), 'failure message');
9797
});
9898

99+
test('Completer unsubscribes to chunk events when disposed', () async {
100+
final codecStream = StreamController<Codec>();
101+
final chunkStream = StreamController<ImageChunkEvent>();
102+
103+
final MultiImageStreamCompleter completer = MultiImageStreamCompleter(
104+
codec: codecStream.stream,
105+
scale: 1.0,
106+
chunkEvents: chunkStream.stream,
107+
);
108+
109+
expect(chunkStream.hasListener, true);
110+
111+
chunkStream.add(
112+
const ImageChunkEvent(cumulativeBytesLoaded: 1, expectedTotalBytes: 3));
113+
114+
final ImageStreamListener listener =
115+
ImageStreamListener((ImageInfo info, bool syncCall) {});
116+
// Cause the completer to dispose.
117+
completer.addListener(listener);
118+
completer.removeListener(listener);
119+
120+
expect(chunkStream.hasListener, false);
121+
122+
// The above expectation should cover this, but the point of this test is to
123+
// make sure the completer does not assert that it's disposed and still
124+
// receiving chunk events. Streams from the network can keep sending data
125+
// even after evicting an image from the cache, for example.
126+
chunkStream.add(
127+
const ImageChunkEvent(cumulativeBytesLoaded: 2, expectedTotalBytes: 3));
128+
});
129+
99130
testWidgets('Decoding starts when a listener is added after codec is ready',
100131
(WidgetTester tester) async {
101132
final codecStream = StreamController<Codec>();

0 commit comments

Comments
 (0)