Created method to report ImageChunkEvents #55044
Conversation
With this method custom ImageStreamCompleter implementations can also send these events.
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| } | ||
| } | ||
|
|
||
| /// Calls all the registered ImageChunkListeners (listeners with an |
There was a problem hiding this comment.
| /// Calls all the registered ImageChunkListeners (listeners with an | |
| /// Calls all the registered [ImageChunkListeners] (listeners with an |
There was a problem hiding this comment.
@goderbauer ImageChunkListener or ImageChunkListeners is not a class or variable or anything on it's own, so there is nothing to link to. Just like
If no error listeners (listeners with an [ImageStreamListener.onError] specified) are attached
I'll rewrite it to image chunk listeners, so it is clear that it is no class.
There was a problem hiding this comment.
https://master-api.flutter.dev/flutter/painting/ImageChunkListener.html is a typedef that can be linked to.
| listener(event); | ||
| } | ||
| } | ||
| reportImageChunkEvent(event); |
There was a problem hiding this comment.
You should be able to pass reportImageChunkEvent directly to listen without the need for an extra closure.
|
This PR will also need a test. |
|
@goderbauer I made an extra FakeEventReportingImageStreamCompleter to run the tests on for the event reporting and renamed the existing tests that run on the MultiFrameImageStreamCompleter implementation. |
| ); | ||
|
|
||
| imageStream.addListener(ImageStreamListener( | ||
| (ImageInfo image, bool synchronousCall) { }, |
There was a problem hiding this comment.
nit formatting: put this on the same indentation as the next line.
There was a problem hiding this comment.
@goderbauer There is no formatting tool for Flutter right?
| streamController.add(const ImageChunkEvent(cumulativeBytesLoaded: 1, expectedTotalBytes: 3)); | ||
| await tester.idle(); | ||
| imageStream.addListener(ImageStreamListener( | ||
| (ImageInfo image, bool synchronousCall) { }, |
|
This seems fine to me once @goderbauer's comments are resolved |
|
@dnfield and @goderbauer It can be merged now right? |
|
Yes, thanks! I've added a label that should cause it to get merged once some unrelated infra issues are resolved. |
With this method custom ImageStreamCompleter implementations can also send these events.
Fixes #55040
Description
As described in #55040 custom implementations of ImageStreamCompleter are currently unable to send report updates to the listeners. Only implementations within the same file are able to do so. The method is made in a very similar way as setImage and reportError.
Related Issues
Fixes issue #55040
Tests
I could use some help with this. This method is covered by existing tests in image_stream_test.dart, for example 'Chunk events are delivered'. So I'm not sure whether I need to add new tests or how they should be made. So I guess the coverage is still 100%, but maybe we need a more specific test.
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.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.