Bug 1400001 - Use AwaitingCacheCallbacks when deciding if racing instead of mCacheAsyncOpenCalled r=michal
authorValentin Gosu <valentin.gosu@gmail.com>
Fri, 15 Sep 2017 02:47:41 +0200
changeset 669703 1942972c6b53d5d1858fc63508edbc3057b0cefa
parent 669702 24116d4c6b8520966854a9b8166770cc6efe0730
child 669704 33dbf9b5444fc121a25481c2e4b2d955ea8ddd01
child 669950 d1b1dd292ff1d672434663e3457bbaaee6074689
push id81398
push userhikezoe@mozilla.com
push dateMon, 25 Sep 2017 08:14:31 +0000
reviewersmichal
bugs1400001
milestone58.0a1
Bug 1400001 - Use AwaitingCacheCallbacks when deciding if racing instead of mCacheAsyncOpenCalled r=michal MozReview-Commit-ID: 5VRRXrkNo9Y
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -339,20 +339,18 @@ nsHttpChannel::nsHttpChannel()
     , mUsedNetwork(0)
     , mAuthConnectionRestartable(0)
     , mPushedStream(nullptr)
     , mLocalBlocklist(false)
     , mOnTailUnblock(nullptr)
     , mWarningReporter(nullptr)
     , mIsReadingFromCache(false)
     , mFirstResponseSource(RESPONSE_PENDING)
-    , mOnCacheAvailableCalled(false)
     , mRaceCacheWithNetwork(false)
     , mRaceDelay(0)
-    , mCacheAsyncOpenCalled(false)
     , mIgnoreCacheEntry(false)
     , mRCWNLock("nsHttpChannel.mRCWNLock")
     , mDidReval(false)
 {
     LOG(("Creating nsHttpChannel [this=%p]\n", this));
     mChannelCreationTime = PR_Now();
     mChannelCreationTimestamp = TimeStamp::Now();
 }
@@ -1103,17 +1101,17 @@ nsHttpChannel::SetupTransaction()
     nsresult rv;
 
     mozilla::MutexAutoLock lock(mRCWNLock);
 
     // If we're racing cache with network, conditional or byte range header
     // could be added in OnCacheEntryCheck. We cannot send conditional request
     // without having the entry, so we need to remove the headers here and
     // ignore the cache entry in OnCacheEntryAvailable.
-    if (mRaceCacheWithNetwork && !mOnCacheAvailableCalled) {
+    if (mRaceCacheWithNetwork && AwaitingCacheCallbacks()) {
         if (mDidReval) {
             LOG(("  Removing conditional request headers"));
             UntieValidationRequest();
             mDidReval = false;
             mIgnoreCacheEntry = true;
         }
 
         if (mCachedContentIsPartial) {
@@ -3978,38 +3976,29 @@ nsHttpChannel::OpenCacheEntry(bool isHtt
             if (NS_SUCCEEDED(rv) && !hasAltData &&
                 sizeInKb < sRCWNSmallResourceSizeKB) {
                 MaybeRaceCacheWithNetwork();
             }
         }
 
         if (!mCacheOpenDelay) {
             MOZ_ASSERT(NS_IsMainThread(), "Should be called on the main thread");
-            mCacheAsyncOpenCalled = true;
             if (mNetworkTriggered) {
                 mRaceCacheWithNetwork = sRCWNEnabled;
             }
             rv = cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, this);
-            if (NS_FAILED(rv)) {
-                // Drop the flag since the cache open failed
-                mCacheAsyncOpenCalled = false;
-            }
         } else {
             // We pass `this` explicitly as a parameter due to the raw pointer
             // to refcounted object in lambda analysis.
             mCacheOpenFunc = [openURI, extension, cacheEntryOpenFlags, cacheStorage] (nsHttpChannel* self) -> void {
                 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);
-                if (NS_FAILED(rv)) {
-                    self->mCacheAsyncOpenCalled = false;
-                }
+                cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, self);
             };
 
             mCacheOpenTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
             // calls nsHttpChannel::Notify after `mCacheOpenDelay` milliseconds
             mCacheOpenTimer->InitWithCallback(this, mCacheOpenDelay, nsITimer::TYPE_ONE_SHOT);
 
         }
         NS_ENSURE_SUCCESS(rv, rv);
@@ -4429,17 +4418,16 @@ nsHttpChannel::OnCacheEntryCheck(nsICach
 
 NS_IMETHODIMP
 nsHttpChannel::OnCacheEntryAvailable(nsICacheEntry *entry,
                                      bool aNew,
                                      nsIApplicationCache* aAppCache,
                                      nsresult status)
 {
     MOZ_ASSERT(NS_IsMainThread());
-    mOnCacheAvailableCalled = true;
 
     nsresult rv;
 
     LOG(("nsHttpChannel::OnCacheEntryAvailable [this=%p entry=%p "
          "new=%d appcache=%p status=%" PRIx32 " mAppCache=%p mAppCacheForWrite=%p]\n",
          this, entry, aNew, aAppCache, static_cast<uint32_t>(status),
          mApplicationCache.get(), mApplicationCacheForWrite.get()));
 
@@ -9399,17 +9387,17 @@ nsHttpChannel::TriggerNetwork()
     // BeginConnect, and Connect will call TryHSTSPriming even if it's
     // for the cache callbacks.
     if (mProxyRequest) {
         LOG(("  proxy request in progress. Delaying network trigger.\n"));
         mWaitingForProxy = true;
         return NS_OK;
     }
 
-    if (mCacheAsyncOpenCalled && !mOnCacheAvailableCalled) {
+    if (AwaitingCacheCallbacks()) {
         mRaceCacheWithNetwork = sRCWNEnabled;
     }
 
     LOG(("  triggering network\n"));
     return TryHSTSPriming();
 }
 
 nsresult
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -726,23 +726,20 @@ private:
     nsresult TriggerNetwork();
     void CancelNetworkRequest(nsresult aStatus);
     // Timer used to delay the network request, or to trigger the network
     // request if retrieving the cache entry takes too long.
     nsCOMPtr<nsITimer> mNetworkTriggerTimer;
     // Is true if the network request has been triggered.
     bool mNetworkTriggered = false;
     bool mWaitingForProxy = false;
-    // Is true if the onCacheEntryAvailable callback has been called.
-    bool mOnCacheAvailableCalled;
     // Will be true if the onCacheEntryAvailable callback is not called by the
     // time we send the network request
     Atomic<bool> mRaceCacheWithNetwork;
     uint32_t mRaceDelay;
-    bool mCacheAsyncOpenCalled;
     // If true then OnCacheEntryAvailable should ignore the entry, because
     // SetupTransaction removed conditional headers and decisions made in
     // OnCacheEntryCheck are no longer valid.
     bool mIgnoreCacheEntry;
     // Lock preventing OnCacheEntryCheck and SetupTransaction being called at
     // the same time.
     mozilla::Mutex mRCWNLock;