Closed
Bug 752675
Opened 12 years ago
Closed 12 years ago
Fix misuses of LOAD_FROM_CACHE
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: jduell.mcbugs, Assigned: jduell.mcbugs)
References
Details
(Keywords: dev-doc-needed)
Attachments
(3 files, 2 obsolete files)
1.27 KB,
patch
|
michal
:
review+
|
Details | Diff | Splinter Review |
1.35 KB,
patch
|
mcmanus
:
review+
|
Details | Diff | Splinter Review |
1.43 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #621720 -
Flags: review?(michal.novotny)
Assignee | ||
Comment 2•12 years ago
|
||
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)
Assignee | ||
Comment 3•12 years ago
|
||
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 4•12 years ago
|
||
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-
Assignee | ||
Comment 5•12 years ago
|
||
Patrick: ok, fair enough. Let's document it?
Attachment #621723 -
Attachment is obsolete: true
Attachment #621735 -
Flags: review?(mcmanus)
Updated•12 years ago
|
Attachment #621735 -
Flags: review?(mcmanus) → review+
Updated•12 years ago
|
Attachment #621720 -
Flags: review?(michal.novotny) → review+
Comment 6•12 years ago
|
||
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 | ||
Comment 7•12 years ago
|
||
Assignee: nobody → jduell.mcbugs
Attachment #621724 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #622204 -
Flags: review?(bzbarsky)
Comment 8•12 years ago
|
||
Comment on attachment 622204 [details] [diff] [review] Fix nsIWebBrowserPersist.idl comment r=me
Attachment #622204 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f16221d9c85d https://hg.mozilla.org/integration/mozilla-inbound/rev/06da6c1f3bed https://hg.mozilla.org/integration/mozilla-inbound/rev/f4599e608efb
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f4599e608efb https://hg.mozilla.org/mozilla-central/rev/06da6c1f3bed https://hg.mozilla.org/mozilla-central/rev/f16221d9c85d
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Assignee | ||
Comment 11•12 years ago
|
||
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.
Description
•