Check for a null codec in MultiFrameImageStreamCompleter after calling _emitFrame - #165009

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…r calling _emitFrame (flutter/flutter#165009)

…g _emitFrame (flutter#165009)

_emitFrame invokes image listeners. A listener callback could remove all
of the MultiFrameImageStreamCompleter's listeners, resulting in a
_maybeDispose call that destroys the codec.

Fixes flutter#164944
Fixes flutter#164537

… when a listener is re-added (flutter#193575)

**Title:** Fix null `_nextFrame` in
`MultiFrameImageStreamCompleter._handleAppFrame` when a listener is
re-added

---

`MultiFrameImageStreamCompleter._handleAppFrame` can run after its
decoded
frame has been thrown away. When that happens it dereferences a null
`_nextFrame`. In debug builds this fails the `_nextFrame != null`
assertion; in
release builds it throws `Null check operator used on a null value`.

### How it happens

1. A frame is decoded and `_handleAppFrame` is scheduled as a frame
callback.
2. Before that frame runs, the stream drops to zero listeners and a
listener is
added back. `_ImageState` does this whenever `TickerMode` changes, for
example during a route push or pop. Something must keep the completer
alive
   while it has no listeners, as `ImageCache` does.
3. `removeListener` cancels only `_timer`. It does not cancel the
pending frame
   callback.
4. `addListener` calls `_decodeNextFrameAndSchedule()`. That
synchronously
disposes `_nextFrame` and sets it to null before it awaits the next
frame.
5. The pending `_handleAppFrame` then runs with listeners present and
   `_nextFrame == null`.

flutter#30346 fixed flutter#30340 by stopping `_handleAppFrame` from being registered
twice
(`_frameCallbackScheduled`). It did not handle the callback that is
already
pending when decoding restarts; that PR's test for this scenario was
merged
disabled. The same failure was reported again in flutter#122576.

### Fix

`_handleAppFrame` now returns early when `_nextFrame` is null. The
restarted
decode sets `_nextFrame` and calls `_scheduleAppFrame()` when it
completes.
`_frameCallbackScheduled` has already been reset at that point, so the
animation
resumes on the next frame. The only visible effect is one skipped
animation
frame instead of an exception. This follows the same pattern as the
`_codec == null` check added in flutter#165009.

We see this crash in production in several apps that treat framework
errors as
fatal. The trigger is animated images that are on screen during route
transitions.

### Tests

Adds `re-adding a listener while a frame callback is pending does not
crash`
to `image_stream_test.dart`. It reproduces the race without depending on
timing:

- Without the fix it fails with
  `Failed assertion: line 1064 pos 12: '_nextFrame != null'`.
- With the fix it passes, and the animation resumes once the restarted
decode
  completes.
- All other tests in `image_stream_test.dart` still pass.

Fixes flutter#30340
Related: flutter#122576, flutter#30346

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

<!-- Keep the checklist link references from the upstream PR template.
-->

… when a listener is re-added (flutter#193575)

**Title:** Fix null `_nextFrame` in
`MultiFrameImageStreamCompleter._handleAppFrame` when a listener is
re-added

---

`MultiFrameImageStreamCompleter._handleAppFrame` can run after its
decoded
frame has been thrown away. When that happens it dereferences a null
`_nextFrame`. In debug builds this fails the `_nextFrame != null`
assertion; in
release builds it throws `Null check operator used on a null value`.

### How it happens

1. A frame is decoded and `_handleAppFrame` is scheduled as a frame
callback.
2. Before that frame runs, the stream drops to zero listeners and a
listener is
added back. `_ImageState` does this whenever `TickerMode` changes, for
example during a route push or pop. Something must keep the completer
alive
   while it has no listeners, as `ImageCache` does.
3. `removeListener` cancels only `_timer`. It does not cancel the
pending frame
   callback.
4. `addListener` calls `_decodeNextFrameAndSchedule()`. That
synchronously
disposes `_nextFrame` and sets it to null before it awaits the next
frame.
5. The pending `_handleAppFrame` then runs with listeners present and
   `_nextFrame == null`.

flutter#30346 fixed flutter#30340 by stopping `_handleAppFrame` from being registered
twice
(`_frameCallbackScheduled`). It did not handle the callback that is
already
pending when decoding restarts; that PR's test for this scenario was
merged
disabled. The same failure was reported again in flutter#122576.

### Fix

`_handleAppFrame` now returns early when `_nextFrame` is null. The
restarted
decode sets `_nextFrame` and calls `_scheduleAppFrame()` when it
completes.
`_frameCallbackScheduled` has already been reset at that point, so the
animation
resumes on the next frame. The only visible effect is one skipped
animation
frame instead of an exception. This follows the same pattern as the
`_codec == null` check added in flutter#165009.

We see this crash in production in several apps that treat framework
errors as
fatal. The trigger is animated images that are on screen during route
transitions.

### Tests

Adds `re-adding a listener while a frame callback is pending does not
crash`
to `image_stream_test.dart`. It reproduces the race without depending on
timing:

- Without the fix it fails with
  `Failed assertion: line 1064 pos 12: '_nextFrame != null'`.
- With the fix it passes, and the animation resumes once the restarted
decode
  completes.
- All other tests in `image_stream_test.dart` still pass.

Fixes flutter#30340
Related: flutter#122576, flutter#30346

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

<!-- Keep the checklist link references from the upstream PR template.
-->