Closed
Bug 1188387
Opened 10 years ago
Closed 9 years ago
URI is not loaded in case some failure occurs in nsHttpChannel::ProcessNotModified
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
FIXED
mozilla50
People
(Reporter: michal, Assigned: mayhemer)
Details
(Whiteboard: [necko-active])
Attachments
(1 file, 2 obsolete files)
|
4.67 KB,
patch
|
mayhemer
:
review+
|
Details | Diff | Splinter Review |
I hit this problem while testing bug 1177278. If something goes wrong in nsHttpChannel::ProcessNotModified (e.g. CacheEntry::SetMetadataElement returns NS_ERROR_OUT_OF_MEMORY) then nsHttpChannel::ProcessNormal is called and continues like it wasn't a conditional request, so no content is served by the channel. We should handle this in the same way we recover from CacheFile error, i.e. do an internal redirect.
| Reporter | ||
Comment 1•10 years ago
|
||
Honza, you know this code better than me, do you want to take the bug?
Flags: needinfo?(honzab.moz)
| Assignee | ||
Comment 2•10 years ago
|
||
Yep. Thanks.
Assignee: nobody → honzab.moz
Flags: needinfo?(honzab.moz)
| Assignee | ||
Comment 3•10 years ago
|
||
potential patch, I'll look over it more and then submit rr.
| Assignee | ||
Comment 4•10 years ago
|
||
Slightly updated.
- problem was that ProcessNotModified could fail even when it just had to be bypassed
- hence I introduced ShouldBypassProcessNotModified() method that does the checks and can be used separately
- when process modified fails, we do the internal redirect via StartRedirectChannelToURI (it correctly handles redirect vetoing/proceeding)
- entry is doomed and threw away
https://treeherder.mozilla.org/#/jobs?repo=try&revision=252d6d76a3f4
Attachment #8640123 -
Attachment is obsolete: true
Attachment #8640453 -
Flags: review?(michal.novotny)
| Assignee | ||
Comment 5•10 years ago
|
||
Michal, ping on a review here. Thanks.
Status: NEW → ASSIGNED
Flags: needinfo?(michal.novotny)
Updated•9 years ago
|
Whiteboard: [necko-active]
| Reporter | ||
Updated•9 years ago
|
Attachment #8640453 -
Flags: review?(michal.novotny) → review+
| Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(michal.novotny)
| Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8640453 -
Attachment is obsolete: true
Flags: needinfo?(honzab.moz)
Attachment #8761145 -
Flags: review+
| Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d922a8d7ed26
On failure of nsHttpChannel::ProcessNotModified redirect to reload from network, r=michal
Keywords: checkin-needed
Comment 9•9 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•