Bug 1419889 - Don't force the image cache to validate if it hasn't started yet. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Thu, 07 Dec 2017 08:28:28 -0500
changeset 395483 1a1ebee6fb8ca96748db96d5b5651460c20c0f38
parent 395482 412a612a30d09fdf0d7c04a23de3c0c78337a81b
child 395484 8972884c1ac7e699f081c012cde67d45eb323975
push id33045
push usershindli@mozilla.com
push dateThu, 07 Dec 2017 22:12:36 +0000
treeherdermozilla-central@91cecf141b8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1419889
milestone59.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 1419889 - Don't force the image cache to validate if it hasn't started yet. r=tnikkel imgLoader::ValidateEntry would aggressively determine an entry has expired, even when the request hasn't yet begun. This is because the expiration time for the entry was not set unless it was for a channel which supports caching. Now we set the expiration time for all channels, and if it doesn't support caching, it just expires at the current time when imgRequest::OnStartRequest is called. Additionally, imgLoader::ValidateEntry will not consider the expiration time in the entry until it is non-zero.
image/imgLoader.cpp
image/imgLoader.h
image/imgRequest.cpp
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -925,18 +925,18 @@ NewImageChannel(nsIChannel** aResult,
   if (childLoadGroup) {
     childLoadGroup->SetParentLoadGroup(aLoadGroup);
   }
   (*aResult)->SetLoadGroup(loadGroup);
 
   return NS_OK;
 }
 
-static uint32_t
-SecondsFromPRTime(PRTime prTime)
+/* static */ uint32_t
+imgCacheEntry::SecondsFromPRTime(PRTime prTime)
 {
   return uint32_t(int64_t(prTime) / int64_t(PR_USEC_PER_SEC));
 }
 
 imgCacheEntry::imgCacheEntry(imgLoader* loader, imgRequest* request,
                              bool forcePrincipalCheck)
  : mLoader(loader),
    mRequest(request),
@@ -1876,40 +1876,39 @@ imgLoader::ValidateEntry(imgCacheEntry* 
                          nsContentPolicyType aLoadPolicyType,
                          bool aCanMakeNewChannel,
                          imgRequestProxy** aProxyRequest,
                          nsIPrincipal* aTriggeringPrincipal,
                          int32_t aCORSMode)
 {
   LOG_SCOPE(gImgLog, "imgLoader::ValidateEntry");
 
-  bool hasExpired;
-  uint32_t expirationTime = aEntry->GetExpiryTime();
-  if (expirationTime <= SecondsFromPRTime(PR_Now())) {
-    hasExpired = true;
-  } else {
-    hasExpired = false;
-  }
+  // If the expiration time is zero, then the request has not gotten far enough
+  // to know when it will expire.
+  uint32_t expiryTime = aEntry->GetExpiryTime();
+  bool hasExpired = expiryTime != 0 &&
+                    expiryTime <= imgCacheEntry::SecondsFromPRTime(PR_Now());
 
   nsresult rv;
 
   // Special treatment for file URLs - aEntry has expired if file has changed
   nsCOMPtr<nsIFileURL> fileUrl(do_QueryInterface(aURI));
   if (fileUrl) {
     uint32_t lastModTime = aEntry->GetLoadTime();
 
     nsCOMPtr<nsIFile> theFile;
     rv = fileUrl->GetFile(getter_AddRefs(theFile));
     if (NS_SUCCEEDED(rv)) {
       PRTime fileLastMod;
       rv = theFile->GetLastModifiedTime(&fileLastMod);
       if (NS_SUCCEEDED(rv)) {
         // nsIFile uses millisec, NSPR usec
         fileLastMod *= 1000;
-        hasExpired = SecondsFromPRTime((PRTime)fileLastMod) > lastModTime;
+        hasExpired =
+          imgCacheEntry::SecondsFromPRTime((PRTime)fileLastMod) > lastModTime;
       }
     }
   }
 
   RefPtr<imgRequest> request(aEntry->GetRequest());
 
   if (!request) {
     return false;
--- a/image/imgLoader.h
+++ b/image/imgLoader.h
@@ -36,16 +36,18 @@ namespace mozilla {
 namespace image {
 class ImageURL;
 } // namespace image
 } // namespace mozilla
 
 class imgCacheEntry
 {
 public:
+  static uint32_t SecondsFromPRTime(PRTime prTime);
+
   imgCacheEntry(imgLoader* loader, imgRequest* request,
                 bool aForcePrincipalCheck);
   ~imgCacheEntry();
 
   nsrefcnt AddRef()
   {
     NS_PRECONDITION(int32_t(mRefCnt) >= 0, "illegal refcnt");
     NS_ASSERT_OWNINGTHREAD(imgCacheEntry);
--- a/image/imgRequest.cpp
+++ b/image/imgRequest.cpp
@@ -28,16 +28,17 @@
 #include "nsMimeTypes.h"
 
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsISupportsPrimitives.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsContentUtils.h"
 
 #include "plstr.h" // PL_strcasestr(...)
+#include "prtime.h" // for PR_Now
 #include "nsNetUtil.h"
 #include "nsIProtocolHandler.h"
 #include "imgIRequest.h"
 
 #include "mozilla/IntegerPrintfMacros.h"
 
 using namespace mozilla;
 using namespace mozilla::image;
@@ -632,27 +633,31 @@ imgRequest::UpdateCacheEntrySize()
   mCacheEntry->SetDataSize(size);
 }
 
 void
 imgRequest::SetCacheValidation(imgCacheEntry* aCacheEntry, nsIRequest* aRequest)
 {
   /* get the expires info */
   if (aCacheEntry) {
-    nsCOMPtr<nsICacheInfoChannel> cacheChannel(do_QueryInterface(aRequest));
-    if (cacheChannel) {
+    // Expiration time defaults to 0. We set the expiration time on our
+    // entry if it hasn't been set yet.
+    if (aCacheEntry->GetExpiryTime() == 0) {
       uint32_t expiration = 0;
-      /* get the expiration time from the caching channel's token */
-      if (NS_SUCCEEDED(cacheChannel->GetCacheTokenExpirationTime(&expiration))) {
-        // Expiration time defaults to 0. We set the expiration time on our
-        // entry if it hasn't been set yet.
-        if (aCacheEntry->GetExpiryTime() == 0) {
-          aCacheEntry->SetExpiryTime(expiration);
-        }
+      nsCOMPtr<nsICacheInfoChannel> cacheChannel(do_QueryInterface(aRequest));
+      if (cacheChannel) {
+        /* get the expiration time from the caching channel's token */
+        cacheChannel->GetCacheTokenExpirationTime(&expiration);
       }
+      if (expiration == 0) {
+        // If the channel doesn't support caching, then ensure this expires the
+        // next time it is used.
+        expiration = imgCacheEntry::SecondsFromPRTime(PR_Now()) - 1;
+      }
+      aCacheEntry->SetExpiryTime(expiration);
     }
 
     // Determine whether the cache entry must be revalidated when we try to use
     // it. Currently, only HTTP specifies this information...
     nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(aRequest));
     if (httpChannel) {
       bool bMustRevalidate = false;