Closed Bug 1112956 Opened 7 years ago Closed 7 years ago

Add an IProgressObserver interface to permit multiple kinds of observers for ProgressTracker


(Core :: ImageLib, defect)

Not set



Tracking Status
firefox36 --- fixed
firefox37 --- fixed


(Reporter: seth, Assigned: seth)




(1 file)

Right now, the only kind of observer ProgressTracker supports is imgRequestProxy. We'll need multiple kinds of observers to allow ImageWrappers to also wrap ProgressTracker notifications.

In the short term I plan to use this to reimplement multipart/x-mixed-replace image support in a way that's compatible with multiple decoders per image, but in the long term this will make it possible to solve some other ugly design problems - for example, imgIContainer::GetImageSpaceInvalidationRect shouldn't need to exist at all.
To clarify, GetImageSpaceInvalidationRect exists because ImageWrappers like OrientedImage and ClippedImage currently have no way of transforming the invalidation rect that accompanies FRAME_UPDATE notifications. It would be a lot less error-prone to handle that transparently instead of requiring all users of ImageWrappers to remember to call GetImageSpaceInvalidationRect.
Here's the patch. There are several files that are only touched because they weren't including some file that they needed, but the file happened to be included transitively by |imgRequestProxy.h|. Now that |imgRequestProxy.h| is included in fewer places, those issues had to be fixed.

I deliberately did not make this an XPCOM interface since it is private to ImageLib.
Attachment #8538196 - Flags: review?(tnikkel)
Blocks: 1112972
Try got reset, but this was green on try.
Comment on attachment 8538196 [details] [diff] [review]
Add IProgressObserver to permit more than one class to observe ProgressTracker

Does the cid of imgRequestProxy need to be bumped?
Attachment #8538196 - Flags: review?(tnikkel) → review+
Thanks for the review!

(In reply to Timothy Nikkel (:tn) from comment #5)
> Does the cid of imgRequestProxy need to be bumped?

I had no idea, so I checked in #content and I was told that you basically never need to bump CIDs. Hopefully that's right. =)

Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Approval Request Comment
[Feature/regressing bug #]: Unknown.
[User impact if declined]: MJPEG streams and webcams stutter and don't render correctly.
[Describe test coverage new/current, TreeHerder]: Already in Firefox 38 and 37.
[Risks and why]: Very low. This is pure refactoring, but bug 1112956, which actually fixes the issue, depends on it.
[String/UUID change made/needed]: None.
Attachment #8538196 - Flags: approval-mozilla-beta?
Attachment #8538196 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Needs rebasing (or more deps getting approved).
Flags: needinfo?(seth)
I sorted it out. I accept payments in beer and whiskey.
Flags: needinfo?(seth)
You need to log in before you can comment on or make changes to this bug.