Bug 1416774 - Ensure that imgRequestProxy::CancelAndForgetObserver removes itself from the cache validator. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 14 Nov 2017 12:02:59 -0500
changeset 391706 7e63329ef60853e9525d712cd03958908e6f33b3
parent 391705 207a13b699ac7b2a3eb58384dac109c3269552d5
child 391707 3f861ed63d1194889a059fe8d0650416d613187a
push id97316
push useraosmond@gmail.com
push dateTue, 14 Nov 2017 17:03:08 +0000
treeherdermozilla-inbound@7e63329ef608 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1416774
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 1416774 - Ensure that imgRequestProxy::CancelAndForgetObserver removes itself from the cache validator. r=tnikkel An imgRequestProxy may defer notifications when it needs to block on an imgCacheValidator. It may also be cancelled before the validator has completed its operation, but before this change, we did not remove the request from the set of proxies, imgCacheValidator::mProxies. When the deferral was completed, it would assert to ensure each proxy was still expecting a deferral before issuing the notifications. Cancelling a request can actually reset that state, which means we fail the assert. Failing the assert is actually harmless; in release we suffer no negative consequences as a result of this sequence of events. Now we just remove the proxy from the validator set to avoid asserting.
image/imgLoader.cpp
image/imgLoader.h
image/imgRequestProxy.cpp
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -2903,16 +2903,22 @@ imgCacheValidator::AddProxy(imgRequestPr
 {
   // aProxy needs to be in the loadgroup since we're validating from
   // the network.
   aProxy->AddToLoadGroup();
 
   mProxies.AppendObject(aProxy);
 }
 
+void
+imgCacheValidator::RemoveProxy(imgRequestProxy* aProxy)
+{
+  mProxies.RemoveObject(aProxy);
+}
+
 /** nsIRequestObserver methods **/
 
 NS_IMETHODIMP
 imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt)
 {
   // We may be holding on to a document, so ensure that it's released.
   nsCOMPtr<nsISupports> context = mContext.forget();
 
--- a/image/imgLoader.h
+++ b/image/imgLoader.h
@@ -550,16 +550,17 @@ class imgCacheValidator : public nsIStre
                           public nsIAsyncVerifyRedirectCallback
 {
 public:
   imgCacheValidator(nsProgressNotificationProxy* progress, imgLoader* loader,
                     imgRequest* aRequest, nsISupports* aContext,
                     bool forcePrincipalCheckForCacheEntry);
 
   void AddProxy(imgRequestProxy* aProxy);
+  void RemoveProxy(imgRequestProxy* aProxy);
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSIASYNCVERIFYREDIRECTCALLBACK
--- a/image/imgRequestProxy.cpp
+++ b/image/imgRequestProxy.cpp
@@ -512,18 +512,24 @@ imgRequestProxy::CancelAndForgetObserver
     return NS_ERROR_FAILURE;
   }
 
   LOG_SCOPE(gImgLog, "imgRequestProxy::CancelAndForgetObserver");
 
   mCanceled = true;
   mForceDispatchLoadGroup = true;
 
-  if (GetOwner()) {
-    GetOwner()->RemoveProxy(this, aStatus);
+  imgRequest* owner = GetOwner();
+  if (owner) {
+    imgCacheValidator* validator = owner->GetValidator();
+    if (validator) {
+      validator->RemoveProxy(this);
+    }
+
+    owner->RemoveProxy(this, aStatus);
   }
 
   RemoveFromLoadGroup();
   mForceDispatchLoadGroup = false;
 
   NullOutListener();
 
   return NS_OK;