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

Status: RESOLVED FIXED

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

Description

[(no longer active)]

Boris, can nsPrefetchNode::OpenChannel use the load group for mSource->OwnerDoc()?

Comment 1

[Boris Zbarsky [:bzbarsky]]

I'm 99.9% sure mSource is always null in practice.

If that were fixed, then yes, it could.

Comment 2

[(no longer active)]

(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...

Comment 3

[(no longer active)]

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?

Comment 4

[Boris Zbarsky [:bzbarsky]]

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().

Comment 5

[(no longer active)]

Attachment: Part 1: When prefetching a document using a Link header, use the document element as the source node

Comment 6

[(no longer active)]

Attachment: Part 2: Use the load group of the document that the source node belongs to when prefetching links

Comment 7

[Boris Zbarsky [: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.

Comment 8

[Boris Zbarsky [:bzbarsky]]

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?

Comment 9

[(no longer active)]

(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.

Comment 10

[(no longer active)]

Attachment: Part 1: When prefetching a document using a Link header, use the document element as the source node

Comment 11

[(no longer active)]

Attachment: Part 2: Use the load group of the document that the source node belongs to when prefetching links

Comment 12

[Boris Zbarsky [: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

Comment 13

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e1e9a9400f07
https://hg.mozilla.org/integration/mozilla-inbound/rev/672a11af05c3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e1e9a9400f07
https://hg.mozilla.org/mozilla-central/rev/672a11af05c3