Closed Bug 714752 Opened 8 years ago Closed 8 years ago

Make imgIContainerObserver::FrameChanged take an imgIRequest*

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: khuey, Assigned: khuey)

References

Details

(Keywords: addon-compat, dev-doc-complete)

Attachments

(1 file)

Attached patch PatchSplinter Review
No description provided.
Attachment #585377 - Flags: superreview?
Attachment #585377 - Flags: review?(bobbyholley+bmo)
Attachment #585377 - Flags: superreview? → superreview?(joe)
> observer->FrameChanged(nsnull, this, &dirtyRect);

Is there a way we could get the imgIRequest object associated with this imgIContainer and pass it in here? It seems counter intuitive to me that we would get nsnull during some of these function calls from within RasterImage.
That's how the rest of these work.
Note that that call doesn't go outside of imagelib, that goes to imgRequestProxy.
I'm not a super-reviewer; I'll redirect to folk who are.
Attachment #585377 - Flags: superreview?(joe) → superreview?(roc)
Comment on attachment 585377 [details] [diff] [review]
Patch

>--- a/image/public/imgIContainerObserver.idl
>+++ b/image/public/imgIContainerObserver.idl

>  * @author Stuart Parmenter <pavlov@netscape.com>
>  * @version 0.1

Surely this should be version 0.2 now.

>--- a/layout/base/nsImageLoader.cpp
>+++ b/layout/base/nsImageLoader.cpp
>+NS_IMETHODIMP nsImageLoader::FrameChanged(imgIRequest *aRequest,
> 
>+  NS_ASSERTION(aRequest == mRequest, "This is a neat trick.");

Fatal?
(In reply to Ms2ger from comment #5)
> Comment on attachment 585377 [details] [diff] [review]
> Patch
> 
> >--- a/image/public/imgIContainerObserver.idl
> >+++ b/image/public/imgIContainerObserver.idl
> 
> >  * @author Stuart Parmenter <pavlov@netscape.com>
> >  * @version 0.1
> 
> Surely this should be version 0.2 now.

:-P

> >--- a/layout/base/nsImageLoader.cpp
> >+++ b/layout/base/nsImageLoader.cpp
> >+NS_IMETHODIMP nsImageLoader::FrameChanged(imgIRequest *aRequest,
> > 
> >+  NS_ASSERTION(aRequest == mRequest, "This is a neat trick.");
> 
> Fatal?

nsImageLoader is going away entirely in Bug 697230, so I didn't bother.
Comment on attachment 585377 [details] [diff] [review]
Patch

I think joe should look at this one way or another, so I'm switching over review.
Attachment #585377 - Flags: review?(bobbyholley+bmo) → review?(joe)
Attachment #585377 - Flags: superreview?(roc) → superreview+
Comment on attachment 585377 [details] [diff] [review]
Patch

Review of attachment 585377 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsImageLoader.cpp
@@ +223,5 @@
>      // We're in the middle of a paint anyway
>      return NS_OK;
>    }
>  
> +  NS_ASSERTION(aRequest == mRequest, "This is a neat trick.");

yeah, NS_ABORT_IF_FALSE is the usual in imagelib-related stuff.
Attachment #585377 - Flags: review?(joe) → review+
https://hg.mozilla.org/mozilla-central/rev/af6501ede378
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
These interfaces were not documented at all. I've added very rudimentary documentation that could use a lot of fleshing out, but these are documented for the purposes of this bug.

https://developer.mozilla.org/en/XPCOM_Interface_Reference/imgIContainerObserver
https://developer.mozilla.org/en/XPCOM_Interface_Reference/imgIDecoderObserver
https://developer.mozilla.org/en/XPCOM_Interface_Reference/imgIDecoder
https://developer.mozilla.org/en/XPCOM_Interface_Reference/imgIContainer

Also mentioned on Firefox 12 for developers.
You need to log in before you can comment on or make changes to this bug.