Bug 1342567. r=aosmond a=abillings
authorTimothy Nikkel <tnikkel@gmail.com>
Fri, 21 Apr 2017 00:18:49 -0500
changeset 354353 f577512b1a299c7dd3325ac30538e021eeee6c90
parent 354352 289645ac65c1e2feed3b8942c6cf775a5548102a
child 354354 2414ce702247171efef83942a63007da73746864
push id31695
push userkwierso@gmail.com
push dateSat, 22 Apr 2017 00:30:52 +0000
treeherdermozilla-central@9550eedc0bd8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond, abillings
bugs1342567
milestone55.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 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,                                         \