Closed
Bug 1400001
Opened 7 years ago
Closed 7 years ago
Use AwaitingCacheCallbacks when deciding if racing instead of mCacheAsyncOpenCalled
Categories
(Core :: Networking, enhancement, P1)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: valentin, Assigned: valentin)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-active])
Attachments
(1 file)
We have a bug in the RCWN implementation where if AsyncOpenURI fails mCacheAsyncOpenCalled stays true, but mOnCacheAvailableCalled is false, thus leading the code to think that we are racing. Michal pointed out that we'd be better off using AwaitingCacheCallbacks() since it mostly has the same function
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8908435 [details] Bug 1400001 - Use AwaitingCacheCallbacks when deciding if racing instead of mCacheAsyncOpenCalled https://reviewboard.mozilla.org/r/180076/#review185614 ::: netwerk/protocol/http/nsHttpChannel.cpp:3987 (Diff revision 1) > MOZ_ASSERT(NS_IsMainThread(), "Should be called on the main thread"); > - self->mCacheAsyncOpenCalled = true; > if (self->mNetworkTriggered) { > self->mRaceCacheWithNetwork = true; > } > - nsresult rv = cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, self); > + cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, self); In case of delayed AsyncOpenURI, AwaitingCacheCallbacks() will return true before AsyncOpenURI is called. Not sure if it is really a problem.
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Michal Novotny (:michal) from comment #2) > ::: netwerk/protocol/http/nsHttpChannel.cpp:3987 > (Diff revision 1) > > MOZ_ASSERT(NS_IsMainThread(), "Should be called on the main thread"); > > - self->mCacheAsyncOpenCalled = true; > > if (self->mNetworkTriggered) { > > self->mRaceCacheWithNetwork = true; > > } > > - nsresult rv = cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, self); > > + cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, self); > > In case of delayed AsyncOpenURI, AwaitingCacheCallbacks() will return true > before AsyncOpenURI is called. Not sure if it is really a problem. That should be OK - technically it should have been that way from the beginning, to better simulate how the real cache works.
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8908435 [details] Bug 1400001 - Use AwaitingCacheCallbacks when deciding if racing instead of mCacheAsyncOpenCalled https://reviewboard.mozilla.org/r/180076/#review186024
Attachment #8908435 -
Flags: review?(michal.novotny) → review+
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/1942972c6b53 Use AwaitingCacheCallbacks when deciding if racing instead of mCacheAsyncOpenCalled r=michal
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1942972c6b53
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•