Closed
Bug 1112956
Opened 10 years ago
Closed 10 years ago
Add an IProgressObserver interface to permit multiple kinds of observers for ProgressTracker
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: seth, Assigned: seth)
References
Details
Attachments
(1 file)
(deleted),
patch
|
tnikkel
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
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.
Assignee | ||
Comment 2•10 years ago
|
||
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)
Assignee | ||
Comment 3•10 years ago
|
||
Here's a try job:
https://tbpl.mozilla.org/?tree=Try&rev=c9ce7b25886d
Assignee | ||
Comment 4•10 years ago
|
||
Try got reset, but this was green on try.
Comment 5•10 years ago
|
||
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+
Assignee | ||
Comment 6•10 years ago
|
||
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. =)
Pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/07c75e0d6f67
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Assignee | ||
Comment 8•10 years ago
|
||
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.
Assignee | ||
Updated•10 years ago
|
Attachment #8538196 -
Flags: approval-mozilla-beta?
Updated•10 years ago
|
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
Updated•10 years ago
|
Attachment #8538196 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 9•10 years ago
|
||
Needs rebasing (or more deps getting approved).
Flags: needinfo?(seth)
Keywords: branch-patch-needed
Comment 10•10 years ago
|
||
I sorted it out. I accept payments in beer and whiskey.
https://hg.mozilla.org/releases/mozilla-beta/rev/842d25881e21
You need to log in
before you can comment on or make changes to this bug.
Description
•