Bug 1367551 - Cancel pushes when we already have the item in cache. r=mayhemer,mcmanus
authorNicholas Hurley <hurley@todesschaf.org>
Wed, 28 Jun 2017 10:34:55 -0700
changeset 420457 324cdf85739ce0d8bfdd0e5753c6e796cf61ffa3
parent 420456 2ef9bf18b3619aef31edd49be1946e3036b8be5e
child 420458 b822fd4c1977a6ee7a6f8f054aa3c15646c320f1
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer, mcmanus
bugs1367551
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1367551 - Cancel pushes when we already have the item in cache. r=mayhemer,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
@@ -3950,37 +3950,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);
@@ -4052,42 +4034,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) {
@@ -4183,136 +4142,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);
 }