Last Comment Bug 714752 - Make imgIContainerObserver::FrameChanged take an imgIRequest*
: Make imgIContainerObserver::FrameChanged take an imgIRequest*
Status: RESOLVED FIXED
: addon-compat, dev-doc-complete
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: 697230
  Show dependency treegraph
 
Reported: 2012-01-03 04:50 PST by Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
Modified: 2012-04-19 12:58 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (27.52 KB, patch)
2012-01-03 04:50 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
joe: review+
roc: superreview+
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-03 04:50:04 PST
Created attachment 585377 [details] [diff] [review]
Patch
Comment 1 Scott Johnson (:jwir3) 2012-01-03 08:24:07 PST
> 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.
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-03 08:28:27 PST
That's how the rest of these work.
Comment 3 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-03 08:28:59 PST
Note that that call doesn't go outside of imagelib, that goes to imgRequestProxy.
Comment 4 Joe Drew (not getting mail) 2012-01-03 09:34:36 PST
I'm not a super-reviewer; I'll redirect to folk who are.
Comment 5 :Ms2ger (⌚ UTC+1/+2) 2012-01-03 09:40:00 PST
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?
Comment 6 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-03 09:46:04 PST
(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 7 Bobby Holley (:bholley) (busy with Stylo) 2012-01-03 10:33:46 PST
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.
Comment 8 Joe Drew (not getting mail) 2012-01-03 15:29:02 PST
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.
Comment 9 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-06 04:35:05 PST
https://hg.mozilla.org/mozilla-central/rev/af6501ede378
Comment 10 Eric Shepherd [:sheppy] 2012-04-19 12:58:48 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.