Closed Bug 752675 Opened 12 years ago Closed 12 years ago

Fix misuses of LOAD_FROM_CACHE

Categories

(Core :: Networking: Cache, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: jduell.mcbugs, Assigned: jduell.mcbugs)

References

Details

(Keywords: dev-doc-needed)

Attachments

(3 files, 2 obsolete files)

This bug contains some uses of LOAD_FROM_CACHE that appear to (mistakenly) assume that the flag guarantees we'll never load the channel from the network. See bug 752663 for overview of the issue.
Attachment #621720 - Flags: review?(michal.novotny)
Patrick:  I'm not sure 100% that we want this.  My understanding is that the primary use of LOAD_FROM_CACHE is session restore and history back/forward navigation.  We do hit the net if a resource isn't available in those cases, so the speculative cxn is a benefit.  OTOH we're also opening a lot of channels, most of which will be hit from cache, so they'll be a lot of speculative opens and most of them will not be used.
Attachment #621723 - Flags: review?(mcmanus)
bz: I don't know anything about nsWebBrowserPersist, but the comment here seems to imply that we want LOAD_ONLY_FROM_CACHE in addition (or instead?) of LOAD_FROM_CACHE when PERSIST_FLAGS_FROM_CACHE is set:

   http://mxr.mozilla.org/mozilla-central/source/embedding/components/webbrowserpersist/public/nsIWebBrowserPersist.idl#57
Attachment #621724 - Flags: review?(bzbarsky)
Comment on attachment 621723 [details] [diff] [review]
Don't block SpeculativeConnect on LOAD_FROM_CACHE.

its all about how you define likely.. so in this case I did mean to disable preconnects on this basis, lacking real data.
Attachment #621723 - Flags: review?(mcmanus) → review-
Blocks: 752663
Patrick: ok, fair enough.  Let's document it?
Attachment #621723 - Attachment is obsolete: true
Attachment #621735 - Flags: review?(mcmanus)
Attachment #621735 - Flags: review?(mcmanus) → review+
Attachment #621720 - Flags: review?(michal.novotny) → review+
Comment on attachment 621724 [details] [diff] [review]
Definition of PERSIST_FLAGS_FROM_CACHE implies LOAD_ONLY_FROM_CACHE should be used?

I have no idea what the intent was, but the main in-tree consumer ("save as" in the Firefox UI) certainly expects the current behavior.  I believe the right thing here is to just fix the IDL comment to reflect reality.
Attachment #621724 - Flags: review?(bzbarsky) → review-
Assignee: nobody → jduell.mcbugs
Attachment #621724 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #622204 - Flags: review?(bzbarsky)
Comment on attachment 622204 [details] [diff] [review]
Fix nsIWebBrowserPersist.idl comment

r=me
Attachment #622204 - Flags: review?(bzbarsky) → review+
dev-doc-needed?  No behavior actually changed, just correcting/refining IDL comments.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: