Bug 1367551 - Cancel pushes when we already have the item in cache. r?mcmanus draft
authorNicholas Hurley <hurley@todesschaf.org>
Wed, 28 Jun 2017 10:34:55 -0700
changeset 617594 b0e7bb26ad9264fe14b7d38a4bb944bb2a0346d1
parent 614544 462d7561089c98e33382384896434861ad7bc491
child 619371 363de666a7773fac82a93728ddb353bdd077c2e9
push id71094
push userbmo:hurley@mozilla.com
push dateFri, 28 Jul 2017 18:14:53 +0000
reviewersmcmanus
bugs1367551
milestone56.0a1
Bug 1367551 - Cancel pushes when we already have the item in cache. r?mcmanus MozReview-Commit-ID: 24N0Jm85wcC
netwerk/protocol/http/Http2Session.cpp
netwerk/protocol/http/Http2Session.h
netwerk/protocol/http/Http2Stream.cpp
netwerk/protocol/http/Http2Stream.h
netwerk/protocol/http/nsHttp.cpp
netwerk/protocol/http/nsHttp.h
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/test/unit/test_http2.js
testing/xpcshell/moz-http2/moz-http2.js
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -32,16 +32,21 @@
 #include "nsISupportsPriority.h"
 #include "nsStandardURL.h"
 #include "nsURLHelper.h"
 #include "prnetdb.h"
 #include "sslt.h"
 #include "mozilla/Sprintf.h"
 #include "nsSocketTransportService2.h"
 #include "nsNetUtil.h"
+#include "nsICacheEntry.h"
+#include "nsICacheStorageService.h"
+#include "nsICacheStorage.h"
+#include "CacheControlParser.h"
+#include "LoadContextInfo.h"
 
 namespace mozilla {
 namespace net {
 
 // Http2Session has multiple inheritance of things that implement nsISupports
 NS_IMPL_ADDREF(Http2Session)
 NS_IMPL_RELEASE(Http2Session)
 NS_INTERFACE_MAP_BEGIN(Http2Session)
@@ -1804,25 +1809,25 @@ Http2Session::RecvPushPromise(Http2Sessi
     self->CleanupStream(pushedStream, NS_ERROR_FAILURE, PROTOCOL_ERROR);
     self->ResetDownstreamState();
     return NS_OK;
   }
 
   // does the pushed origin belong on this connection?
   LOG3(("Http2Session::RecvPushPromise %p origin check %s", self,
         pushedStream->Origin().get()));
-  RefPtr<nsStandardURL> pushedURL;
-  rv = Http2Stream::MakeOriginURL(pushedStream->Origin(), pushedURL);
+  RefPtr<nsStandardURL> pushedOrigin;
+  rv = Http2Stream::MakeOriginURL(pushedStream->Origin(), pushedOrigin);
   nsAutoCString pushedHostName;
   int32_t pushedPort = -1;
   if (NS_SUCCEEDED(rv)) {
-    rv = pushedURL->GetHost(pushedHostName);
+    rv = pushedOrigin->GetHost(pushedHostName);
   }
   if (NS_SUCCEEDED(rv)) {
-    rv = pushedURL->GetPort(&pushedPort);
+    rv = pushedOrigin->GetPort(&pushedPort);
   }
   if (NS_FAILED(rv) ||
       !self->TestJoinConnection(pushedHostName, pushedPort)) {
     LOG3(("Http2Session::RecvPushPromise %p pushed stream mismatched origin %s\n",
           self, pushedStream->Origin().get()));
     self->CleanupStream(pushedStream, NS_ERROR_FAILURE, REFUSED_STREAM_ERROR);
     self->ResetDownstreamState();
     return NS_OK;
@@ -1835,29 +1840,219 @@ Http2Session::RecvPushPromise(Http2Sessi
     LOG3(("Http2Session::RecvPushPromise %p place stream into session cache\n", self));
     if (!cache->RegisterPushedStreamHttp2(key, pushedStream)) {
       // This only happens if they've already pushed us this item.
       LOG3(("Http2Session::RecvPushPromise registerPushedStream Failed\n"));
       self->CleanupStream(pushedStream, NS_ERROR_FAILURE, REFUSED_STREAM_ERROR);
       self->ResetDownstreamState();
       return NS_OK;
     }
+
+    // Kick off a lookup into the HTTP cache so we can cancel the push if it's
+    // unneeded (we already have it in our local regular cache). See bug 1367551.
+    nsCOMPtr<nsICacheStorageService> css =
+      do_GetService("@mozilla.org/netwerk/cache-storage-service;1");
+    mozilla::OriginAttributes oa;
+    pushedStream->GetOriginAttributes(&oa);
+    RefPtr<LoadContextInfo> lci = GetLoadContextInfo(false, oa);
+    nsCOMPtr<nsICacheStorage> ds;
+    css->DiskCacheStorage(lci, false, getter_AddRefs(ds));
+    // Build up our full URL for the cache lookup
+    nsAutoCString spec;
+    spec.Assign(pushedStream->Origin());
+    spec.Append(pushedStream->Path());
+    RefPtr<nsStandardURL> pushedURL;
+    // Nifty trick: this doesn't actually do anything origin-specific, it's just
+    // named that way. So by passing it the full spec here, we get a URL with
+    // the full path.
+    // Another nifty trick! Even though this is using nsIURIs (which are not
+    // generally ok off the main thread), since we're not using the protocol
+    // handler to create any URIs, this will work just fine here. Don't try this
+    // at home, though, kids. I'm a trained professional.
+    if (NS_SUCCEEDED(Http2Stream::MakeOriginURL(spec, pushedURL))) {
+      LOG3(("Http2Session::RecvPushPromise %p check disk cache for entry", self));
+      RefPtr<CachePushCheckCallback> cpcc = new CachePushCheckCallback(self, promisedID, pushedStream->GetRequestString());
+      if (NS_FAILED(ds->AsyncOpenURI(pushedURL, EmptyCString(), nsICacheStorage::OPEN_READONLY|nsICacheStorage::OPEN_SECRETLY, cpcc))) {
+        LOG3(("Http2Session::RecvPushPromise %p failed to open cache entry for push check", self));
+      }
+    }
   }
 
   pushedStream->SetHTTPState(Http2Stream::RESERVED_BY_REMOTE);
   static_assert(Http2Stream::kWorstPriority >= 0,
                 "kWorstPriority out of range");
   uint8_t priorityWeight = (nsISupportsPriority::PRIORITY_LOWEST + 1) -
     (Http2Stream::kWorstPriority - Http2Stream::kNormalPriority);
   pushedStream->SetPriority(Http2Stream::kWorstPriority);
   self->GeneratePriority(promisedID, priorityWeight);
   self->ResetDownstreamState();
   return NS_OK;
 }
 
+NS_IMPL_ISUPPORTS(Http2Session::CachePushCheckCallback, nsICacheEntryOpenCallback);
+
+Http2Session::CachePushCheckCallback::CachePushCheckCallback(Http2Session *session, uint32_t promisedID, const nsACString &requestString)
+  :mPromisedID(promisedID)
+{
+  mSession = session;
+  mRequestHead.ParseHeaderSet(requestString.BeginReading());
+}
+
+NS_IMETHODIMP
+Http2Session::CachePushCheckCallback::OnCacheEntryCheck(nsICacheEntry *entry, nsIApplicationCache *appCache, uint32_t *result)
+{
+  MOZ_ASSERT(OnSocketThread(), "Not on socket thread?!");
+
+  // We never care to fully open the entry, since we won't actually use it.
+  // We just want to be able to do all our checks to see if a future channel can
+  // use this entry, or if we need to accept the push.
+  *result = nsICacheEntryOpenCallback::ENTRY_NOT_WANTED;
+
+  bool isForcedValid = false;
+  entry->GetIsForcedValid(&isForcedValid);
+
+  nsHttpResponseHead cachedResponseHead;
+  nsresult rv = nsHttp::GetHttpResponseHeadFromCacheEntry(entry, &cachedResponseHead);
+  if (NS_FAILED(rv)) {
+    // Couldn't make sense of what's in the cache entry, go ahead and accept
+    // the push.
+    return NS_OK;
+  }
+
+  if ((cachedResponseHead.Status() / 100) != 2) {
+    // Assume the push is sending us a success, while we don't have one in the
+    // cache, so we'll accept the push.
+    return NS_OK;
+  }
+
+  // Get the method that was used to generate the cached response
+  nsXPIDLCString buf;
+  rv = entry->GetMetaDataElement("request-method", getter_Copies(buf));
+  if (NS_FAILED(rv)) {
+    // Can't check request method, accept the push
+    return NS_OK;
+  }
+  nsAutoCString pushedMethod;
+  mRequestHead.Method(pushedMethod);
+  if (!buf.Equals(pushedMethod)) {
+    // Methods don't match, accept the push
+    return NS_OK;
+  }
+
+  int64_t size, contentLength;
+  rv = nsHttp::CheckPartial(entry, &size, &contentLength, &cachedResponseHead);
+  if (NS_FAILED(rv)) {
+    // Couldn't figure out if this was partial or not, accept the push.
+    return NS_OK;
+  }
+
+  if (size == int64_t(-1) || contentLength != size) {
+    // This is partial content in the cache, accept the push.
+    return NS_OK;
+  }
+
+  nsAutoCString requestedETag;
+  if (NS_FAILED(mRequestHead.GetHeader(nsHttp::If_Match, requestedETag))) {
+    // Can't check etag
+    return NS_OK;
+  }
+  if (!requestedETag.IsEmpty()) {
+    nsAutoCString cachedETag;
+    if (NS_FAILED(cachedResponseHead.GetHeader(nsHttp::ETag, cachedETag))) {
+      // Can't check etag
+      return NS_OK;
+    }
+    if (!requestedETag.Equals(cachedETag)) {
+      // ETags don't match, accept the push.
+      return NS_OK;
+    }
+  }
+
+  nsAutoCString imsString;
+  Unused << mRequestHead.GetHeader(nsHttp::If_Modified_Since, imsString);
+  if (!buf.IsEmpty()) {
+    uint32_t ims = buf.ToInteger(&rv);
+    uint32_t lm;
+    rv = cachedResponseHead.GetLastModifiedValue(&lm);
+    if (NS_SUCCEEDED(rv) && lm && lm < ims) {
+      // The push appears to be newer than what's in our cache, accept it.
+      return NS_OK;
+    }
+  }
+
+  nsAutoCString cacheControlRequestHeader;
+  Unused << mRequestHead.GetHeader(nsHttp::Cache_Control, cacheControlRequestHeader);
+  CacheControlParser cacheControlRequest(cacheControlRequestHeader);
+  if (cacheControlRequest.NoStore()) {
+    // Don't use a no-store cache entry, accept the push.
+    return NS_OK;
+  }
+
+  nsXPIDLCString cachedAuth;
+  rv = entry->GetMetaDataElement("auth", getter_Copies(cachedAuth));
+  if (NS_SUCCEEDED(rv)) {
+    uint32_t lastModifiedTime;
+    rv = entry->GetLastModified(&lastModifiedTime);
+    if (NS_SUCCEEDED(rv)) {
+      if ((gHttpHandler->SessionStartTime() > lastModifiedTime) && !cachedAuth.IsEmpty()) {
+        // Need to revalidate this, as the auth is old. Accept the push.
+        return NS_OK;
+      }
+
+      if (cachedAuth.IsEmpty() && mRequestHead.HasHeader(nsHttp::Authorization)) {
+        // They're pushing us something with auth, but we didn't cache anything
+        // with auth. Accept the push.
+        return NS_OK;
+      }
+    }
+  }
+
+  bool weaklyFramed, isImmutable;
+  nsHttp::DetermineFramingAndImmutability(entry, &cachedResponseHead, true,
+                                          &weaklyFramed, &isImmutable);
+
+  // We'll need this value in later computations...
+  uint32_t lastModifiedTime;
+  rv = entry->GetLastModified(&lastModifiedTime);
+  if (NS_FAILED(rv)) {
+    // Ugh, this really sucks. OK, accept the push.
+    return NS_OK;
+  }
+
+  // Determine if this is the first time that this cache entry
+  // has been accessed during this session.
+  bool fromPreviousSession =
+          (gHttpHandler->SessionStartTime() > lastModifiedTime);
+
+  bool validationRequired = nsHttp::ValidationRequired(isForcedValid,
+    &cachedResponseHead, 0/*NWGH: ??? - loadFlags*/, false, isImmutable, false, mRequestHead, entry,
+    cacheControlRequest, fromPreviousSession);
+
+  if (validationRequired) {
+    // A real channel would most likely hit the net at this point, so let's
+    // accept the push.
+    return NS_OK;
+  }
+
+  // If we get here, then we would be able to use this cache entry. Cancel the
+  // push so as not to waste any more bandwidth.
+  mSession->CleanupStream(mPromisedID, NS_ERROR_FAILURE, Http2Session::REFUSED_STREAM_ERROR);
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+Http2Session::CachePushCheckCallback::OnCacheEntryAvailable(
+    nsICacheEntry *entry, bool isNew, nsIApplicationCache *appCache,
+    nsresult result)
+{
+  // Nothing to do here, all the work is in OnCacheEntryCheck.
+  return NS_OK;
+}
+
 nsresult
 Http2Session::RecvPing(Http2Session *self)
 {
   MOZ_ASSERT(self->mInputFrameType == FRAME_TYPE_PING);
 
   LOG3(("Http2Session::RecvPing %p PING Flags 0x%X.", self,
         self->mInputFrameFlags));
 
--- a/netwerk/protocol/http/Http2Session.h
+++ b/netwerk/protocol/http/Http2Session.h
@@ -12,16 +12,18 @@
 #include "ASpdySession.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/UniquePtr.h"
 #include "nsAHttpConnection.h"
 #include "nsClassHashtable.h"
 #include "nsDataHashtable.h"
 #include "nsDeque.h"
 #include "nsHashKeys.h"
+#include "nsHttpRequestHead.h"
+#include "nsICacheEntryOpenCallback.h"
 
 #include "Http2Compression.h"
 
 class nsISocketTransport;
 
 namespace mozilla {
 namespace net {
 
@@ -524,16 +526,32 @@ private:
 
   bool RealJoinConnection(const nsACString &hostname, int32_t port, bool jk);
   bool TestOriginFrame(const nsACString &name, int32_t port);
   bool mOriginFrameActivated;
   nsDataHashtable<nsCStringHashKey, bool> mOriginFrame;
 
   nsDataHashtable<nsCStringHashKey, bool> mJoinConnectionCache;
 
+  class CachePushCheckCallback final : public nsICacheEntryOpenCallback
+  {
+  public:
+    CachePushCheckCallback(Http2Session *session, uint32_t promisedID, const nsACString &requestString);
+
+    NS_DECL_ISUPPORTS
+    NS_DECL_NSICACHEENTRYOPENCALLBACK
+
+  private:
+    ~CachePushCheckCallback() { }
+
+    RefPtr<Http2Session> mSession;
+    uint32_t mPromisedID;
+    nsHttpRequestHead mRequestHead;
+  };
+
 private:
 /// connect tunnels
   void DispatchOnTunnel(nsAHttpTransaction *, nsIInterfaceRequestor *);
   void CreateTunnel(nsHttpTransaction *, nsHttpConnectionInfo *, nsIInterfaceRequestor *);
   void RegisterTunnel(Http2Stream *);
   void UnRegisterTunnel(Http2Stream *);
   uint32_t FindTunnelCount(nsHttpConnectionInfo *);
   nsDataHashtable<nsCStringHashKey, uint32_t> mTunnelHash;
--- a/netwerk/protocol/http/Http2Stream.cpp
+++ b/netwerk/protocol/http/Http2Stream.cpp
@@ -1552,10 +1552,20 @@ Http2Stream::Finish0RTT(bool aRestart, b
     nsHttpTransaction *trans = mTransaction->QueryHttpTransaction();
     if (trans) {
       trans->Refused0RTT();
     }
   }
   return rv;
 }
 
+nsresult
+Http2Stream::GetOriginAttributes(mozilla::OriginAttributes *oa)
+{
+  if (!mSocketTransport) {
+    return NS_ERROR_UNEXPECTED;
+  }
+
+  return mSocketTransport->GetOriginAttributes(oa);
+}
+
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/Http2Stream.h
+++ b/netwerk/protocol/http/Http2Stream.h
@@ -163,16 +163,18 @@ public:
   static MOZ_MUST_USE nsresult MakeOriginURL(const nsACString &scheme,
                                              const nsACString &origin,
                                              RefPtr<nsStandardURL> &url);
 
   // Mirrors nsAHttpTransaction
   bool Do0RTT();
   nsresult Finish0RTT(bool aRestart, bool aAlpnIgnored);
 
+  nsresult GetOriginAttributes(mozilla::OriginAttributes *oa);
+
 protected:
   static void CreatePushHashKey(const nsCString &scheme,
                                 const nsCString &hostHeader,
                                 const mozilla::OriginAttributes &originAttributes,
                                 uint64_t serial,
                                 const nsACString& pathInfo,
                                 nsCString &outOrigin,
                                 nsCString &outKey);
--- a/netwerk/protocol/http/nsHttp.cpp
+++ b/netwerk/protocol/http/nsHttp.cpp
@@ -3,29 +3,36 @@
 /* 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/. */
 
 // HttpLog.h should generally be included first
 #include "HttpLog.h"
 
 #include "nsHttp.h"
+#include "CacheControlParser.h"
 #include "PLDHashTable.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/HashFunctions.h"
 #include "nsCRT.h"
+#include "nsHttpRequestHead.h"
+#include "nsHttpResponseHead.h"
+#include "nsICacheEntry.h"
+#include "nsIRequest.h"
 #include <errno.h>
 
 namespace mozilla {
 namespace net {
 
 // define storage for all atoms
-#define HTTP_ATOM(_name, _value) nsHttpAtom nsHttp::_name = { _value };
+namespace nsHttp {
+#define HTTP_ATOM(_name, _value) nsHttpAtom _name = { _value };
 #include "nsHttpAtomList.h"
 #undef HTTP_ATOM
+}
 
 // find out how many atoms we have
 #define HTTP_ATOM(_name, _value) Unused_ ## _name,
 enum {
 #include "nsHttpAtomList.h"
     NUM_HTTP_ATOMS
 };
 #undef HTTP_ATOM
@@ -84,34 +91,35 @@ static const PLDHashTableOps ops = {
     StringHash,
     StringCompare,
     PLDHashTable::MoveEntryStub,
     PLDHashTable::ClearEntryStub,
     nullptr
 };
 
 // We put the atoms in a hash table for speedy lookup.. see ResolveAtom.
+namespace nsHttp {
 nsresult
-nsHttp::CreateAtomTable()
+CreateAtomTable()
 {
     MOZ_ASSERT(!sAtomTable, "atom table already initialized");
 
     if (!sLock) {
         sLock = new Mutex("nsHttp.sLock");
     }
 
     // The initial length for this table is a value greater than the number of
     // known atoms (NUM_HTTP_ATOMS) because we expect to encounter a few random
     // headers right off the bat.
     sAtomTable = new PLDHashTable(&ops, sizeof(PLDHashEntryStub),
                                   NUM_HTTP_ATOMS + 10);
 
     // fill the table with our known atoms
     const char *const atoms[] = {
-#define HTTP_ATOM(_name, _value) nsHttp::_name._val,
+#define HTTP_ATOM(_name, _value) _name._val,
 #include "nsHttpAtomList.h"
 #undef HTTP_ATOM
         nullptr
     };
 
     for (int i = 0; atoms[i]; ++i) {
         auto stub = static_cast<PLDHashEntryStub*>
                                (sAtomTable->Add(atoms[i], fallible));
@@ -121,40 +129,40 @@ nsHttp::CreateAtomTable()
         MOZ_ASSERT(!stub->key, "duplicate static atom");
         stub->key = atoms[i];
     }
 
     return NS_OK;
 }
 
 void
-nsHttp::DestroyAtomTable()
+DestroyAtomTable()
 {
     delete sAtomTable;
     sAtomTable = nullptr;
 
     while (sHeapAtoms) {
         HttpHeapAtom *next = sHeapAtoms->next;
         free(sHeapAtoms);
         sHeapAtoms = next;
     }
 
     delete sLock;
     sLock = nullptr;
 }
 
 Mutex *
-nsHttp::GetLock()
+GetLock()
 {
     return sLock;
 }
 
 // this function may be called from multiple threads
 nsHttpAtom
-nsHttp::ResolveAtom(const char *str)
+ResolveAtom(const char *str)
 {
     nsHttpAtom atom = { nullptr };
 
     if (!str || !sAtomTable)
         return atom;
 
     MutexAutoLock lock(*sLock);
 
@@ -208,32 +216,32 @@ static const char kValidTokenMap[128] = 
     1, 1, 1, 0, 0, 0, 1, 1, //  88
 
     1, 1, 1, 1, 1, 1, 1, 1, //  96
     1, 1, 1, 1, 1, 1, 1, 1, // 104
     1, 1, 1, 1, 1, 1, 1, 1, // 112
     1, 1, 1, 0, 1, 0, 1, 0  // 120
 };
 bool
-nsHttp::IsValidToken(const char *start, const char *end)
+IsValidToken(const char *start, const char *end)
 {
     if (start == end)
         return false;
 
     for (; start != end; ++start) {
         const unsigned char idx = *start;
         if (idx > 127 || !kValidTokenMap[idx])
             return false;
     }
 
     return true;
 }
 
 const char*
-nsHttp::GetProtocolVersion(uint32_t pv)
+GetProtocolVersion(uint32_t pv)
 {
     switch (pv) {
     case HTTP_VERSION_2:
     case NS_HTTP_VERSION_2_0:
         return "h2";
     case NS_HTTP_VERSION_1_0:
         return "http/1.0";
     case NS_HTTP_VERSION_1_1:
@@ -242,46 +250,46 @@ nsHttp::GetProtocolVersion(uint32_t pv)
         NS_WARNING(nsPrintfCString("Unkown protocol version: 0x%X. "
                                    "Please file a bug", pv).get());
         return "http/1.1";
     }
 }
 
 // static
 void
-nsHttp::TrimHTTPWhitespace(const nsACString& aSource, nsACString& aDest)
+TrimHTTPWhitespace(const nsACString& aSource, nsACString& aDest)
 {
   nsAutoCString str(aSource);
 
   // HTTP whitespace 0x09: '\t', 0x0A: '\n', 0x0D: '\r', 0x20: ' '
   static const char kHTTPWhitespace[] = "\t\n\r ";
   str.Trim(kHTTPWhitespace);
   aDest.Assign(str);
 }
 
 // static
 bool
-nsHttp::IsReasonableHeaderValue(const nsACString &s)
+IsReasonableHeaderValue(const nsACString &s)
 {
   // Header values MUST NOT contain line-breaks.  RFC 2616 technically
   // permits CTL characters, including CR and LF, in header values provided
   // they are quoted.  However, this can lead to problems if servers do not
   // interpret quoted strings properly.  Disallowing CR and LF here seems
   // reasonable and keeps things simple.  We also disallow a null byte.
   const nsACString::char_type* end = s.EndReading();
   for (const nsACString::char_type* i = s.BeginReading(); i != end; ++i) {
     if (*i == '\r' || *i == '\n' || *i == '\0') {
       return false;
     }
   }
   return true;
 }
 
 const char *
-nsHttp::FindToken(const char *input, const char *token, const char *seps)
+FindToken(const char *input, const char *token, const char *seps)
 {
     if (!input)
         return nullptr;
 
     int inputLen = strlen(input);
     int tokenLen = strlen(token);
 
     if (inputLen < tokenLen)
@@ -298,17 +306,17 @@ nsHttp::FindToken(const char *input, con
             return input;
         }
     }
 
     return nullptr;
 }
 
 bool
-nsHttp::ParseInt64(const char *input, const char **next, int64_t *r)
+ParseInt64(const char *input, const char **next, int64_t *r)
 {
     MOZ_ASSERT(input);
     MOZ_ASSERT(r);
 
     char *end = nullptr;
     errno = 0; // Clear errno to make sure its value is set by strtoll
     int64_t value = strtoll(input, &end, /* base */ 10);
 
@@ -323,21 +331,222 @@ nsHttp::ParseInt64(const char *input, co
     if (next) {
         *next = end;
     }
     *r = value;
     return true;
 }
 
 bool
-nsHttp::IsPermanentRedirect(uint32_t httpStatus)
+IsPermanentRedirect(uint32_t httpStatus)
 {
   return httpStatus == 301 || httpStatus == 308;
 }
 
+bool
+ValidationRequired(bool isForcedValid, nsHttpResponseHead *cachedResponseHead,
+                   uint32_t loadFlags, bool allowStaleCacheContent,
+                   bool isImmutable, bool customConditionalRequest,
+                   nsHttpRequestHead &requestHead,
+                   nsICacheEntry *entry, CacheControlParser &cacheControlRequest,
+                   bool fromPreviousSession)
+{
+    // Check isForcedValid to see if it is possible to skip validation.
+    // Don't skip validation if we have serious reason to believe that this
+    // content is invalid (it's expired).
+    // See netwerk/cache2/nsICacheEntry.idl for details
+    if (isForcedValid &&
+        (!cachedResponseHead->ExpiresInPast() ||
+         !cachedResponseHead->MustValidateIfExpired())) {
+        LOG(("NOT validating based on isForcedValid being true.\n"));
+        return false;
+    }
+
+    // If the LOAD_FROM_CACHE flag is set, any cached data can simply be used
+    if (loadFlags & nsIRequest::LOAD_FROM_CACHE || allowStaleCacheContent) {
+        LOG(("NOT validating based on LOAD_FROM_CACHE load flag\n"));
+        return false;
+    }
+
+    // If the VALIDATE_ALWAYS flag is set, any cached data won't be used until
+    // it's revalidated with the server.
+    if ((loadFlags & nsIRequest::VALIDATE_ALWAYS) && !isImmutable) {
+        LOG(("Validating based on VALIDATE_ALWAYS load flag\n"));
+        return true;
+    }
+
+    // Even if the VALIDATE_NEVER flag is set, there are still some cases in
+    // which we must validate the cached response with the server.
+    if (loadFlags & nsIRequest::VALIDATE_NEVER) {
+        LOG(("VALIDATE_NEVER set\n"));
+        // if no-store validate cached response (see bug 112564)
+        if (cachedResponseHead->NoStore()) {
+            LOG(("Validating based on no-store logic\n"));
+            return true;
+        } else {
+            LOG(("NOT validating based on VALIDATE_NEVER load flag\n"));
+            return false;
+        }
+    }
+
+    // check if validation is strictly required...
+    if (cachedResponseHead->MustValidate()) {
+        LOG(("Validating based on MustValidate() returning TRUE\n"));
+        return true;
+    }
+
+    // possibly serve from cache for a custom If-Match/If-Unmodified-Since
+    // conditional request
+    if (customConditionalRequest &&
+        !requestHead.HasHeader(nsHttp::If_Match) &&
+        !requestHead.HasHeader(nsHttp::If_Unmodified_Since)) {
+        LOG(("Validating based on a custom conditional request\n"));
+        return true;
+    }
+
+    // previously we also checked for a query-url w/out expiration
+    // and didn't do heuristic on it. but defacto that is allowed now.
+    //
+    // Check if the cache entry has expired...
+
+    bool doValidation = true;
+    uint32_t now = NowInSeconds();
+
+    uint32_t age = 0;
+    nsresult rv = cachedResponseHead->ComputeCurrentAge(now, now, &age);
+    if (NS_FAILED(rv)) {
+        return true;
+    }
+
+    uint32_t freshness = 0;
+    rv = cachedResponseHead->ComputeFreshnessLifetime(&freshness);
+    if (NS_FAILED(rv)) {
+        return true;
+    }
+
+    uint32_t expiration = 0;
+    rv = entry->GetExpirationTime(&expiration);
+    if (NS_FAILED(rv)) {
+        return true;
+    }
+
+    uint32_t maxAgeRequest, maxStaleRequest, minFreshRequest;
+
+    LOG(("  NowInSeconds()=%u, expiration time=%u, freshness lifetime=%u, age=%u",
+            now, expiration, freshness, age));
+
+    if (cacheControlRequest.NoCache()) {
+        LOG(("  validating, no-cache request"));
+        doValidation = true;
+    } else if (cacheControlRequest.MaxStale(&maxStaleRequest)) {
+        uint32_t staleTime = age > freshness ? age - freshness : 0;
+        doValidation = staleTime > maxStaleRequest;
+        LOG(("  validating=%d, max-stale=%u requested", doValidation, maxStaleRequest));
+    } else if (cacheControlRequest.MaxAge(&maxAgeRequest)) {
+        doValidation = age > maxAgeRequest;
+        LOG(("  validating=%d, max-age=%u requested", doValidation, maxAgeRequest));
+    } else if (cacheControlRequest.MinFresh(&minFreshRequest)) {
+        uint32_t freshTime = freshness > age ? freshness - age : 0;
+        doValidation = freshTime < minFreshRequest;
+        LOG(("  validating=%d, min-fresh=%u requested", doValidation, minFreshRequest));
+    } else if (now <= expiration) {
+        doValidation = false;
+        LOG(("  not validating, expire time not in the past"));
+    } else if (cachedResponseHead->MustValidateIfExpired()) {
+        doValidation = true;
+    } else if (loadFlags & nsIRequest::VALIDATE_ONCE_PER_SESSION) {
+        // If the cached response does not include expiration infor-
+        // mation, then we must validate the response, despite whether
+        // or not this is the first access this session.  This behavior
+        // is consistent with existing browsers and is generally expected
+        // by web authors.
+        if (freshness == 0)
+            doValidation = true;
+        else
+            doValidation = fromPreviousSession;
+    } else {
+        doValidation = true;
+    }
+
+    LOG(("%salidating based on expiration time\n", doValidation ? "V" : "Not v"));
+    return doValidation;
+}
+
+nsresult
+GetHttpResponseHeadFromCacheEntry(nsICacheEntry *entry, nsHttpResponseHead *cachedResponseHead)
+{
+    nsXPIDLCString buf;
+    // A "original-response-headers" metadata element holds network original headers,
+    // i.e. the headers in the form as they arrieved from the network.
+    // We need to get the network original headers first, because we need to keep them
+    // in order.
+    nsresult rv = entry->GetMetaDataElement("original-response-headers", getter_Copies(buf));
+    if (NS_SUCCEEDED(rv)) {
+        rv = cachedResponseHead->ParseCachedOriginalHeaders((char *) buf.get());
+        if (NS_FAILED(rv)) {
+            LOG(("  failed to parse original-response-headers\n"));
+        }
+    }
+
+    buf.Adopt(0);
+    // A "response-head" metadata element holds response head, e.g. response status
+    // line and headers in the form Firefox uses them internally (no dupicate
+    // headers, etc.).
+    rv = entry->GetMetaDataElement("response-head", getter_Copies(buf));
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    // Parse string stored in a "response-head" metadata element.
+    // These response headers will be merged with the orignal headers (i.e. the
+    // headers stored in a "original-response-headers" metadata element).
+    rv = cachedResponseHead->ParseCachedHead(buf.get());
+    NS_ENSURE_SUCCESS(rv, rv);
+    buf.Adopt(0);
+
+    return NS_OK;
+}
+
+nsresult
+CheckPartial(nsICacheEntry* aEntry, int64_t *aSize, int64_t *aContentLength,
+             nsHttpResponseHead *responseHead)
+{
+    nsresult rv;
+
+    rv = aEntry->GetDataSize(aSize);
+
+    if (NS_ERROR_IN_PROGRESS == rv) {
+        *aSize = -1;
+        rv = NS_OK;
+    }
+
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    if (!responseHead) {
+        return NS_ERROR_UNEXPECTED;
+    }
+
+    *aContentLength = responseHead->ContentLength();
+
+    return NS_OK;
+}
+
+void
+DetermineFramingAndImmutability(nsICacheEntry *entry,
+        nsHttpResponseHead *responseHead, bool isHttps,
+        bool *weaklyFramed, bool *isImmutable)
+{
+    nsXPIDLCString framedBuf;
+    nsresult rv = entry->GetMetaDataElement("strongly-framed", getter_Copies(framedBuf));
+    // describe this in terms of explicitly weakly framed so as to be backwards
+    // compatible with old cache contents which dont have strongly-framed makers
+    *weaklyFramed = NS_SUCCEEDED(rv) && framedBuf.EqualsLiteral("0");
+    *isImmutable = !*weaklyFramed && isHttps && responseHead->Immutable();
+}
+
+} // namespace nsHttp
+
 
 template<typename T> void
 localEnsureBuffer(UniquePtr<T[]> &buf, uint32_t newSize,
              uint32_t preserve, uint32_t &objSize)
 {
   if (objSize >= newSize)
     return;
 
--- a/netwerk/protocol/http/nsHttp.h
+++ b/netwerk/protocol/http/nsHttp.h
@@ -17,21 +17,27 @@
 
 // http version codes
 #define NS_HTTP_VERSION_UNKNOWN  0
 #define NS_HTTP_VERSION_0_9      9
 #define NS_HTTP_VERSION_1_0     10
 #define NS_HTTP_VERSION_1_1     11
 #define NS_HTTP_VERSION_2_0     20
 
+class nsICacheEntry;
+
 namespace mozilla {
 
 class Mutex;
 
 namespace net {
+    class nsHttpResponseHead;
+    class nsHttpRequestHead;
+    class CacheControlParser;
+
     enum {
         // SPDY_VERSION_2 = 2, REMOVED
         // SPDY_VERSION_3 = 3, REMOVED
         // SPDY_VERSION_31 = 4, REMOVED
         HTTP_VERSION_2 = 5
 
         // leave room for official versions. telem goes to 48
         // 24 was a internal spdy/3.1
@@ -120,93 +126,111 @@ struct nsHttpAtom
 
     void operator=(const char *v) { _val = v; }
     void operator=(const nsHttpAtom &a) { _val = a._val; }
 
     // private
     const char *_val;
 };
 
-struct nsHttp
+namespace nsHttp
 {
-    static MOZ_MUST_USE nsresult CreateAtomTable();
-    static void DestroyAtomTable();
+    MOZ_MUST_USE nsresult CreateAtomTable();
+    void DestroyAtomTable();
 
     // The mutex is valid any time the Atom Table is valid
     // This mutex is used in the unusual case that the network thread and
     // main thread might access the same data
-    static Mutex *GetLock();
+    Mutex *GetLock();
 
     // will dynamically add atoms to the table if they don't already exist
-    static nsHttpAtom ResolveAtom(const char *);
-    static nsHttpAtom ResolveAtom(const nsACString &s)
+    nsHttpAtom ResolveAtom(const char *);
+    inline nsHttpAtom ResolveAtom(const nsACString &s)
     {
         return ResolveAtom(PromiseFlatCString(s).get());
     }
 
     // returns true if the specified token [start,end) is valid per RFC 2616
     // section 2.2
-    static bool IsValidToken(const char *start, const char *end);
+    bool IsValidToken(const char *start, const char *end);
 
-    static inline bool IsValidToken(const nsACString &s) {
+    inline bool IsValidToken(const nsACString &s) {
         return IsValidToken(s.BeginReading(), s.EndReading());
     }
 
     // Strip the leading or trailing HTTP whitespace per fetch spec section 2.2.
-    static void TrimHTTPWhitespace(const nsACString& aSource,
+    void TrimHTTPWhitespace(const nsACString& aSource,
                                    nsACString& aDest);
 
     // Returns true if the specified value is reasonable given the defintion
     // in RFC 2616 section 4.2.  Full strict validation is not performed
     // currently as it would require full parsing of the value.
-    static bool IsReasonableHeaderValue(const nsACString &s);
+    bool IsReasonableHeaderValue(const nsACString &s);
 
     // find the first instance (case-insensitive comparison) of the given
     // |token| in the |input| string.  the |token| is bounded by elements of
     // |separators| and may appear at the beginning or end of the |input|
     // string.  null is returned if the |token| is not found.  |input| may be
     // null, in which case null is returned.
-    static const char *FindToken(const char *input, const char *token,
+    const char *FindToken(const char *input, const char *token,
                                  const char *separators);
 
     // This function parses a string containing a decimal-valued, non-negative
     // 64-bit integer.  If the value would exceed INT64_MAX, then false is
     // returned.  Otherwise, this function returns true and stores the
     // parsed value in |result|.  The next unparsed character in |input| is
     // optionally returned via |next| if |next| is non-null.
     //
     // TODO(darin): Replace this with something generic.
     //
-    static MOZ_MUST_USE bool ParseInt64(const char *input, const char **next,
+    MOZ_MUST_USE bool ParseInt64(const char *input, const char **next,
                                         int64_t *result);
 
     // Variant on ParseInt64 that expects the input string to contain nothing
     // more than the value being parsed.
-    static inline MOZ_MUST_USE bool ParseInt64(const char *input,
+    inline MOZ_MUST_USE bool ParseInt64(const char *input,
                                                int64_t *result) {
         const char *next;
         return ParseInt64(input, &next, result) && *next == '\0';
     }
 
     // Return whether the HTTP status code represents a permanent redirect
-    static bool IsPermanentRedirect(uint32_t httpStatus);
+    bool IsPermanentRedirect(uint32_t httpStatus);
 
     // Returns the APLN token which represents the used protocol version.
-    static const char* GetProtocolVersion(uint32_t pv);
+    const char* GetProtocolVersion(uint32_t pv);
+
+    bool ValidationRequired(bool isForcedValid, nsHttpResponseHead *cachedResponseHead,
+                   uint32_t loadFlags, bool allowStaleCacheContent,
+                   bool isImmutable, bool customConditionalRequest,
+                   nsHttpRequestHead &requestHead,
+                   nsICacheEntry *entry, CacheControlParser &cacheControlRequest,
+                   bool fromPreviousSession);
+
+    nsresult GetHttpResponseHeadFromCacheEntry(nsICacheEntry *entry,
+                                               nsHttpResponseHead *cachedResponseHead);
+
+    nsresult CheckPartial(nsICacheEntry* aEntry, int64_t *aSize,
+                          int64_t *aContentLength,
+                          nsHttpResponseHead *responseHead);
+
+    void DetermineFramingAndImmutability(nsICacheEntry *entry, nsHttpResponseHead *cachedResponseHead,
+                                         bool isHttps, bool *weaklyFramed,
+                                         bool *isImmutable);
 
     // Declare all atoms
     //
     // The atom names and values are stored in nsHttpAtomList.h and are brought
     // to you by the magic of C preprocessing.  Add new atoms to nsHttpAtomList
     // and all support logic will be auto-generated.
     //
-#define HTTP_ATOM(_name, _value) static nsHttpAtom _name;
+#define HTTP_ATOM(_name, _value) extern nsHttpAtom _name;
 #include "nsHttpAtomList.h"
 #undef HTTP_ATOM
-};
+}
 
 //-----------------------------------------------------------------------------
 // utilities...
 //-----------------------------------------------------------------------------
 
 static inline uint32_t
 PRTimeToSeconds(PRTime t_usec)
 {
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -3930,37 +3930,19 @@ bypassCacheEntryOpen:
     waitFlags.Keep(WAIT_FOR_OFFLINE_CACHE_ENTRY);
 
     return NS_OK;
 }
 
 nsresult
 nsHttpChannel::CheckPartial(nsICacheEntry* aEntry, int64_t *aSize, int64_t *aContentLength)
 {
-    nsresult rv;
-
-    rv = aEntry->GetDataSize(aSize);
-
-    if (NS_ERROR_IN_PROGRESS == rv) {
-        *aSize = -1;
-        rv = NS_OK;
-    }
-
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    nsHttpResponseHead* responseHead = mCachedResponseHead
-        ? mCachedResponseHead
-        : mResponseHead;
-
-    if (!responseHead)
-        return NS_ERROR_UNEXPECTED;
-
-    *aContentLength = responseHead->ContentLength();
-
-    return NS_OK;
+    return nsHttp::CheckPartial(aEntry, aSize, aContentLength,
+                                mCachedResponseHead ? mCachedResponseHead
+                                                    : mResponseHead);
 }
 
 void
 nsHttpChannel::UntieValidationRequest()
 {
     DebugOnly<nsresult> rv;
     // Make the request unconditional again.
     rv = mRequestHead.ClearHeader(nsHttp::If_Modified_Since);
@@ -4032,42 +4014,19 @@ nsHttpChannel::OnCacheEntryCheck(nsICach
     // Determine if this is the first time that this cache entry
     // has been accessed during this session.
     bool fromPreviousSession =
             (gHttpHandler->SessionStartTime() > lastModifiedTime);
 
     // Get the cached HTTP response headers
     mCachedResponseHead = new nsHttpResponseHead();
 
-    // A "original-response-headers" metadata element holds network original headers,
-    // i.e. the headers in the form as they arrieved from the network.
-    // We need to get the network original headers first, because we need to keep them
-    // in order.
-    rv = entry->GetMetaDataElement("original-response-headers", getter_Copies(buf));
-    if (NS_SUCCEEDED(rv)) {
-        rv = mCachedResponseHead->ParseCachedOriginalHeaders((char *) buf.get());
-        if (NS_FAILED(rv)) {
-            LOG(("  failed to parse original-response-headers\n"));
-        }
-    }
-
-    buf.Adopt(0);
-    // A "response-head" metadata element holds response head, e.g. response status
-    // line and headers in the form Firefox uses them internally (no dupicate
-    // headers, etc.).
-    rv = entry->GetMetaDataElement("response-head", getter_Copies(buf));
+    rv = nsHttp::GetHttpResponseHeadFromCacheEntry(entry, mCachedResponseHead);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Parse string stored in a "response-head" metadata element.
-    // These response headers will be merged with the orignal headers (i.e. the
-    // headers stored in a "original-response-headers" metadata element).
-    rv = mCachedResponseHead->ParseCachedHead(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);
 
     if (mCachedResponseHead->NoStore() && mCacheEntryIsReadOnly) {
@@ -4163,136 +4122,30 @@ nsHttpChannel::OnCacheEntryCheck(nsICach
     NS_ENSURE_SUCCESS(rv,rv);
 
     bool doValidation = false;
     bool canAddImsHeader = true;
 
     bool isForcedValid = false;
     entry->GetIsForcedValid(&isForcedValid);
 
-    nsXPIDLCString framedBuf;
-    rv = entry->GetMetaDataElement("strongly-framed", getter_Copies(framedBuf));
-    // describe this in terms of explicitly weakly framed so as to be backwards
-    // compatible with old cache contents which dont have strongly-framed makers
-    bool weaklyFramed = NS_SUCCEEDED(rv) && framedBuf.EqualsLiteral("0");
-    bool isImmutable = !weaklyFramed && isHttps && mCachedResponseHead->Immutable();
+    bool weaklyFramed, isImmutable;
+    nsHttp::DetermineFramingAndImmutability(entry, mCachedResponseHead, isHttps,
+                                            &weaklyFramed, &isImmutable);
 
     // Cached entry is not the entity we request (see bug #633743)
     if (ResponseWouldVary(entry)) {
         LOG(("Validating based on Vary headers returning TRUE\n"));
         canAddImsHeader = false;
         doValidation = true;
-    }
-    // Check isForcedValid to see if it is possible to skip validation.
-    // Don't skip validation if we have serious reason to believe that this
-    // content is invalid (it's expired).
-    // See netwerk/cache2/nsICacheEntry.idl for details
-    else if (isForcedValid &&
-             (!mCachedResponseHead->ExpiresInPast() ||
-              !mCachedResponseHead->MustValidateIfExpired())) {
-        LOG(("NOT validating based on isForcedValid being true.\n"));
-        Telemetry::AutoCounter<Telemetry::PREDICTOR_TOTAL_PREFETCHES_USED> used;
-        ++used;
-        doValidation = false;
-    }
-    // If the LOAD_FROM_CACHE flag is set, any cached data can simply be used
-    else if (mLoadFlags & nsIRequest::LOAD_FROM_CACHE || mAllowStaleCacheContent) {
-        LOG(("NOT validating based on LOAD_FROM_CACHE load flag\n"));
-        doValidation = false;
-    }
-    // If the VALIDATE_ALWAYS flag is set, any cached data won't be used until
-    // it's revalidated with the server.
-    else if ((mLoadFlags & nsIRequest::VALIDATE_ALWAYS) && !isImmutable) {
-        LOG(("Validating based on VALIDATE_ALWAYS load flag\n"));
-        doValidation = true;
-    }
-    // Even if the VALIDATE_NEVER flag is set, there are still some cases in
-    // which we must validate the cached response with the server.
-    else if (mLoadFlags & nsIRequest::VALIDATE_NEVER) {
-        LOG(("VALIDATE_NEVER set\n"));
-        // if no-store validate cached response (see bug 112564)
-        if (mCachedResponseHead->NoStore()) {
-            LOG(("Validating based on no-store logic\n"));
-            doValidation = true;
-        }
-        else {
-            LOG(("NOT validating based on VALIDATE_NEVER load flag\n"));
-            doValidation = false;
-        }
-    }
-    // check if validation is strictly required...
-    else if (mCachedResponseHead->MustValidate()) {
-        LOG(("Validating based on MustValidate() returning TRUE\n"));
-        doValidation = true;
-    // possibly serve from cache for a custom If-Match/If-Unmodified-Since
-    // conditional request
-    } else if (mCustomConditionalRequest &&
-               !mRequestHead.HasHeader(nsHttp::If_Match) &&
-               !mRequestHead.HasHeader(nsHttp::If_Unmodified_Since)) {
-        LOG(("Validating based on a custom conditional request\n"));
-        doValidation = true;
     } else {
-        // previously we also checked for a query-url w/out expiration
-        // and didn't do heuristic on it. but defacto that is allowed now.
-        //
-        // Check if the cache entry has expired...
-
-        uint32_t now = NowInSeconds();
-
-        uint32_t age = 0;
-        rv = mCachedResponseHead->ComputeCurrentAge(now, now, &age);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        uint32_t freshness = 0;
-        rv = mCachedResponseHead->ComputeFreshnessLifetime(&freshness);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        uint32_t expiration = 0;
-        rv = entry->GetExpirationTime(&expiration);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        uint32_t maxAgeRequest, maxStaleRequest, minFreshRequest;
-
-        LOG(("  NowInSeconds()=%u, expiration time=%u, freshness lifetime=%u, age=%u",
-             now, expiration, freshness, age));
-
-        if (cacheControlRequest.NoCache()) {
-            LOG(("  validating, no-cache request"));
-            doValidation = true;
-        } else if (cacheControlRequest.MaxStale(&maxStaleRequest)) {
-            uint32_t staleTime = age > freshness ? age - freshness : 0;
-            doValidation = staleTime > maxStaleRequest;
-            LOG(("  validating=%d, max-stale=%u requested", doValidation, maxStaleRequest));
-        } else if (cacheControlRequest.MaxAge(&maxAgeRequest)) {
-            doValidation = age > maxAgeRequest;
-            LOG(("  validating=%d, max-age=%u requested", doValidation, maxAgeRequest));
-        } else if (cacheControlRequest.MinFresh(&minFreshRequest)) {
-            uint32_t freshTime = freshness > age ? freshness - age : 0;
-            doValidation = freshTime < minFreshRequest;
-            LOG(("  validating=%d, min-fresh=%u requested", doValidation, minFreshRequest));
-        } else if (now <= expiration) {
-            doValidation = false;
-            LOG(("  not validating, expire time not in the past"));
-        } else if (mCachedResponseHead->MustValidateIfExpired()) {
-            doValidation = true;
-        } else if (mLoadFlags & nsIRequest::VALIDATE_ONCE_PER_SESSION) {
-            // If the cached response does not include expiration infor-
-            // mation, then we must validate the response, despite whether
-            // or not this is the first access this session.  This behavior
-            // is consistent with existing browsers and is generally expected
-            // by web authors.
-            if (freshness == 0)
-                doValidation = true;
-            else
-                doValidation = fromPreviousSession;
-        }
-        else
-            doValidation = true;
-
-        LOG(("%salidating based on expiration time\n", doValidation ? "V" : "Not v"));
+        doValidation = nsHttp::ValidationRequired(
+            isForcedValid, mCachedResponseHead, mLoadFlags,
+            mAllowStaleCacheContent, isImmutable, mCustomConditionalRequest,
+            mRequestHead, entry, cacheControlRequest, fromPreviousSession);
     }
 
 
     // If a content signature is expected to be valid in this load,
     // set doValidation to force a signature check.
     if (!doValidation &&
         mLoadInfo && mLoadInfo->GetVerifySignedContent()) {
         doValidation = true;
--- a/netwerk/test/unit/test_http2.js
+++ b/netwerk/test/unit/test_http2.js
@@ -953,16 +953,112 @@ function test_http2_push_userContext3() 
 
 function test_http2_status_phrase() {
   var chan = makeChan("https://localhost:" + serverPort + "/statusphrase");
   var listener = new Http2CheckListener();
   listener.shouldSucceed = false;
   chan.asyncOpen2(listener);
 }
 
+var PulledDiskCacheListener = function() {};
+PulledDiskCacheListener.prototype = new Http2CheckListener();
+PulledDiskCacheListener.prototype.EXPECTED_DATA = "this was pulled via h2";
+PulledDiskCacheListener.prototype.readData = "";
+PulledDiskCacheListener.prototype.onDataAvailable = function testOnDataAvailable(request, ctx, stream, off, cnt) {
+  this.onDataAvailableFired = true;
+  this.isHttp2Connection = checkIsHttp2(request);
+  this.accum += cnt;
+  this.readData += read_stream(stream, cnt);
+};
+PulledDiskCacheListener.prototype.onStopRequest = function testOnStopRequest(request, ctx, status) {
+  do_check_eq(this.EXPECTED_DATA, this.readData);
+  Http2CheckListener.prorotype.onStopRequest.call(this, request, ctx, status);
+};
+
+const DISK_CACHE_DATA = "this is from disk cache";
+
+var FromDiskCacheListener = function() {};
+FromDiskCacheListener.prototype = {
+  onStartRequestFired: false,
+  onDataAvailableFired: false,
+  readData: "",
+
+  onStartRequest: function testOnStartRequest(request, ctx) {
+    this.onStartRequestFired = true;
+    if (!Components.isSuccessCode(request.status)) {
+      do_throw("Channel should have a success code! (" + request.status + ")");
+    }
+
+    do_check_true(request instanceof Components.interfaces.nsIHttpChannel);
+    do_check_true(request.requestSucceeded);
+    do_check_eq(request.responseStatus, 200);
+  },
+
+  onDataAvailable: function testOnDataAvailable(request, ctx, stream, off, cnt) {
+    this.onDataAvailableFired = true;
+    this.readData += read_stream(stream, cnt);
+  },
+
+  onStopRequest: function testOnStopRequest(request, ctx, status) {
+    do_check_true(this.onStartRequestFired);
+    do_check_true(Components.isSuccessCode(status));
+    do_check_true(this.onDataAvailableFired);
+    do_check_eq(this.readData, DISK_CACHE_DATA);
+
+    evict_cache_entries("disk");
+    syncWithCacheIOThread(() => {
+      // Now that we know the entry is out of the disk cache, check to make sure
+      // we don't have this hiding in the push cache somewhere - if we do, it
+      // didn't get cancelled, and we have a bug.
+      var chan = makeChan("https://localhost:" + serverPort + "/diskcache");
+      chan.listener = new PulledDiskCacheListener();
+      chan.loadGroup = loadGroup;
+      chan.asyncOpen2(listener);
+    });
+  }
+};
+
+var Http2DiskCachePushListener = function() {};
+
+Http2DiskCachePushListener.prototype = new Http2CheckListener();
+
+Http2DiskCachePushListener.onStopRequest = function(request, ctx, status) {
+    do_check_true(this.onStartRequestFired);
+    do_check_true(Components.isSuccessCode(status));
+    do_check_true(this.onDataAvailableFired);
+    do_check_true(this.isHttp2Connection == this.shouldBeHttp2);
+
+    // Now we need to open a channel to ensure we get data from the disk cache
+    // for the pushed item, instead of from the push cache.
+    var chan = makeChan("https://localhost:" + serverPort + "/diskcache");
+    chan.listener = new FromDiskCacheListener();
+    chan.loadGroup = loadGroup;
+    chan.asyncOpen2(listener);
+};
+
+function continue_test_http2_disk_cache_push(status, entry, appCache) {
+  // TODO - store stuff in cache entry, then open an h2 channel that will push
+  // this, once that completes, open a channel for the cache entry we made and
+  // ensure it came from disk cache, not the push cache.
+  var outputStream = entry.openOutputStream(0);
+  outputStream.write(DISK_CACHE_DATA, DISK_CACHE_DATA.length);
+
+  // Now we open our URL that will push data for the URL above
+  var chan = makeChan("https://localhost:" + serverPort + "/pushindisk");
+  var listener = new Http2DiskCachePushListener();
+  chan.loadGroup = loadGroup;
+  chan.asyncOpen2(listener);
+}
+
+function test_http2_disk_cache_push() {
+  asyncOpenCacheEntry("https://localhost:" + serverPort + "/diskcache",
+                      "disk", Ci.nsICacheStorage.OPEN_NORMALLY, null,
+                      continue_test_http2_disk_cache_push, false);
+}
+
 function test_complete() {
   resetPrefs();
   do_test_pending();
   httpserv.stop(do_test_finished);
   do_test_pending();
   httpserv2.stop(do_test_finished);
 
   do_test_finished();
@@ -1039,16 +1135,17 @@ var tests = [ test_http2_post_big
             , test_http2_continuations
             , test_http2_blocking_download
             , test_http2_illegalhpacksoft
             , test_http2_illegalhpackhard
             , test_http2_folded_header
             , test_http2_empty_data
             , test_http2_status_phrase
             , test_http2_doublepush
+            , test_http2_disk_cache_push
             // Add new tests above here - best to add new tests before h1
             // streams get too involved
             // These next two must always come in this order
             , test_http2_h11required_stream
             , test_http2_h11required_session
             , test_http2_retry_rst
             , test_http2_wrongsuite
             , test_http2_push_firstparty1
--- a/testing/xpcshell/moz-http2/moz-http2.js
+++ b/testing/xpcshell/moz-http2/moz-http2.js
@@ -826,16 +826,32 @@ function handleRequest(req, res) {
     });
     push2.end('pushed');
   }
 
   else if (u.pathname === "/doublypushed") {
     content = 'not pushed';
   }
 
+  else if (u.pathname === "/diskcache") {
+    content = "this was pulled via h2";
+  }
+
+  else if (u.pathname === "/pushindisk") {
+    var pushedContent = "this was pushed via h2";
+    push = res.push('/diskcache');
+    push.writeHead(200, {
+      'content-type': 'text/html',
+      'pushed' : 'yes',
+      'content-length' : pushedContent.length,
+      'X-Connection-Http2': 'yes'
+    });
+    push.end(pushedContent);
+  }
+
   res.setHeader('Content-Type', 'text/html');
   if (req.httpVersionMajor != 2) {
     res.setHeader('Connection', 'close');
   }
   res.writeHead(200);
   res.end(content);
 }