Link prefetching needs to open its channel with the correct loadgroup for the document it belongs to

RESOLVED FIXED in Firefox 18

Status

()

defect
RESOLVED FIXED
7 years ago
5 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

Trunk
mozilla18
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox18+ fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Boris, can nsPrefetchNode::OpenChannel use the load group for mSource->OwnerDoc()?
I'm 99.9% sure mSource is always null in practice.

If that were fixed, then yes, it could.
(In reply to comment #1)
> I'm 99.9% sure mSource is always null in practice.
> 
> If that were fixed, then yes, it could.

I'm new to this code, but as far as I can tell, prefetching is triggered from here <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#834>, and the only call site which passes in null is: <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#691>, which seems to be triggered from here: <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#353>.  Does the link element exist when that code is called?  If not, I don't see how this can be (easily) fixed...
Hmm, we could also modify the prefetch service to accept a source document, and just grab the load context from the document directly.  Would that be a better solution?
Oh, we no longer go through ProcessLink for <link> elements, apparently!

In that case, the only case mSource will be null in is an HTTP <link> header.  In which case, we should arguably pass in the document node as the source node.  And then use the loadgroup of mSource->OwnerDoc().
Attachment #659047 - Flags: review? → review?(bzbarsky)
Comment on attachment 659038 [details] [diff] [review]
Part 1: When prefetching a document using a Link header, use the document element as the source node

I'd prefer it if we changed PrefetchHref to take an nsINode and passed in mDocument here, perhaps.  But this is OK too.
Attachment #659038 - Flags: review?(bzbarsky) → review+
Comment on attachment 659047 [details] [diff] [review]
Part 2: Use the load group of the document that the source node belongs to when prefetching links

I think if !source we should just not prefetch.  Thoughts?
(In reply to Boris Zbarsky (:bz) from comment #8)
> Comment on attachment 659047 [details] [diff] [review]
> Part 2: Use the load group of the document that the source node belongs to
> when prefetching links
> 
> I think if !source we should just not prefetch.  Thoughts?

Hmm, that makes sense.  I'll do that in a new version of this patch.
Attachment #659047 - Attachment is obsolete: true
Attachment #659047 - Flags: review?(bzbarsky)
Attachment #659297 - Flags: review?(bzbarsky)
Comment on attachment 659297 [details] [diff] [review]
Part 2: Use the load group of the document that the source node belongs to when prefetching links

r=me
Attachment #659297 - Flags: review?(bzbarsky) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.