Bug 1409210 - Don't prefetch resources with a Vary header. r=valentin
authorNicholas Hurley <hurley@mozilla.com>
Mon, 16 Oct 2017 16:55:46 -0700
changeset 440163 326eb2eebb66addc46fc0e238f551aee9fbdb971
parent 440162 4bdb274daa63e61cac419b0f2f1483f710a6644f
child 440164 43af8c34f8ccc62df38c3dde550db38358a9ad15
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1409210
milestone58.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 1409210 - Don't prefetch resources with a Vary header. r=valentin Conceivably, we could allow a few more prefetches than this would (based on the headers in the original request matching up to a header listed in the Vary response header), but this is safer in case (for example) future requests of this resource end up sending a cookie that wasn't set on the original request. In practice, the difference is likely to be small enough that this broader stroke won't make a huge impact on the number of things we do or don't prefetch. MozReview-Commit-ID: GhD9mZR6aOX
netwerk/base/Predictor.cpp
netwerk/base/Predictor.h
--- a/netwerk/base/Predictor.cpp
+++ b/netwerk/base/Predictor.cpp
@@ -2604,28 +2604,30 @@ Predictor::UpdateCacheability(nsIURI *so
     PREDICTOR_LOG(("Predictor::UpdateCacheability non-http(s) uri"));
     return;
   }
 
   RefPtr<Predictor> self = sSelf;
   if (self) {
     nsAutoCString method;
     requestHead.Method(method);
+    nsAutoCString vary;
+    Unused << responseHead->GetHeader(nsHttp::Vary, vary);
     self->UpdateCacheabilityInternal(sourceURI, targetURI, httpStatus,
                                      method, *lci->OriginAttributesPtr(),
-                                     isTracking);
+                                     isTracking, !vary.IsEmpty());
   }
 }
 
 void
 Predictor::UpdateCacheabilityInternal(nsIURI *sourceURI, nsIURI *targetURI,
                                       uint32_t httpStatus,
                                       const nsCString &method,
                                       const OriginAttributes& originAttributes,
-                                      bool isTracking)
+                                      bool isTracking, bool couldVary)
 {
   PREDICTOR_LOG(("Predictor::UpdateCacheability httpStatus=%u", httpStatus));
 
   nsresult rv;
 
   if (!mInitialized) {
     PREDICTOR_LOG(("    not initialized"));
     return;
@@ -2653,17 +2655,17 @@ Predictor::UpdateCacheabilityInternal(ns
     return;
   }
 
   uint32_t openFlags = nsICacheStorage::OPEN_READONLY |
                        nsICacheStorage::OPEN_SECRETLY |
                        nsICacheStorage::CHECK_MULTITHREADED;
   RefPtr<Predictor::CacheabilityAction> action =
     new Predictor::CacheabilityAction(targetURI, httpStatus, method, isTracking,
-                                      this);
+                                      couldVary, this);
   nsAutoCString uri;
   targetURI->GetAsciiSpec(uri);
   PREDICTOR_LOG(("    uri=%s action=%p", uri.get(), action.get()));
   cacheDiskStorage->AsyncOpenURI(sourceURI, EmptyCString(), openFlags, action);
 }
 
 NS_IMPL_ISUPPORTS(Predictor::CacheabilityAction,
                   nsICacheEntryOpenCallback,
@@ -2730,17 +2732,18 @@ Predictor::CacheabilityAction::OnCacheEn
                                         flags)) {
       PREDICTOR_LOG(("    failed to parse key=%s value=%s", key, value));
       continue;
     }
 
     if (strTargetURI.Equals(uri)) {
       if (mHttpStatus == 200 && mMethod.EqualsLiteral("GET") &&
           !hasQueryString &&
-          !mIsTracking) {
+          !mIsTracking &&
+          !mCouldVary) {
         PREDICTOR_LOG(("    marking %s cacheable", key));
         flags |= FLAG_PREFETCHABLE;
       } else {
         PREDICTOR_LOG(("    marking %s uncacheable", key));
         flags &= ~FLAG_PREFETCHABLE;
       }
       nsCString newValue;
       MakeMetadataEntry(hitCount, lastHit, flags, newValue);
--- a/netwerk/base/Predictor.h
+++ b/netwerk/base/Predictor.h
@@ -134,32 +134,34 @@ private:
                            , public nsICacheEntryMetaDataVisitor
   {
   public:
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSICACHEENTRYOPENCALLBACK
     NS_DECL_NSICACHEENTRYMETADATAVISITOR
 
     CacheabilityAction(nsIURI *targetURI, uint32_t httpStatus,
-                       const nsCString &method, bool isTracking,
+                       const nsCString &method, bool isTracking, bool couldVary,
                        Predictor *predictor)
       :mTargetURI(targetURI)
       ,mHttpStatus(httpStatus)
       ,mMethod(method)
       ,mIsTracking(isTracking)
+      ,mCouldVary(couldVary)
       ,mPredictor(predictor)
     { }
 
   private:
     virtual ~CacheabilityAction() { }
 
     nsCOMPtr<nsIURI> mTargetURI;
     uint32_t mHttpStatus;
     nsCString mMethod;
     bool mIsTracking;
+    bool mCouldVary;
     RefPtr<Predictor> mPredictor;
     nsTArray<nsCString> mKeysToCheck;
     nsTArray<nsCString> mValuesToCheck;
   };
 
   class Resetter : public nsICacheEntryOpenCallback,
                    public nsICacheEntryMetaDataVisitor,
                    public nsICacheStorageVisitor
@@ -421,17 +423,17 @@ private:
                           uint32_t &flags);
 
   // Used to update whether a particular URI was cacheable or not.
   // sourceURI and targetURI are the same as the arguments to Learn
   // and httpStatus is the status code we got while loading targetURI.
   void UpdateCacheabilityInternal(nsIURI *sourceURI, nsIURI *targetURI,
                                   uint32_t httpStatus, const nsCString &method,
                                   const OriginAttributes& originAttributes,
-                                  bool isTracking);
+                                  bool isTracking, bool couldVary);
 
   // Make sure our prefs are in their expected range of values
   void SanitizePrefs();
 
   // Our state
   bool mInitialized;
 
   bool mEnabled;