1237405 - imgLoader::LoadImage doesn't forward the LOAD_ANONYMOUS flag

Status: NEW

(Core :: Graphics: ImageLib, defect, P3)

Description

[Lina Butler (ex-Mozilla)]

https://dxr.mozilla.org/mozilla-central/rev/9d6ffc7a08b6b47056eefe1e652710a3849adbf7/toolkit/system/gnome/nsAlertsIconListener.cpp#240-246 and https://dxr.mozilla.org/mozilla-central/rev/9d6ffc7a08b6b47056eefe1e652710a3849adbf7/widget/cocoa/OSXNotificationCenter.mm#381-389 pass the `LOAD_ANONYMOUS` flag when loading alert images in private browsing mode. But `LoadImage` doesn't pass that flag to the channel.

It *seems* like that's intentional, per this comment: https://dxr.mozilla.org/mozilla-central/rev/9d6ffc7a08b6b47056eefe1e652710a3849adbf7/image/imgLoader.cpp#2057-2059

(The alert callers also pass `null` for the load group, so maybe it's moot. But I just wanted to double-check).

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

This seems like a real bug to me.  I think LOAD_ANONYMOUS should always be honored if its set.  Maybe Jonas or Ehsan would have an opinion.

Comment 2

[Timothy Nikkel (:tnikkel)]

(In reply to Kit Cambridge [:kitcambridge] from comment #0)
> It *seems* like that's intentional, per this comment:
> https://dxr.mozilla.org/mozilla-central/rev/
> 9d6ffc7a08b6b47056eefe1e652710a3849adbf7/image/imgLoader.cpp#2057-2059

That comment was checked in on 2008-09-05 with http://hg.mozilla.org/mozilla-central/rev/44853b2a5fc2
The LOAD_ANONYMOUS flag was added on 2008-10-01 with http://hg.mozilla.org/mozilla-central/rev/ca957fbe0ff6
So the comment pre-dates the LOAD_ANONYMOUS flag by a month.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't really have an opinion here. It seems like LOAD_ANONYMOUS is passed to some other API (LoadImage), not set on a channel. That other API ignores it rather than set the LOAD_ANONYMOUS flag when it creates a channel.

Generally speaking though, if you create an API which creates channels, and then make that API take a loadFlags argument, it seems reasonable to expect that *all* flags set in that argument are set on the channel. LOAD_ANONYMOUS doesn't seem any more or less important than any other flag.

But again, I don't know LoadImage well enough to know if there's a strong reason to do something else here. But it certainly looks surprising to me.


Another comment is that LOAD_ANONYMOUS means that necko adds *no* cookies. That's not what you want for private browsing. Private browsing uses a separate (temporary) cookie jar, it does completely disable cookies.