Created attachment 664939[details][diff][review]
Ugly patch that does the job
Passing around bools everywhere is ugly but the lack of enumerated types in xpcom makes the alternative bad too.
At this point I'm more interested in feedback on the behaviour change. Does this seem sane to everyone?

Comment on attachment 664939[details][diff][review]
Ugly patch that does the job
The bool-ness would be much nicer IMO if you did
RequestDecode(/* decodeSomeImmediately */ true)
As an alternative, having two methods seems even better to me.
RequestDecodeWithSomeSync()
RequestDecodeWithNoSync()
You can collapse them into one method with a bool arg in RasterImage.
> + NS_IMETHOD RequestDecode(bool decodeSome = true);
If we believe explicit is better than implicit [1], we should get rid of the default arg.
I guess I'd have to play around with the patch to judge whether it's an improvement. We synchronously decode a bit on draw, so we don't get a white rectangle for /every/ image on tab switch?
FYI, If you use git bz to attach your patches, it should automatically do -U8.
[1] http://www.python.org/dev/peps/pep-0020/

You didn't do this:
::: image/public/imgIRequest.idl
@@ +145,5 @@
> * container already exists, or calls it once it gets OnStartContainer if the
> * container does not yet exist.
> */
> void requestDecode();
> + void startDecoding();
change uuid and document the difference between these two
Also:
+ NS_IMETHOD RequestDecodeCore(RequestDecodeType aDecodeType);
This should just have an nsresult return type, since it's not part of the interface ("I" method).

(In reply to Joe Drew (:JOEDREW! \o/) from comment #9)
> OH. And I said this:
>
> > I am supportive of this. We should put it in at the beginning of a cycle, though, so we shake out
> > possible issues.
>
> So we should probably back this out.
I was planing on backing it out of Aurora when the switch happens instead of postponing landing so that we get more testing.

Do I suppose to see some improvement when opening page with tons of images?
I don't notice any, a page used to took 2GB RAM, which 99% of the photos is *not* visible, still took 2GB RAM in today's nightly

(In reply to dindog from comment #15)
> Do I suppose to see some improvement when opening page with tons of images?
> I don't notice any, a page used to took 2GB RAM, which 99% of the photos is
> *not* visible, still took 2GB RAM in today's nightly
That's not what this bug fixes, confusingly enough. You're looking for bug 689623.

How about don't discard visible images when switching to other tabs? since we can already identify whether images are visible, it will improve tab switching even more, only cost a little more memory.
Adding "image.mem.discard_visible" to "about:config", people can choose at will.

Created attachment 669640[details][diff][review]
back out of aurora
[Approval Request Comment]
Bug caused by (feature/regressing bug #): this bug
User impact if declined: Unfinished work, needs to be reverted so we can finish
Testing completed (on m-c, etc.): revert to pre-existing situation
Risk to taking this patch (and alternatives if risky): little bit jankier tab switch, i.e., what we have had all the time
String or UUID changes made by this patch: Changes imgIContainer and imgIRequest back to what they were prior to this patch

(In reply to Joe Drew (:JOEDREW! \o/) from comment #21)
> Created attachment 669640[details][diff][review]
> back out of aurora
>
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): this bug
> User impact if declined: Unfinished work, needs to be reverted so we can
> finish
> Testing completed (on m-c, etc.): revert to pre-existing situation
> Risk to taking this patch (and alternatives if risky): little bit jankier
> tab switch, i.e., what we have had all the time
> String or UUID changes made by this patch: Changes imgIContainer and
> imgIRequest back to what they were prior to this patch
By a little jankier, you mean 30-40% slower tab switch times as per http://is.gd/fPVXJY. This was a pretty awesome change.

(In reply to Olli Pettay [:smaug] from comment #25)
> Well, bug 799335 isn't necessarily wrong. I personally favor the current way
> faster tab switching
> over the never-paint-wrong-when-switch-to-a-tab behavior.
Hopefully, with the correct version of this patch we'll be able to get a good compromise between both of them.

(In reply to Taras Glek (:taras) from comment #23)
> By a little jankier, you mean 30-40% slower tab switch times as per
> http://is.gd/fPVXJY. This was a pretty awesome change.
Don't worry, this isn't going away long-term, only for 18 (it should be in 19). As I said in comment 4, though, this needs more testing and fixing before going in long-term; it's utterly broken right now, despite some people's preference for that behaviour. I can virtually guarantee that bug 799335 isn't the only thing that will need fixing.