109156 - on browser reload more than one request will be sent to refetch an image

Status: RESOLVED DUPLICATE of bug 79020

(Core :: Graphics: ImageLib, defect)

Description

[Christopher Blizzard (:blizzard)]

Build is from Nov 08, 2001 off the tip.

If you have an image loaded in the browser and you use the reload button in the
browser to reload the image mozilla will make anywhere from 2 - 4 requests for
the same image, all at once.  Stick _that_ in your pipe and smoke it.

Comment 1

[Chris Waterson]

Is this _any_ image in _any_ page, or just image documents (e.g.,
http://www.foo.com/booty.gif)?

Comment 2

[Christopher Blizzard (:blizzard)]

I've only seen it on image only reloads but I haven't done much playing around
with it.

Comment 3

[Christopher Blizzard (:blizzard)]

So far I've only been able to reproduce this on image pages and it doesn't do it
every time.

Comment 4

[John Morrison]

There is a known (right?) bug with re-fetching _every_ referenced image in a
document when you do Shift-Reload.

For this bug, I can't get this to double-fetch when viewing an image-only and 
doing a reload, but I always get a double-fetch on a shift-reload.

Comment 5

[Christopher Blizzard (:blizzard)]

Attachment: HTTP trace

This is an HTTP trace of two requests to the web server from a single reload.

Comment 6

[Christopher Blizzard (:blizzard)]

Adding HTTP folks.

1024[8055fb8]: nsHttpTransaction::Cancel [this=420c8408 status=80004005]
1024[8055fb8]: ignoring cancel since transaction has already failed
[this=420c8408 mStatus=804b0002]

That's interesting.

Comment 7

[Matthias Versen [:Matti]]

related to bug 92722 ?

Comment 8

[Darin Fisher]

looks like someone is canceling the first channel with NS_BINDING_ABORTED, then
creating and AsyncOpen'ing another identical channel, and then canceling the
original channel _again_ with NS_ERROR_FAILURE.  an NS_BREAK added to
nsHttpTransaction::Cancel with XPCOM_DEBUG_BREAK=stack might help.

Comment 9

[Christopher Blizzard (:blizzard)]

What a heaping pile of dung.

Here's what's happening.  To set up this situation, load an image off the web so
that it's loaded in a browser window.  Then reload.  ( Forced reloads and
regular reloads both act incorrectly but in different manners as I will describe
below. )

If you start a load of an image from the URI loader, say you're just loading a
random URI, it eventually finds its way into |ImageListener::OnStartRequest|. 
The image listener will then call |imgLoader::LoadImageWithChannel| with the
channel as the argument as it was passed in.

The first thing that LoadImageWithChannel does is to check the image cache, by
the URI and if it's in the cache, it calls |Cancel| on the channel.  This is bad
because you can't tell from only the URI alone if the image that you have in the
image cache is the one that you want.  This basically treats all image requests
as non-idempotent.  If you have a channel you should let the channel handle the
caching and not the image library.  That's why we have the HTTP cache.  It does
all of this for you.  So, as an aside this code should be changed so that it is
either correct or removed entirely.  I would personally suggest leaving the
cache in one place and just removing it.

Once the ImageProxy is created and the OnStartRequest() continues you have to
notify the image request proxy that the request has been started.

Once that is done, you have to call |StartLayout()| on the nsImageDocument
(mDocument).  This is where the pain begins.  This calls down through the frame
constructors and creates a new frame.  When |Init| is called on the new
ImageFrame, it takes its URL and restarts the request.  It creates a new HTTP
request for the image that we just loaded without using the channel that was
handed to us!

Silly, silly.  So, the question is, how do I notify mDocument that it should
start laying out the image without triggering the creation of a new HTTP
request?  nsImageFrame::Init() blindly calls LoadImage() on itself.

Comment 10

[Christopher Blizzard (:blizzard)]

So it looks like after my changes that a use cache response will still go
through the frame creation machinery and eventually call back into the loader
through |imgLoader::LoadImage| and use the cached image so there's no win there.
 Force requests still generate an entire new HTTP request that gets the new image.

Isn't there a way to use the channel that's already passed in and not trigger a
new image load?  It seems strange to me that there isn't.

Comment 11

[Jeremy M. Dolan]

This is a dupe.
101165, 92722, 79020, or 91738, take your pick. Probably 91738, which was marked
a dupe of 79020, but it seems differant to me.

Also, one or more of the above is probably causing 76565.

Comment 12

[Bradley Baetz (:bbaetz)]

imagelib (ab)uses its own set of loadgroups, for the imgcache stuff.

Dupe of bug 79020 via bug 91738 (see my comment for why it is a dupe - its the
same code).

*** This bug has been marked as a duplicate of 79020 ***