179983 - prefetching is broken on the 1.0 branch

Status: VERIFIED FIXED

(Core :: Networking, defect, P1)

Description

[Darin Fisher]

prefetching is broken on the 1.0 branch.

this is caused by bug 142135, which was fixed only on the trunk just after the
1.0 branch point.

the fix for bug 174984 caused prefetching to regress.

since the fix for bug 142135 affects core code (the cache), there might be some
value in fixing this bug by workaround bug 142135 in the prefetching code. 
doing so isolates the fix to a non-core part of the codebase.

Comment 1

[Darin Fisher]

Attachment: workaround patch - touches only the prefetching code

Comment 2

[Daniel Veditz [:dveditz]]

Comment on attachment 106119 [details] [diff] [review]
workaround patch - touches only the prefetching code

sr=dveditz, can't hurt, but I'd rather fix the real cause bug 142135

Comment 3

[gordon]

Comment on attachment 106119 [details] [diff] [review]
workaround patch - touches only the prefetching code

r=gordon

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 106119 [details] [diff] [review]
workaround patch - touches only the prefetching code

This seems ok too, but I'd rather the QI map were complete and correct on the
1.0 branch, soon.

/be

Comment 5

[Michael Buckland]

plussing for adt please land asap.

Comment 6

[Darin Fisher]

fixed1.0.2

Comment 7

[John Morrison]

I spent a few hours with this patch applied to a current branch build of
Blackbird on Win2k. I added some additional PR_LOG calls to
nsPrefetchService.cpp and set FORCE_PR_LOG in that file, so I could check that
it was hitting the right code at the right time.  I set up testcases on a
server where I could check the servers logs, and also looked at packet traces
with Ethereal.

The current branch build does the following (which are the correct actions
[or I believe these are all correct actions]):

 1) Sends a referrer header for the current page when initiating a prefetch.

 2) Sends 'X-Moz: prefetch' with a request initiated by the prefetch service.

 3) Initiates prefetch for <link rel='...'> with either the value of 'next'
    or 'prefetch'.

 4) Will only initiate one prefetch when presented with multiple valid 
    keywords in the 'rel' attribute (i.e., for 
    <link rel="next prefetch foobar prefetch">).

 5) Will abort a prefetch if the resource requested indicates that it cannot 
    be cached (e.g., 'Cache-control: no-cache' but what other headers should
    I test, darin). 

 6) Will NOT perform a prefetch for a resource that is already present in the 
    cache.

 7) Will NOT initiate a prefetch if the URI contains a query string.

 8) Will abort all prefetches if another document load is initiated before 
    the prefetch queue has been emptied (and will then empty the queue when 
    the prefetch is aborted. Any partially cached resources will remain in 
    the cache and the next request for that URI will do a partial GET for the
    remainder of the resource.

 9) Will NOT do a prefetch if the HTML document is received by the mail 
    client.

10) If the corrent HTTP 'Link' header is sent (and no '<link rel="...">' is 
    contained in the document, a prefetch will be initiated for the resource
    specified in the 'Link' header.

11) Will NOT do a prefetch for a '<link rel="...">' that is enclosed in 
    comments in the document.

12) Will initiate a prefetch for a properly formatted <META> element, e.g.,
    <meta HTTP-EQUIV="link" CONTENT="&lt;res/Screenshot4.png&gt;; rel=next">

13) Will NOT do any prefetches if 'user_pref("network.prefetch-next", false);'
    is set in the user's profile (which is the default in mozilla builds).

This still needs checking in the actual release builds tomorrow, but it all 
looks good to me at this point.

Comment 8

[Darin Fisher]

jrgm: this looks really great!

as for #5, i don't think we need to worry too much about the other header
combinations.  the decision not to prefetch a response w/ the 'cache-control:
no-cache' header is actually based on the effective cache expiration time, and
errors in that calculation would effect lots of other areas of the product.

Comment 9

[John Morrison]

verified fixed for the 11/14 1.0.2 blackbird branch candidate builds on 
win2k, linux (6.1), mac osx 10.2, mac os9 10.2 per
http://jrgm.mcom.com/bugs/prefetch/test-cases.html

Comment 10

[John Morrison]

> win2k, linux (6.1), mac osx 10.2, mac os9 10.2 

er, mac os9 9.2