Bug 1342567. r=aosmond a=abillings
authorTimothy Nikkel <tnikkel@gmail.com>
Fri, 21 Apr 2017 00:18:49 -0500
changeset 388390 f577512b1a299c7dd3325ac30538e021eeee6c90
parent 388389 289645ac65c1e2feed3b8942c6cf775a5548102a
child 388391 2414ce702247171efef83942a63007da73746864
push id51
push userfmarier@mozilla.com
push dateFri, 05 May 2017 18:41:45 +0000
reviewersaosmond, abillings
bugs1342567
milestone55.0a1
Bug 1342567. r=aosmond a=abillings
image/imgLoader.cpp
image/imgLoader.h
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -888,17 +888,18 @@ imgCacheEntry::imgCacheEntry(imgLoader* 
    mTouchedTime(SecondsFromPRTime(PR_Now())),
    mLoadTime(SecondsFromPRTime(PR_Now())),
    mExpiryTime(0),
    mMustValidate(false),
    // We start off as evicted so we don't try to update the cache. PutIntoCache
    // will set this to false.
    mEvicted(true),
    mHasNoProxies(true),
-   mForcePrincipalCheck(forcePrincipalCheck)
+   mForcePrincipalCheck(forcePrincipalCheck),
+   mInUse(false)
 { }
 
 imgCacheEntry::~imgCacheEntry()
 {
   LOG_FUNC(gImgLog, "imgCacheEntry::~imgCacheEntry()");
 }
 
 void
@@ -1909,16 +1910,20 @@ imgLoader::RemoveFromCache(const ImageCa
   LOG_STATIC_FUNC_WITH_PARAM(gImgLog,
                              "imgLoader::RemoveFromCache", "uri", aKey.Spec());
 
   imgCacheTable& cache = GetCache(aKey);
   imgCacheQueue& queue = GetCacheQueue(aKey);
 
   RefPtr<imgCacheEntry> entry;
   if (cache.Get(aKey, getter_AddRefs(entry)) && entry) {
+    if (MOZ_UNLIKELY(entry->GetInUse())) {
+      gfxCriticalNoteOnce << "RemoveFromCache(key) removing inuse cache entry";
+    }
+
     cache.Remove(aKey);
 
     MOZ_ASSERT(!entry->Evicted(), "Evicting an already-evicted cache entry!");
 
     // Entries with no proxies are in the tracker.
     if (entry->HasNoProxies()) {
       if (mCacheTracker) {
         mCacheTracker->RemoveObject(entry);
@@ -1949,16 +1954,20 @@ imgLoader::RemoveFromCache(imgCacheEntry
     const ImageCacheKey& key = request->CacheKey();
     imgCacheTable& cache = GetCache(key);
     imgCacheQueue& queue = GetCacheQueue(key);
 
     LOG_STATIC_FUNC_WITH_PARAM(gImgLog,
                                "imgLoader::RemoveFromCache", "entry's uri",
                                key.Spec());
 
+    if (MOZ_UNLIKELY(entry->GetInUse())) {
+      gfxCriticalNoteOnce << "RemoveFromCache(entry) removing inuse cache entry";
+    }
+
     cache.Remove(key);
 
     if (entry->HasNoProxies()) {
       LOG_STATIC_FUNC(gImgLog,
                       "imgLoader::RemoveFromCache removing from tracker");
       if (mCacheTracker) {
         mCacheTracker->RemoveObject(entry);
       }
@@ -2178,16 +2187,17 @@ imgLoader::LoadImage(nsIURI* aURI,
   if (aLoadingPrincipal) {
     attrs = aLoadingPrincipal->OriginAttributesRef();
   }
   ImageCacheKey key(aURI, attrs, aLoadingDocument, rv);
   NS_ENSURE_SUCCESS(rv, rv);
   imgCacheTable& cache = GetCache(key);
 
   if (cache.Get(key, getter_AddRefs(entry)) && entry) {
+    entry->SetInUse(true);
     if (ValidateEntry(entry, aURI, aInitialDocumentURI, aReferrerURI,
                       aReferrerPolicy, aLoadGroup, aObserver, aLoadingDocument,
                       requestFlags, aContentPolicyType, true, _retval,
                       aLoadingPrincipal, corsmode)) {
       request = entry->GetRequest();
 
       // If this entry has no proxies, its request has no reference to the
       // entry.
@@ -2215,17 +2225,21 @@ imgLoader::LoadImage(nsIURI* aURI,
           }
 
           mCacheTracker->MarkUsed(entry);
         }
       }
 
       entry->Touch();
 
+      entry->SetInUse(false);
+
     } else {
+      entry->SetInUse(false);
+
       // We can't use this entry. We'll try to load it off the network, and if
       // successful, overwrite the old entry in the cache with a new one.
       entry = nullptr;
     }
   }
 
   // Keep the channel in this scope, so we can adjust its notificationCallbacks
   // later when we create the proxy.
@@ -2421,16 +2435,18 @@ imgLoader::LoadImageWithChannel(nsIChann
     RemoveFromCache(key);
   } else {
     // Look in the cache for our URI, and then validate it.
     // XXX For now ignore aCacheKey. We will need it in the future
     // for correctly dealing with image load requests that are a result
     // of post data.
     imgCacheTable& cache = GetCache(key);
     if (cache.Get(key, getter_AddRefs(entry)) && entry) {
+      entry->SetInUse(true);
+
       // We don't want to kick off another network load. So we ask
       // ValidateEntry to only do validation without creating a new proxy. If
       // it says that the entry isn't valid any more, we'll only use the entry
       // we're getting if the channel is loading from the cache anyways.
       //
       // XXX -- should this be changed? it's pretty much verbatim from the old
       // code, but seems nonsensical.
       //
@@ -2455,16 +2471,17 @@ imgLoader::LoadImageWithChannel(nsIChann
 
         if (cacheChan) {
           cacheChan->IsFromCache(&bUseCacheCopy);
         } else {
           bUseCacheCopy = false;
         }
 
         if (!bUseCacheCopy) {
+          entry->SetInUse(false);
           entry = nullptr;
         } else {
           request = entry->GetRequest();
         }
       }
 
       if (request && entry) {
         // If this entry has no proxies, its request has no reference to
@@ -2477,16 +2494,19 @@ imgLoader::LoadImageWithChannel(nsIChann
             "Proxyless entry's request has cache entry!");
           request->SetCacheEntry(entry);
 
           if (mCacheTracker) {
             mCacheTracker->MarkUsed(entry);
           }
         }
       }
+      if (entry) {
+        entry->SetInUse(false);
+      }
     }
   }
 
   nsCOMPtr<nsILoadGroup> loadGroup;
   channel->GetLoadGroup(getter_AddRefs(loadGroup));
 
   // Filter out any load flags not from nsIRequest
   requestFlags &= nsIRequest::LOAD_REQUESTMASK;
--- a/image/imgLoader.h
+++ b/image/imgLoader.h
@@ -139,16 +139,24 @@ public:
     return mHasNoProxies;
   }
 
   bool ForcePrincipalCheck() const
   {
     return mForcePrincipalCheck;
   }
 
+  bool GetInUse() const {
+    return mInUse;
+  }
+
+  void SetInUse(bool aInUse) {
+    mInUse = aInUse;
+  }
+
   imgLoader* Loader() const
   {
     return mLoader;
   }
 
 private: // methods
   friend class imgLoader;
   friend class imgCacheQueue;
@@ -173,16 +181,17 @@ private: // data
   int32_t mTouchedTime;
   uint32_t mLoadTime;
   int32_t mExpiryTime;
   nsExpirationState mExpirationState;
   bool mMustValidate : 1;
   bool mEvicted : 1;
   bool mHasNoProxies : 1;
   bool mForcePrincipalCheck : 1;
+  bool mInUse : 1;
 };
 
 #include <vector>
 
 #define NS_IMGLOADER_CID \
 { /* c1354898-e3fe-4602-88a7-c4520c21cb4e */         \
      0xc1354898,                                     \
      0xe3fe,                                         \