Skip to content

Conversation

@ds84182
Copy link
Contributor

@ds84182 ds84182 commented Oct 28, 2020

Description

Saw a crash report this morning that showed that _checkDisposed was throwing a StateError within _decodeNextFrameAndSchedule.

Related Issue

Tests

I added the following tests:

  • Test that ensures there is no error thrown when an image finishes decoding after there are no listeners.
  • Updated test since transient callbacks are no longer registered after an image finishes decoding when there are no listeners. Before this change, another app frame would be scheduled, but that app frame does nothing.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

@ds84182 ds84182 requested a review from goderbauer October 28, 2020 18:14
@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Oct 28, 2020
@google-cla google-cla bot added the cla: yes label Oct 28, 2020
@goderbauer goderbauer requested a review from dnfield October 28, 2020 18:15
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nit

@ds84182
Copy link
Contributor Author

ds84182 commented Oct 28, 2020

The original change breaks obscured_animated_image_test, so now this only returns when the image has a single frame. I'm not exactly sure why it broke, but since this crash is only seen with single frame images I think moving the check is a bit safer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants