Bug 722033 - Use asyncOpenCacheEntry() in nsHttpChannel when flag LOAD_BYPASS_LOCAL_CACHE_IF_BUSY is set
authorMichal Novotny <michal.novotny@gmail.com>
Thu, 22 Mar 2012 23:53:10 +0100
changeset 91899 6c8d1da1e1db869a7a9588be955142085810c445
parent 91898 f42ea2a158e451a490db2c0435fc57885ac349be
child 91900 5b61e3d75735e331ac401f5f7b057c9997d03aa0
push idunknown
push userunknown
push dateunknown
bugs722033
milestone14.0a1
Bug 722033 - Use asyncOpenCacheEntry() in nsHttpChannel when flag LOAD_BYPASS_LOCAL_CACHE_IF_BUSY is set
netwerk/cache/nsCacheService.cpp
netwerk/cache/nsCacheSession.cpp
netwerk/cache/nsICacheSession.idl
netwerk/protocol/ftp/nsFtpConnectionThread.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
--- a/netwerk/cache/nsCacheService.cpp
+++ b/netwerk/cache/nsCacheService.cpp
@@ -1015,17 +1015,18 @@ public:
                      "Sync OpenCacheEntry() posted to background thread!");
 
         nsCacheServiceAutoLock lock;
         rv = nsCacheService::gService->ProcessRequest(mRequest,
                                                       false,
                                                       nsnull);
 
         // Don't delete the request if it was queued
-        if (rv != NS_ERROR_CACHE_WAIT_FOR_VALIDATION)
+        if (!(mRequest->IsBlocking() &&
+            rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION))
             delete mRequest;
 
         return NS_OK;
     }
 
 protected:
     virtual ~nsProcessRequestEvent() {}
 
@@ -1657,21 +1658,23 @@ nsCacheService::ProcessRequest(nsCacheRe
         rv = ActivateEntry(request, &entry, &doomedEntry);  // get the entry for this request
         if (NS_FAILED(rv))  break;
 
         while(1) { // Request Access loop
             NS_ASSERTION(entry, "no entry in Request Access loop!");
             // entry->RequestAccess queues request on entry
             rv = entry->RequestAccess(request, &accessGranted);
             if (rv != NS_ERROR_CACHE_WAIT_FOR_VALIDATION) break;
-            
-            if (request->mListener) // async exits - validate, doom, or close will resume
-                return rv;
-            
+
             if (request->IsBlocking()) {
+                if (request->mListener) {
+                    // async exits - validate, doom, or close will resume
+                    return rv;
+                }
+
                 // XXX this is probably wrong...
                 Unlock();
                 rv = request->WaitForValidation();
                 Lock();
             }
 
             PR_REMOVE_AND_INIT_LINK(request);
             if (NS_FAILED(rv)) break;   // non-blocking mode returns WAIT_FOR_VALIDATION error
@@ -1768,17 +1771,18 @@ nsCacheService::OpenCacheEntry(nsCacheSe
             delete request;
     }
     else {
 
         nsCacheServiceAutoLock lock;
         rv = gService->ProcessRequest(request, true, result);
 
         // delete requests that have completed
-        if (!(listener && (rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION)))
+        if (!(listener && blockingMode &&
+            (rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION)))
             delete request;
     }
 
     return rv;
 }
 
 
 nsresult
--- a/netwerk/cache/nsCacheSession.cpp
+++ b/netwerk/cache/nsCacheSession.cpp
@@ -97,23 +97,24 @@ nsCacheSession::OpenCacheEntry(const nsA
                                          nsnull, // no listener
                                          result);
     return rv;
 }
 
 
 NS_IMETHODIMP nsCacheSession::AsyncOpenCacheEntry(const nsACString & key,
                                                   nsCacheAccessMode accessRequested,
-                                                  nsICacheListener *listener)
+                                                  nsICacheListener *listener,
+                                                  bool              noWait)
 {
     nsresult rv;
     rv = nsCacheService::OpenCacheEntry(this,
                                         key,
                                         accessRequested,
-                                        nsICache::BLOCKING,
+                                        !noWait,
                                         listener,
                                         nsnull); // no result
 
     if (rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION) rv = NS_OK;
     return rv;
 }
 
 NS_IMETHODIMP nsCacheSession::EvictEntries()
--- a/netwerk/cache/nsICacheSession.idl
+++ b/netwerk/cache/nsICacheSession.idl
@@ -41,17 +41,17 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsISupports.idl"
 #include "nsICache.idl"
 
 interface nsICacheEntryDescriptor;
 interface nsICacheListener;
 
-[scriptable, uuid(ae9e84b5-3e2d-457e-8fcd-5bbd2a8b832e)]
+[scriptable, uuid(2ec1026f-e3a5-481c-908e-8d559235b721)]
 interface nsICacheSession : nsISupports
 {
     /**
      * Expired entries will be doomed or evicted if this attribute is set to
      * true.  If false, expired entries will be returned (useful for offline-
      * mode and clients, such as HTTP, that can update the valid lifetime of
      * cached content).  This attribute defaults to true.
      */
@@ -71,23 +71,27 @@ interface nsICacheSession : nsISupports
      * return NS_ERROR_CACHE_WAIT_FOR_VALIDATION rather than block when another
      * descriptor has been given WRITE access but hasn't validated the entry yet.
      */
     nsICacheEntryDescriptor openCacheEntry(in ACString          key,
                                            in nsCacheAccessMode accessRequested,
                                            in boolean           blockingMode);
 
     /**
-     * Asynchronous cache access. Does not block the calling thread.
-     * Instead, the listener will be notified when the descriptor is
-     * available.
+     * Asynchronous cache access. Does not block the calling thread. Instead,
+     * the listener will be notified when the descriptor is available. If
+     * 'noWait' is set to true, the listener will be notified immediately with
+     * status NS_ERROR_CACHE_WAIT_FOR_VALIDATION rather than queuing the request
+     * when another descriptor has been given WRITE access but hasn't validated
+     * the entry yet.
      */
-    void asyncOpenCacheEntry(in ACString          key,
-                             in nsCacheAccessMode accessRequested,
-                             in nsICacheListener  listener);
+    void asyncOpenCacheEntry(in ACString           key,
+                             in nsCacheAccessMode  accessRequested,
+                             in nsICacheListener   listener,
+                             [optional] in boolean noWait);
 
     /**
      * Evict all entries for this session's clientID according to its storagePolicy.
      */
     void evictEntries();
     
     /**
      * Return whether any of the cache devices implied by the session storage policy
--- a/netwerk/protocol/ftp/nsFtpConnectionThread.cpp
+++ b/netwerk/protocol/ftp/nsFtpConnectionThread.cpp
@@ -2283,17 +2283,17 @@ nsFtpState::CheckCache()
     nsresult rv = session->OpenCacheEntry(key, accessReq, false,
                                           getter_AddRefs(mCacheEntry));
     if (NS_SUCCEEDED(rv) && mCacheEntry) {
         mDoomCache = true;
         return false;  // great, we're ready to proceed!
     }
 
     if (rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION) {
-        rv = session->AsyncOpenCacheEntry(key, accessReq, this);
+        rv = session->AsyncOpenCacheEntry(key, accessReq, this, false);
         return NS_SUCCEEDED(rv);
     }
 
     return false;
 }
 
 nsresult
 nsFtpState::ConvertUTF8PathToCharset(const nsACString &aCharset)
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -2112,50 +2112,33 @@ nsHttpChannel::OpenCacheEntry()
         NS_ENSURE_SUCCESS(rv, rv);
 
         rv = serv->CreateSession(appCacheClientID.get(),
                                  nsICache::STORE_OFFLINE,
                                  nsICache::STREAM_BASED,
                                  getter_AddRefs(session));
         NS_ENSURE_SUCCESS(rv, rv);
 
-        if (mLoadFlags & LOAD_BYPASS_LOCAL_CACHE_IF_BUSY) {
-            // must use synchronous open for LOAD_BYPASS_LOCAL_CACHE_IF_BUSY
-            rv = session->OpenCacheEntry(cacheKey,
-                                         nsICache::ACCESS_READ, false,
-                                         getter_AddRefs(mCacheEntry));
-            if (NS_SUCCEEDED(rv)) {
-                mCacheEntry->GetAccessGranted(&mCacheAccess);
-                LOG(("nsHttpChannel::OpenCacheEntry [this=%p grantedAccess=%d]",
-                    this, mCacheAccess));
-                mLoadedFromApplicationCache = true;
-                return NS_OK;
-            } else if (rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION) {
-                LOG(("bypassing local cache since it is busy\n"));
-                // Don't try to load normal cache entry
-                return NS_ERROR_NOT_AVAILABLE;
-            }
-        } else {
-            mOnCacheEntryAvailableCallback =
-                &nsHttpChannel::OnOfflineCacheEntryAvailable;
-            // We open with ACCESS_READ only, because we don't want to
-            // overwrite the offline cache entry non-atomically.
-            // ACCESS_READ will prevent us from writing to the offline
-            // cache as a normal cache entry.
-            rv = session->AsyncOpenCacheEntry(cacheKey,
-                                              nsICache::ACCESS_READ,
-                                              this);
-
-            if (NS_SUCCEEDED(rv)) {
-                mAsyncCacheOpen = true;
-                return NS_OK;
-            }
+        mOnCacheEntryAvailableCallback =
+            &nsHttpChannel::OnOfflineCacheEntryAvailable;
+        // We open with ACCESS_READ only, because we don't want to overwrite
+        // the offline cache entry non-atomically. ACCESS_READ will prevent us
+        // from writing to the offline cache as a normal cache entry.
+        rv = session->AsyncOpenCacheEntry(
+            cacheKey,
+            nsICache::ACCESS_READ,
+            this,
+            mLoadFlags & LOAD_BYPASS_LOCAL_CACHE_IF_BUSY);
+
+        if (NS_SUCCEEDED(rv)) {
+            mAsyncCacheOpen = true;
+            return NS_OK;
         }
 
-        // sync or async opening failed
+        // opening cache entry failed
         return OnOfflineCacheEntryAvailable(nsnull, nsICache::ACCESS_NONE,
                                             rv, true);
     }
 
     return OpenNormalCacheEntry(true);
 }
 
 nsresult
@@ -2169,16 +2152,22 @@ nsHttpChannel::OnOfflineCacheEntryAvaila
     if (NS_SUCCEEDED(aEntryStatus)) {
         // We successfully opened an offline cache session and the entry,
         // so indicate we will load from the offline cache.
         mLoadedFromApplicationCache = true;
         mCacheEntry = aEntry;
         mCacheAccess = aAccess;
     }
 
+    if (aEntryStatus == NS_ERROR_CACHE_WAIT_FOR_VALIDATION) {
+        LOG(("bypassing local cache since it is busy\n"));
+        // Don't try to load normal cache entry
+        return aIsSync ? NS_ERROR_NOT_AVAILABLE : Connect(false);
+    }
+
     if (mCanceled && NS_FAILED(mStatus)) {
         LOG(("channel was canceled [this=%p status=%x]\n", this, mStatus));
         return mStatus;
     }
 
     if (NS_SUCCEEDED(aEntryStatus))
         // Called from OnCacheEntryAvailable, advance to the next state
         return Connect(false);
@@ -2255,48 +2244,27 @@ nsHttpChannel::OpenNormalCacheEntry(bool
     rv = gHttpHandler->GetCacheSession(storagePolicy,
                                        getter_AddRefs(session));
     if (NS_FAILED(rv)) return rv;
 
     nsCacheAccessMode accessRequested;
     rv = DetermineCacheAccess(&accessRequested);
     if (NS_FAILED(rv)) return rv;
 
-    if (mLoadFlags & LOAD_BYPASS_LOCAL_CACHE_IF_BUSY) {
-        if (!aIsSync) {
-            // Unexpected state: we were called from OnCacheEntryAvailable(),
-            // so LOAD_BYPASS_LOCAL_CACHE_IF_BUSY shouldn't be set. Unless
-            // somebody altered mLoadFlags between OpenCacheEntry() and
-            // OnCacheEntryAvailable()...
-            NS_WARNING(
-                "OpenNormalCacheEntry() called from OnCacheEntryAvailable() "
-                "when LOAD_BYPASS_LOCAL_CACHE_IF_BUSY was specified");
-        }
-
-        // must use synchronous open for LOAD_BYPASS_LOCAL_CACHE_IF_BUSY
-        rv = session->OpenCacheEntry(cacheKey, accessRequested, false,
-                                     getter_AddRefs(mCacheEntry));
-        if (NS_SUCCEEDED(rv)) {
-            mCacheEntry->GetAccessGranted(&mCacheAccess);
-            LOG(("nsHttpChannel::OpenCacheEntry [this=%p grantedAccess=%d]",
-                this, mCacheAccess));
-        }
-        else if (rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION) {
-            LOG(("bypassing local cache since it is busy\n"));
-            rv = NS_ERROR_NOT_AVAILABLE;
-        }
-    }
-    else {
-        mOnCacheEntryAvailableCallback =
-            &nsHttpChannel::OnNormalCacheEntryAvailable;
-        rv = session->AsyncOpenCacheEntry(cacheKey, accessRequested, this);
-        if (NS_SUCCEEDED(rv)) {
-            mAsyncCacheOpen = true;
-            return NS_OK;
-        }
+    mOnCacheEntryAvailableCallback =
+        &nsHttpChannel::OnNormalCacheEntryAvailable;
+    rv = session->AsyncOpenCacheEntry(
+        cacheKey,
+        accessRequested,
+        this,
+        mLoadFlags & LOAD_BYPASS_LOCAL_CACHE_IF_BUSY);
+
+    if (NS_SUCCEEDED(rv)) {
+        mAsyncCacheOpen = true;
+        return NS_OK;
     }
 
     if (!aIsSync)
         // Called from OnCacheEntryAvailable, advance to the next state
         rv = Connect(false);
 
     return rv;
 }
@@ -2309,16 +2277,20 @@ nsHttpChannel::OnNormalCacheEntryAvailab
 {
     NS_ASSERTION(!aIsSync, "aIsSync should be false");
 
     if (NS_SUCCEEDED(aEntryStatus)) {
         mCacheEntry = aEntry;
         mCacheAccess = aAccess;
     }
 
+    if (aEntryStatus == NS_ERROR_CACHE_WAIT_FOR_VALIDATION) {
+        LOG(("bypassing local cache since it is busy\n"));
+    }
+
     if (mCanceled && NS_FAILED(mStatus)) {
         LOG(("channel was canceled [this=%p status=%x]\n", this, mStatus));
         return mStatus;
     }
 
     if ((mLoadFlags & LOAD_ONLY_FROM_CACHE) && NS_FAILED(aEntryStatus))
         // if this channel is only allowed to pull from the cache, then
         // we must fail if we were unable to open a cache entry.
--- a/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
+++ b/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
@@ -704,17 +704,17 @@ nsWyciwygChannel::OpenCacheEntry(const n
                                    getter_AddRefs(cacheSession));
   if (!cacheSession) 
     return NS_ERROR_FAILURE;
 
   if (aAccessMode == nsICache::ACCESS_WRITE)
     rv = cacheSession->OpenCacheEntry(aCacheKey, aAccessMode, false,
                                       getter_AddRefs(mCacheEntry));
   else
-    rv = cacheSession->AsyncOpenCacheEntry(aCacheKey, aAccessMode, this);
+    rv = cacheSession->AsyncOpenCacheEntry(aCacheKey, aAccessMode, this, false);
 
   return rv;
 }
 
 nsresult
 nsWyciwygChannel::ReadFromCache()
 {
   LOG(("nsWyciwygChannel::ReadFromCache [this=%x] ", this));