Closed
Bug 1020419
Opened 10 years ago
Closed 10 years ago
Extend nsHttpChannel to respect nsICacheEntry's forcedValidation
Categories
(Core :: Networking: HTTP, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: jaypoulz, Assigned: jaypoulz)
References
Details
Attachments
(1 file, 3 obsolete files)
1.76 KB,
patch
|
michal
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jaypoulz
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Updated•10 years ago
|
Summary: Extend nsIHttpChannel to respect nsICacheEntry's syntheticExpirationTime → Extend nsIHttpChannel to respect nsICacheEntry's forcedValidation
Assignee | ||
Comment 1•10 years ago
|
||
Obviously this depends on #1020416, but I figured since I had some extra time, I'd start implementing this. I think I added the clause in the right place, but I'm wondering if there are any other major parts of the workflow in the file that are affected by cache validation.
Attachment #8438081 -
Flags: feedback?(honzab.moz)
Comment 2•10 years ago
|
||
Comment on attachment 8438081 [details] [diff] [review] bug-1020419-fix.patch Review of attachment 8438081 [details] [diff] [review]: ----------------------------------------------------------------- yep, that's it :) ::: netwerk/protocol/http/nsHttpChannel.cpp @@ -2144,5 @@ > (entitySize != cachedContentLength)) { > LOG(("nsHttpChannel::ProcessPartialContent [this=%p] " > "206 has different total entity size than the content length " > "of the original partially cached entity.\n", this)); > - no ws only changes please.
Attachment #8438081 -
Flags: feedback?(honzab.moz) → feedback+
Updated•10 years ago
|
Summary: Extend nsIHttpChannel to respect nsICacheEntry's forcedValidation → Extend nsHttpChannel to respect nsICacheEntry's forcedValidation
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8438081 -
Attachment is obsolete: true
Attachment #8458818 -
Flags: review?(honzab.moz)
Assignee | ||
Comment 4•10 years ago
|
||
Actually removed the ws changes.
Attachment #8458818 -
Attachment is obsolete: true
Attachment #8458818 -
Flags: review?(honzab.moz)
Assignee | ||
Updated•10 years ago
|
Attachment #8458820 -
Flags: review?(honzab.moz)
Comment 5•10 years ago
|
||
Comment on attachment 8458820 [details] [diff] [review] bug-1020419-fix.patch Review of attachment 8458820 [details] [diff] [review]: ----------------------------------------------------------------- Push to try before landing. ::: netwerk/protocol/http/nsHttpChannel.cpp @@ +2806,5 @@ > bool canAddImsHeader = true; > > + bool isForcedValid; > + rv = entry->GetIsForcedValid(&isForcedValid); > + NS_ENSURE_SUCCESS(rv,rv); hmm.. app cache entries don't implement this. if the method fails, just set isForceValid = false and go on (no need to log, ns_warn etc...).
Attachment #8458820 -
Flags: review?(honzab.moz) → feedback+
Assignee | ||
Comment 6•10 years ago
|
||
Of course I'll push to try, but right now 1020416 is still in flux with Michals stuff so I'd rather wait for that to stabilize.
Attachment #8458820 -
Attachment is obsolete: true
Attachment #8458857 -
Flags: review?(honzab.moz)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8458857 [details] [diff] [review] bug-1020419-fix.patch Hi Pat - would you mind glancing over this since Honza is on PTO?
Attachment #8458857 -
Flags: review?(honzab.moz) → review?(mcmanus)
Assignee | ||
Comment 8•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=b12c1d38a1da
Comment 9•10 years ago
|
||
Hey Jeremy - this seems sane to me, but I'm not real familiar with the new cache code yet. I'll delegate it to michal.
Comment 10•10 years ago
|
||
Comment on attachment 8458857 [details] [diff] [review] bug-1020419-fix.patch Review of attachment 8458857 [details] [diff] [review]: ----------------------------------------------------------------- delegate to michal
Attachment #8458857 -
Flags: review?(mcmanus) → review?(michal.novotny)
Assignee | ||
Comment 11•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=73f6656e05c2
Updated•10 years ago
|
Attachment #8458857 -
Flags: review?(michal.novotny) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/27a066378f69
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/27a066378f69
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•