Bug 746018 - Part 6 - Start buffering cache entries in memory before we validate them, r=honzab
authorBrian Smith <bsmith@mozilla.com>
Thu, 31 May 2012 15:20:04 -0700
changeset 97633 301d0d6bf24e9806c1a19670238f41164fd6c7df
parent 97632 0f3f9ff4439dc17f8c45289475c95d755d7dca9a
child 97634 369cb6c883309b9028fb11907a31643077302863
push idunknown
push userunknown
push dateunknown
reviewershonzab
bugs746018
milestone15.0a1
Bug 746018 - Part 6 - Start buffering cache entries in memory before we validate them, r=honzab
netwerk/base/src/AutoClose.h
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
netwerk/protocol/http/nsHttpResponseHead.cpp
new file mode 100644
--- /dev/null
+++ b/netwerk/base/src/AutoClose.h
@@ -0,0 +1,76 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_net_AutoClose_h
+#define mozilla_net_AutoClose_h
+
+#include "nsCOMPtr.h"
+
+namespace mozilla { namespace net {
+
+// Like an nsAutoPtr for XPCOM streams (e.g. nsIAsyncInputStream) and other
+// refcounted classes that need to have the Close() method called explicitly
+// before they are destroyed.
+template <typename T>
+class AutoClose
+{
+public:
+  AutoClose() { } 
+  ~AutoClose(){
+    Close();
+  }
+
+  operator bool() const
+  {
+    return mPtr;
+  }
+
+  already_AddRefed<T> forget()
+  {
+    return mPtr.forget();
+  }
+
+  void takeOver(AutoClose<T> & rhs)
+  {
+    Close();
+    mPtr = rhs.mPtr.forget();
+  }
+
+  // assign from |do_QueryInterface(expr, &rv)|
+  void operator=(const nsQueryInterfaceWithError rhs)
+  {
+    Close();
+    mPtr = rhs;
+  }
+
+  void CloseAndRelease()
+  {
+    Close();
+    mPtr = nsnull;
+  }
+
+  T* operator->() const
+  {
+    return mPtr.operator->();
+  }
+
+private:
+  void Close()
+  {
+    if (mPtr) {
+      mPtr->Close();
+    }
+  }
+
+  void operator=(const AutoClose<T> &) MOZ_DELETE;
+  AutoClose(const AutoClose<T> &) MOZ_DELETE;
+
+  nsCOMPtr<T> mPtr;
+};
+
+} } // namespace mozilla::net
+
+#endif // mozilla_net_AutoClose_h
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -29,30 +29,69 @@
 #include "mozilla/Telemetry.h"
 #include "nsDOMError.h"
 #include "nsAlgorithm.h"
 #include "sampler.h"
 #include "nsIConsoleService.h"
 #include "base/compiler_specific.h"
 #include "NullHttpTransaction.h"
 
-using namespace mozilla;
+namespace mozilla { namespace net {
+ 
+namespace {
 
 // Device IDs for various cache types
 const char kDiskDeviceID[] = "disk";
 const char kMemoryDeviceID[] = "memory";
 const char kOfflineDeviceID[] = "offline";
 
 // True if the local cache should be bypassed when processing a request.
 #define BYPASS_LOCAL_CACHE(loadFlags) \
         (loadFlags & (nsIRequest::LOAD_BYPASS_CACHE | \
                       nsICachingChannel::LOAD_BYPASS_LOCAL_CACHE))
 
 static NS_DEFINE_CID(kStreamListenerTeeCID, NS_STREAMLISTENERTEE_CID);
 
+bool IsRedirectStatus(PRUint32 status)
+{
+    // 305 disabled as a security measure (see bug 187996).
+    return status == 300 || status == 301 || status == 302 || status == 303 ||
+           status == 307 || status == 308;
+}
+
+// We only treat 3xx responses as redirects if they have a Location header and
+// the status code is in a whitelist.
+bool
+WillRedirect(const nsHttpResponseHead * response)
+{
+    return IsRedirectStatus(response->Status()) &&
+           response->PeekHeader(nsHttp::Location);
+}
+
+void
+MaybeMarkCacheEntryValid(const void * channel,
+                         nsICacheEntryDescriptor * cacheEntry,
+                         nsCacheAccessMode cacheAccess)
+{
+    // Mark the cache entry as valid in order to allow others access to it.
+    // XXX: Is it really necessary to check for write acccess to the entry?
+    if (cacheAccess & nsICache::ACCESS_WRITE) {
+        nsresult rv = cacheEntry->MarkValid();
+        LOG(("Marking cache entry valid "
+             "[channel=%p, entry=%p, access=%d, result=%d]",
+             channel, cacheEntry, PRIntn(cacheAccess), PRIntn(rv)));
+    } else {
+        LOG(("Not marking read-only cache entry valid "
+             "[channel=%p, entry=%p, access=%d]", 
+             channel, cacheEntry, PRIntn(cacheAccess)));
+    }
+}
+
+} // unnamed namespace
+
 class AutoRedirectVetoNotifier
 {
 public:
     AutoRedirectVetoNotifier(nsHttpChannel* channel) : mChannel(channel) {}
     ~AutoRedirectVetoNotifier() {ReportRedirectResult(false);}
     void RedirectSucceeded() {ReportRedirectResult(true);}
 
 private:
@@ -255,17 +294,17 @@ nsHttpChannel::ContinueConnect()
             NS_WARNING("cache check failed");
 
         // read straight from the cache if possible...
         if (mCachedContentIsValid) {
             nsRunnableMethod<nsHttpChannel> *event = nsnull;
             if (!mCachedContentIsPartial) {
                 AsyncCall(&nsHttpChannel::AsyncOnExamineCachedResponse, &event);
             }
-            rv = ReadFromCache();
+            rv = ReadFromCache(true);
             if (NS_FAILED(rv) && event) {
                 event->Revoke();
             }
             mozilla::Telemetry::Accumulate(
                     mozilla::Telemetry::HTTP_CACHE_DISPOSITION, kCacheHit);
 
             char* cacheDeviceID = nsnull;
             mCacheEntry->GetDeviceID(&cacheDeviceID);
@@ -985,16 +1024,21 @@ nsHttpChannel::ProcessResponse()
         // If SSL proxy response needs to complete, wait to process connection
         // for Strict-Transport-Security.
     } else {
         // Given a successful connection, process any STS data that's relevant.
         rv = ProcessSTSHeader();
         NS_ASSERTION(NS_SUCCEEDED(rv), "ProcessSTSHeader failed, continuing load.");
     }
 
+    MOZ_ASSERT(!mCachedContentIsValid);
+    if (httpStatus != 304 && httpStatus != 206) {
+        mCacheAsyncInputStream.CloseAndRelease();
+    }
+
     // notify "http-on-examine-response" observers
     gHttpHandler->OnExamineResponse(this);
 
     SetCookie(mResponseHead->PeekHeader(nsHttp::Set_Cookie));
 
     // handle unused username and password in url (see bug 232567)
     if (httpStatus != 401 && httpStatus != 407) {
         if (!mAuthRetryPending)
@@ -1030,18 +1074,20 @@ nsHttpChannel::ProcessResponse()
         }
         // these can normally be cached
         rv = ProcessNormal();
         MaybeInvalidateCacheEntryForSubsequentGet();
         break;
     case 206:
         if (mCachedContentIsPartial) // an internal byte range request...
             rv = ProcessPartialContent();
-        else
+        else {
+            mCacheAsyncInputStream.CloseAndRelease();
             rv = ProcessNormal();
+        }
         break;
     case 300:
     case 301:
     case 302:
     case 307:
     case 308:
     case 303:
 #if 0
@@ -1056,16 +1102,17 @@ nsHttpChannel::ProcessResponse()
             LOG(("AsyncProcessRedirection failed [rv=%x]\n", rv));
             rv = ContinueProcessResponse(rv);
         }
         break;
     case 304:
         rv = ProcessNotModified();
         if (NS_FAILED(rv)) {
             LOG(("ProcessNotModified failed [rv=%x]\n", rv));
+            mCacheAsyncInputStream.CloseAndRelease();
             rv = ProcessNormal();
         }
         else {
             successfulReval = true;
         }
         break;
     case 401:
     case 407:
@@ -1909,17 +1956,17 @@ nsHttpChannel::ProcessPartialContent()
     if (NS_FAILED(rv)) return rv;
 
     // notify observers interested in looking at a response that has been
     // merged with any cached headers (http-on-examine-merged-response).
     gHttpHandler->OnExamineMergedResponse(this);
 
     // the cached content is valid, although incomplete.
     mCachedContentIsValid = true;
-    return ReadFromCache();
+    return ReadFromCache(false);
 }
 
 nsresult
 nsHttpChannel::OnDoneReadingPartialCacheEntry(bool *streamDone)
 {
     nsresult rv;
 
     LOG(("nsHttpChannel::OnDoneReadingPartialCacheEntry [this=%p]", this));
@@ -2025,17 +2072,17 @@ nsHttpChannel::ProcessNotModified()
     rv = AddCacheEntryHeaders(mCacheEntry);
     if (NS_FAILED(rv)) return rv;
 
     // notify observers interested in looking at a reponse that has been
     // merged with any cached headers
     gHttpHandler->OnExamineMergedResponse(this);
 
     mCachedContentIsValid = true;
-    rv = ReadFromCache();
+    rv = ReadFromCache(false);
     if (NS_FAILED(rv)) return rv;
 
     mTransactionReplaced = true;
     return NS_OK;
 }
 
 nsresult
 nsHttpChannel::ProcessFallback(bool *waitingForRedirectCallback)
@@ -2604,16 +2651,20 @@ nsHttpChannel::UpdateExpirationTime()
 // this URL but before going out to net.  It's purpose is to set or clear the 
 // mCachedContentIsValid flag, and to configure an If-Modified-Since request
 // if validation is required.
 nsresult
 nsHttpChannel::CheckCache()
 {
     nsresult rv = NS_OK;
 
+    bool usingSSL = false;
+    rv = mURI->SchemeIs("https", &usingSSL);
+    NS_ENSURE_SUCCESS(rv,rv);
+
     LOG(("nsHTTPChannel::CheckCache enter [this=%p entry=%p access=%d]",
         this, mCacheEntry.get(), mCacheAccess));
     
     // Be pessimistic: assume the cache entry has no useful data.
     mCachedContentIsValid = false;
 
     // Don't proceed unless we have opened a cache entry for reading.
     if (!mCacheEntry || !(mCacheAccess & nsICache::ACCESS_READ))
@@ -2645,49 +2696,61 @@ nsHttpChannel::CheckCache()
             (gHttpHandler->SessionStartTime() > lastModifiedTime);
 
     // Get the cached HTTP response headers
     rv = mCacheEntry->GetMetaDataElement("response-head", getter_Copies(buf));
     NS_ENSURE_SUCCESS(rv, rv);
 
     // Parse the cached HTTP response headers
     mCachedResponseHead = new nsHttpResponseHead();
-    if (!mCachedResponseHead)
-        return NS_ERROR_OUT_OF_MEMORY;
     rv = mCachedResponseHead->Parse((char *) buf.get());
     NS_ENSURE_SUCCESS(rv, rv);
     buf.Adopt(0);
 
+    bool isCachedRedirect = WillRedirect(mCachedResponseHead);
+
+    // Do not return 304 responses from the cache, and also do not return
+    // any other non-redirect 3xx responses from the cache (see bug 759043).
+    NS_ENSURE_TRUE((mCachedResponseHead->Status() / 100 != 3) ||
+                   isCachedRedirect, NS_ERROR_ABORT);
+
     // Don't bother to validate items that are read-only,
     // unless they are read-only because of INHIBIT_CACHING or because
     // we're updating the offline cache.
     // Don't bother to validate if this is a fallback entry.
     if (!mCacheForOfflineUse &&
         (mLoadedFromApplicationCache ||
          (mCacheAccess == nsICache::ACCESS_READ &&
           !(mLoadFlags & INHIBIT_CACHING)) ||
          mFallbackChannel)) {
-        mCachedContentIsValid = true;
-        return NS_OK;
-    }
-
-    PRUint16 isCachedRedirect = mCachedResponseHead->Status()/100 == 3;
+        rv = StartBufferingCachedEntity(usingSSL);
+        if (NS_SUCCEEDED(rv)) {
+            mCachedContentIsValid = true;
+            // XXX: Isn't the cache entry already valid?
+            MaybeMarkCacheEntryValid(this, mCacheEntry, mCacheAccess);
+        }
+        return rv;
+    }
 
     mCustomConditionalRequest =
         mRequestHead.PeekHeader(nsHttp::If_Modified_Since) ||
         mRequestHead.PeekHeader(nsHttp::If_None_Match) ||
         mRequestHead.PeekHeader(nsHttp::If_Unmodified_Since) ||
         mRequestHead.PeekHeader(nsHttp::If_Match) ||
         mRequestHead.PeekHeader(nsHttp::If_Range);
 
     if (method != nsHttp::Head && !isCachedRedirect) {
         // If the cached content-length is set and it does not match the data
         // size of the cached content, then the cached response is partial...
         // either we need to issue a byte range request or we need to refetch
         // the entire document.
+        //
+        // We exclude redirects from this check because we (usually) strip the
+        // entity when we store the cache entry, and even if we didn't, we
+        // always ignore a cached redirect's entity anyway. See bug 759043.
         PRInt64 contentLength = mCachedResponseHead->ContentLength();
         if (contentLength != PRInt64(-1)) {
             PRUint32 size;
             rv = mCacheEntry->GetDataSize(&size);
             NS_ENSURE_SUCCESS(rv, rv);
 
             if (PRInt64(size) != contentLength) {
                 LOG(("Cached data size does not match the Content-Length header "
@@ -2697,22 +2760,29 @@ nsHttpChannel::CheckCache()
                     mCachedResponseHead->PeekHeader(nsHttp::Content_Encoding)
                     != nsnull;
                 if ((PRInt64(size) < contentLength) &&
                      size > 0 &&
                      !hasContentEncoding &&
                      mCachedResponseHead->IsResumable() &&
                      !mCustomConditionalRequest &&
                      !mCachedResponseHead->NoStore()) {
-                    // looks like a partial entry we can reuse
+                    // looks like a partial entry we can reuse; add If-Range
+                    // and Range headers.
                     rv = SetupByteRangeRequest(size);
-                    NS_ENSURE_SUCCESS(rv, rv);
-                    mCachedContentIsPartial = true;
+                    mCachedContentIsPartial = NS_SUCCEEDED(rv);
+                    if (mCachedContentIsPartial) {
+                        rv = StartBufferingCachedEntity(usingSSL);
+                    } else {
+                        // Make the request unconditional again.
+                        mRequestHead.ClearHeader(nsHttp::Range);
+                        mRequestHead.ClearHeader(nsHttp::If_Range);
+                    }
                 }
-                return NS_OK;
+                return rv;
             }
         }
     }
 
     bool doValidation = false;
     bool canAddImsHeader = true;
 
     // Cached entry is not the entity we request (see bug #633743)
@@ -2876,18 +2946,44 @@ nsHttpChannel::CheckCache()
             val = mCachedResponseHead->PeekHeader(nsHttp::ETag);
             if (val)
                 mRequestHead.SetHeader(nsHttp::If_None_Match,
                                        nsDependentCString(val));
             mDidReval = true;
         }
     }
 
-    LOG(("nsHTTPChannel::CheckCache exit [this=%p doValidation=%d]\n", this, doValidation));
-    return NS_OK;
+    // If there's any possibility that we may use the cached response's entity
+    // then start reading it into memory now. If we have to revalidate the
+    // entry and the revalidation fails, this will be a wasted effort, but
+    // it is much more likely that either we don't need to revalidate the entry
+    // or the entry will successfully revalidate.
+    if (mCachedContentIsValid || mDidReval) {
+        rv = StartBufferingCachedEntity(usingSSL);
+        if (NS_FAILED(rv)) {
+            // If we can't get the entity then we have to act as though we
+            // don't have the cache entry.
+            if (mDidReval) {
+                // Make the request unconditional again.
+                mRequestHead.ClearHeader(nsHttp::If_Modified_Since);
+                mRequestHead.ClearHeader(nsHttp::ETag);
+                mDidReval = false;
+            }
+            mCachedContentIsValid = false;
+        }
+    }
+
+    if (mCachedContentIsValid) {
+        // XXX: Isn't the cache entry already valid?
+        MaybeMarkCacheEntryValid(this, mCacheEntry, mCacheAccess);
+    }
+
+    LOG(("nsHTTPChannel::CheckCache exit [this=%p doValidation=%d]\n",
+         this, doValidation));
+    return rv;
 }
 
 bool
 nsHttpChannel::MustValidateBasedOnQueryUrl()
 {
     // RFC 2616, section 13.9 states that GET-requests with a query-url
     // MUST NOT be treated as fresh unless the server explicitly provides
     // an expiration-time in the response. See bug #468594
@@ -2944,77 +3040,195 @@ nsHttpChannel::ShouldUpdateOfflineCacheE
 
     if (docLastModifiedTime > offlineLastModifiedTime) {
         return true;
     }
 
     return false;
 }
 
-// If the data in the cache hasn't expired, then there's no need to
-// talk with the server, not even to do an if-modified-since.  This
-// method creates a stream from the cache, synthesizing all the various
-// channel-related events.
 nsresult
-nsHttpChannel::ReadFromCache()
+nsHttpChannel::StartBufferingCachedEntity(bool usingSSL)
 {
-    nsresult rv;
-
+    if (usingSSL) {
+        nsresult rv = mCacheEntry->GetSecurityInfo(
+                                      getter_AddRefs(mCachedSecurityInfo));
+        if (NS_FAILED(rv)) {
+            LOG(("failed to parse security-info [channel=%p, entry=%p]",
+                 this, mCacheEntry.get()));
+            NS_WARNING("failed to parse security-info");
+            return rv;
+        }
+        MOZ_ASSERT(mCachedSecurityInfo);
+        if (!mCachedSecurityInfo) {
+            LOG(("mCacheEntry->GetSecurityInfo returned success but did not "
+                 "return the security info [channel=%p, entry=%p]",
+                 this, mCacheEntry.get()));
+            return NS_ERROR_UNEXPECTED; // XXX error code
+        }
+    }
+
+    nsresult rv = NS_OK;
+
+    // Keep the conditions below in sync with the conditions in ReadFromCache.
+
+    if (WillRedirect(mCachedResponseHead)) {
+        // Do not even try to read the entity for a redirect because we do not
+        // return an entity to the application when we process redirects.
+        LOG(("Will skip read of cached redirect entity\n"));
+        return NS_OK;
+    }
+
+    if ((mLoadFlags & LOAD_ONLY_IF_MODIFIED) && !mCachedContentIsPartial) {
+        // For LOAD_ONLY_IF_MODIFIED, we usually don't have to deal with the
+        // cached entity. 
+        if (!mCacheForOfflineUse) {
+            LOG(("Will skip read from cache based on LOAD_ONLY_IF_MODIFIED "
+                 "load flag\n"));
+            return NS_OK;
+        }
+
+        // If offline caching has been requested and the offline cache needs
+        // updating, we must complete the call even if the main cache entry
+        // is up to date. We don't know yet for sure whether the offline
+        // cache needs updating because at this point we haven't opened it
+        // for writing yet, so we have to start reading the cached entity now
+        // just in case.
+        LOG(("May skip read from cache based on LOAD_ONLY_IF_MODIFIED "
+              "load flag\n"));
+    }
+
+    // Open an input stream for the entity, but DO NOT create/connect
+    // mCachePump; that is done only when we decide to actually read the
+    // cached entity. By opening the input stream here, we allow the stream
+    // transport service to start reading the entity on one of its
+    // background threads while we do validation (if any).
+
+    nsCOMPtr<nsIInputStream> wrapper;
+
+    nsCOMPtr<nsIInputStream> stream;
+    nsCOMPtr<nsITransport> transport;
+
+    static NS_DEFINE_CID(kStreamTransportServiceCID,
+                          NS_STREAMTRANSPORTSERVICE_CID);
+    nsCOMPtr<nsIStreamTransportService> sts =
+        do_GetService(kStreamTransportServiceCID, &rv);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = mCacheEntry->OpenInputStream(0, getter_AddRefs(stream));
+    if (NS_SUCCEEDED(rv)) {
+        rv = sts->CreateInputTransport(stream, PRInt64(-1), PRInt64(-1),
+                                        true, getter_AddRefs(transport));
+    }
+    if (NS_SUCCEEDED(rv)) {
+        rv = transport->OpenInputStream(0, 0, 0, getter_AddRefs(wrapper));
+    }
+    if (NS_SUCCEEDED(rv)) {
+        mCacheAsyncInputStream = do_QueryInterface(wrapper, &rv);
+    }
+    if (NS_SUCCEEDED(rv)) {
+        LOG(("Opened cache input stream [channel=%p, wrapper=%p, "
+              "transport=%p, stream=%p]", this, wrapper.get(),
+              transport.get(), stream.get()));
+    } else {
+        LOG(("Failed to open cache input stream [channel=%p, "
+              "wrapper=%p, transport=%p, stream=%p]", this,
+              wrapper.get(), transport.get(), stream.get()));
+
+        if (wrapper)
+            wrapper->Close();
+        if (stream)
+            stream->Close();
+    }
+
+    return rv;
+}
+
+// Actually process the cached response that we started to handle in CheckCache
+// and/or StartBufferingCachedEntity.
+nsresult
+nsHttpChannel::ReadFromCache(bool alreadyMarkedValid)
+{
     NS_ENSURE_TRUE(mCacheEntry, NS_ERROR_FAILURE);
     NS_ENSURE_TRUE(mCachedContentIsValid, NS_ERROR_FAILURE);
 
     LOG(("nsHttpChannel::ReadFromCache [this=%p] "
          "Using cached copy of: %s\n", this, mSpec.get()));
 
     if (mCachedResponseHead)
         mResponseHead = mCachedResponseHead;
 
     UpdateInhibitPersistentCachingFlag();
 
     // if we don't already have security info, try to get it from the cache 
     // entry. there are two cases to consider here: 1) we are just reading
     // from the cache, or 2) this may be due to a 304 not modified response,
     // in which case we could have security info from a socket transport.
     if (!mSecurityInfo)
-        mCacheEntry->GetSecurityInfo(getter_AddRefs(mSecurityInfo));
-
-    if ((mCacheAccess & nsICache::ACCESS_WRITE) && !mCachedContentIsPartial) {
-        // We have write access to the cache, but we don't need to go to the
-        // server to validate at this time, so just mark the cache entry as
-        // valid in order to allow others access to this cache entry.
-        mCacheEntry->MarkValid();
-    }
-
-    // if this is a cached redirect, we must process the redirect asynchronously
-    // since AsyncOpen may not have returned yet.  Make sure there is a Location
-    // header, otherwise we'll have to treat this like a normal 200 response.
-    if (mResponseHead && (mResponseHead->Status() / 100 == 3) 
-                      && (mResponseHead->PeekHeader(nsHttp::Location)))
+        mSecurityInfo = mCachedSecurityInfo;
+
+    if (!alreadyMarkedValid && !mCachedContentIsPartial) {
+        // We validated the entry, and we have write access to the cache, so
+        // mark the cache entry as valid in order to allow others access to
+        // this cache entry.
+        //
+        // TODO: This should be done asynchronously so we don't take the cache
+        // service lock on the main thread.
+        MaybeMarkCacheEntryValid(this, mCacheEntry, mCacheAccess);
+    }
+
+    nsresult rv;
+
+    // Keep the conditions below in sync with the conditions in
+    // StartBufferingCachedEntity.
+
+    if (WillRedirect(mResponseHead)) {
+        // TODO: Bug 759040 - We should call HandleAsyncRedirect directly here,
+        // to avoid event dispatching latency.
+        MOZ_ASSERT(!mCacheAsyncInputStream);
+        LOG(("Skipping skip read of cached redirect entity\n"));
         return AsyncCall(&nsHttpChannel::HandleAsyncRedirect);
-
-    // have we been configured to skip reading from the cache?
-    if ((mLoadFlags & LOAD_ONLY_IF_MODIFIED) && !mCachedContentIsPartial &&
-        !ShouldUpdateOfflineCacheEntry()) {
-        // if offline caching has been requested and the offline cache needs
-        // updating, complete the call even if the main cache entry is
-        // up-to-date
-        LOG(("skipping read from cache based on LOAD_ONLY_IF_MODIFIED "
-              "load flag\n"));
-        return AsyncCall(&nsHttpChannel::HandleAsyncNotModified);
-    }
-
-    // open input stream for reading...
-    nsCOMPtr<nsIInputStream> stream;
-    rv = mCacheEntry->OpenInputStream(0, getter_AddRefs(stream));
-    if (NS_FAILED(rv)) return rv;
-
-    rv = nsInputStreamPump::Create(getter_AddRefs(mCachePump),
-                                   stream, PRInt64(-1), PRInt64(-1), 0, 0,
-                                   true);
-    if (NS_FAILED(rv)) return rv;
+    }
+    
+    if ((mLoadFlags & LOAD_ONLY_IF_MODIFIED) && !mCachedContentIsPartial) {
+        if (!mCacheForOfflineUse) {
+            LOG(("Skipping read from cache based on LOAD_ONLY_IF_MODIFIED "
+                 "load flag\n"));
+            MOZ_ASSERT(!mCacheAsyncInputStream);
+            // TODO: Bug 759040 - We should call HandleAsyncNotModified directly
+            // here, to avoid event dispatching latency.
+            return AsyncCall(&nsHttpChannel::HandleAsyncNotModified);
+        }
+        
+        if (!ShouldUpdateOfflineCacheEntry()) {
+            LOG(("Skipping read from cache based on LOAD_ONLY_IF_MODIFIED "
+                 "load flag (mCacheForOfflineUse case)\n"));
+            mCacheAsyncInputStream.CloseAndRelease();
+            // TODO: Bug 759040 - We should call HandleAsyncNotModified directly
+            // here, to avoid event dispatching latency.
+            return AsyncCall(&nsHttpChannel::HandleAsyncNotModified);
+        }
+    }
+
+    MOZ_ASSERT(mCacheAsyncInputStream);
+    if (!mCacheAsyncInputStream) {
+        NS_ERROR("mCacheAsyncInputStream is null but we're expecting to "
+                        "be able to read from it.");
+        return NS_ERROR_UNEXPECTED;
+    }
+
+
+    nsCOMPtr<nsIAsyncInputStream> inputStream = mCacheAsyncInputStream.forget();
+ 
+    rv = nsInputStreamPump::Create(getter_AddRefs(mCachePump), inputStream,
+                                   PRInt64(-1), PRInt64(-1), 0, 0, true);
+    if (NS_FAILED(rv)) {
+        inputStream->Close();
+        return rv;
+    }
 
     rv = mCachePump->AsyncRead(this, mListenerContext);
     if (NS_FAILED(rv)) return rv;
 
     if (mTimingEnabled)
         mCacheReadStart = mozilla::TimeStamp::Now();
 
     PRUint32 suspendCount = mSuspendCount;
@@ -3022,16 +3236,18 @@ nsHttpChannel::ReadFromCache()
         mCachePump->Suspend();
 
     return NS_OK;
 }
 
 void
 nsHttpChannel::CloseCacheEntry(bool doomOnFailure)
 {
+    mCacheAsyncInputStream.CloseAndRelease();
+
     if (!mCacheEntry)
         return;
 
     LOG(("nsHttpChannel::CloseCacheEntry [this=%p] mStatus=%x mCacheAccess=%x",
          this, mStatus, mCacheAccess));
 
     // If we have begun to create or replace a cache entry, and that cache
     // entry is not complete and not resumable, then it needs to be doomed.
@@ -3770,16 +3986,17 @@ nsHttpChannel::Cancel(nsresult status)
     mCanceled = true;
     mStatus = status;
     if (mProxyRequest)
         mProxyRequest->Cancel(status);
     if (mTransaction)
         gHttpHandler->CancelTransaction(mTransaction, status);
     if (mTransactionPump)
         mTransactionPump->Cancel(status);
+    mCacheAsyncInputStream.CloseAndRelease();
     if (mCachePump)
         mCachePump->Cancel(status);
     if (mAuthProvider)
         mAuthProvider->Cancel(status);
     return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -5392,8 +5609,10 @@ nsHttpChannel::DetermineCacheAccess(nsCa
 }
 
 void
 nsHttpChannel::AsyncOnExamineCachedResponse()
 {
     gHttpHandler->OnExamineCachedResponse(this);
 
 }
+
+} } // namespace mozilla::net
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -25,21 +25,21 @@
 #include "nsICancelable.h"
 #include "nsIHttpAuthenticableChannel.h"
 #include "nsIHttpChannelAuthProvider.h"
 #include "nsIAsyncVerifyRedirectCallback.h"
 #include "nsICryptoHash.h"
 #include "nsITimedChannel.h"
 #include "nsDNSPrefetch.h"
 #include "TimingStruct.h"
+#include "AutoClose.h"
 
 class nsAHttpConnection;
-class AutoRedirectVetoNotifier;
 
-using namespace mozilla::net;
+namespace mozilla { namespace net {
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel
 //-----------------------------------------------------------------------------
 
 class nsHttpChannel : public HttpBaseChannel
                     , public HttpAsyncAborter<nsHttpChannel>
                     , public nsIStreamListener
@@ -216,17 +216,18 @@ private:
         nsresult aResult);
     nsresult OnCacheEntryAvailableInternal(nsICacheEntryDescriptor *entry,
                                            nsCacheAccessMode access,
                                            nsresult status);
     nsresult GenerateCacheKey(PRUint32 postID, nsACString &key);
     nsresult UpdateExpirationTime();
     nsresult CheckCache();
     bool ShouldUpdateOfflineCacheEntry();
-    nsresult ReadFromCache();
+    nsresult StartBufferingCachedEntity(bool usingSSL);
+    nsresult ReadFromCache(bool alreadyMarkedValid);
     void     CloseCacheEntry(bool doomOnFailure);
     void     CloseOfflineCacheEntry();
     nsresult InitCacheEntry();
     void     UpdateInhibitPersistentCachingFlag();
     nsresult InitOfflineCacheEntry();
     nsresult AddCacheEntryHeaders(nsICacheEntryDescriptor *entry);
     nsresult StoreAuthorizationMetaData(nsICacheEntryDescriptor *entry);
     nsresult FinalizeCacheEntry();
@@ -286,18 +287,21 @@ private:
 
     nsRefPtr<nsInputStreamPump>       mTransactionPump;
     nsRefPtr<nsHttpTransaction>       mTransaction;
 
     PRUint64                          mLogicalOffset;
 
     // cache specific data
     nsCOMPtr<nsICacheEntryDescriptor> mCacheEntry;
+    // We must close mCacheAsyncInputStream explicitly to avoid leaks.
+    AutoClose<nsIAsyncInputStream>    mCacheAsyncInputStream;
     nsRefPtr<nsInputStreamPump>       mCachePump;
     nsAutoPtr<nsHttpResponseHead>     mCachedResponseHead;
+    nsCOMPtr<nsISupports>             mCachedSecurityInfo;
     nsCacheAccessMode                 mCacheAccess;
     PRUint32                          mPostID;
     PRUint32                          mRequestTime;
 
     typedef nsresult (nsHttpChannel:: *nsOnCacheEntryAvailableCallback)(
         nsICacheEntryDescriptor *, nsCacheAccessMode, nsresult);
     nsOnCacheEntryAvailableCallback   mOnCacheEntryAvailableCallback;
 
@@ -370,9 +374,11 @@ private: // cache telemetry
         kCacheHit = 1,
         kCacheHitViaReval = 2,
         kCacheMissedViaReval = 3,
         kCacheMissed = 4
     };
     bool mDidReval;
 };
 
+} } // namespace mozilla::net
+
 #endif // nsHttpChannel_h__
--- a/netwerk/protocol/http/nsHttpResponseHead.cpp
+++ b/netwerk/protocol/http/nsHttpResponseHead.cpp
@@ -375,17 +375,21 @@ nsHttpResponseHead::MustValidateIfExpire
     return HasHeaderValue(nsHttp::Cache_Control, "must-revalidate");
 }
 
 bool
 nsHttpResponseHead::IsResumable() const
 {
     // even though some HTTP/1.0 servers may support byte range requests, we're not
     // going to bother with them, since those servers wouldn't understand If-Range.
-    return mVersion >= NS_HTTP_VERSION_1_1 &&
+    // Also, while in theory it may be possible to resume when the status code
+    // is not 200, it is unlikely to be worth the trouble, especially for
+    // non-2xx responses.
+    return mStatus == 200 &&
+           mVersion >= NS_HTTP_VERSION_1_1 &&
            PeekHeader(nsHttp::Content_Length) && 
           (PeekHeader(nsHttp::ETag) || PeekHeader(nsHttp::Last_Modified)) &&
            HasHeaderValue(nsHttp::Accept_Ranges, "bytes");
 }
 
 bool
 nsHttpResponseHead::ExpiresInPast() const
 {