Closed Bug 1067541 Opened 5 years ago Closed 5 years ago

Preloaded images keep downloading even if the actual image ends up having its load canceled

Categories

(Core :: DOM: Core & HTML, defect)

32 Branch
x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: a.derosa, Assigned: bzbarsky)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.120 Safari/537.36

Steps to reproduce:

The demo linked below replaces the src attribute of an image with a 1x1px transparent gif.

Demo: http://jsbin.com/diwefovikizi/1/edit?js,output


Actual results:

Firefox seems to be the only browser to complete the download of the old image ignoring the fact that the resource isn't needed anymore.


Expected results:

Stop the download of the old resource
Component: Untriaged → Networking
Product: Firefox → Core
Downloading looks like pretty random if I hit F5 or Ctrl+F5.
Summary: Firefox downloads of unneeded resources → Firefox downloads unneeded resources
This bug should really get more attention. I can confirm that this is happening. Additionally it seems to be a spec violation:
https://html.spec.whatwg.org/multipage/embedded-content.html#the-img-element:abort-the-image-request-11

Fixing this bug would make Firefox also more perfomant in conjunction with some typicall JS patterns like lazy image loading.

Additionally and due to the fact that this behavior was fixed/implemented now also in Chrome (and was there in Trident/IE for a long time), typicall JS behaviors can build upon this to do further improvements to page speed.

For example, if a page is using AJAX and the user changes to a different content a script can go trough all not yet loaded images and abort them (via. setting the src to a data uri), simply before changing the content. 

(For this it would be nice, if removing images from the dom without having any JS refernces anymore (i.e.: element.innerHTML = "") should also abort, but this last thing might be hard to distinguish from other erase operation, where the author might still have a reference.)
Component: Networking → DOM: Core & HTML
> Additionally it seems to be a spec violation:

Note that the spec was written basically based on our implementation (whether with good fidelity or not), so I wouldn't read too much into it one way or the other.

> Fixing this bug would make Firefox also more perfomant

It depends.  Some sites flip back and forth between two images rapidly (e.g. as the user movs the mouse), so if you cancel the load as soon as they flip you end up having to restart the loads over and over.

In any case, for the scenario described here the DOM does exactly what the spec currently says: when the pending request (the 1x1 thing in this case) is fully loaded, we tell imagelib to cancel the old (then-current) request and start showing the 1x1 thing, which stops being pending and becomes current.  

Then imagelib (in imgRequest::RemoveProxy) does:

207   if (statusTracker->ConsumerCount() == 0) {

in which case it cancels the request.  In this case, ConsumerCount is 1.  The one consumer is a mozilla::detail::WeakReference<imgRequestProxy> for a _different_ imgRequestProxy from the one we're removing.

So where is this other imgRequestProxy coming from?  It's hanging out in the document's mPreloadingImages list.  

It'd be nice to remove things from that list when we hit the real load, I guess. Seth, any bright ideas on how to do that?  I suppose we could switch mPreloadingImages to a hashtable based on URI or something.

> via. setting the src to a data uri

Per spec, you should be able to abort by just setting src to "", no?  Should work in Firefox, modulo the preloading thing above.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(seth)
Summary: Firefox downloads unneeded resources → Preloaded images keep downloading even if the actual image ends up having its load canceled
A hashtable based on URI seems like a good plan to me; I don't see an obvious cleaner way to do it. nsImageLoadingContent and ImageLoader could remove the appropriate entry from the hash table when they start a load.
Flags: needinfo?(seth)
Note to self: http://fiddle.jshell.net/nhkoyuy5/ has a testcase-like thing.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8526451 - Flags: review?(peterv) → review+
https://hg.mozilla.org/mozilla-central/rev/e7ae6e804f7c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
This issue, that I'm glad has been fixed, is similar to issue #1088473 (https://bugzilla.mozilla.org/show_bug.cgi?id=1088473) that hasn't been addressed.
You need to log in before you can comment on or make changes to this bug.