Bug 1416774 - Ensure that imgRequestProxy::CancelAndForgetObserver removes itself from the cache validator. r=tnikkel a=gchang
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 14 Nov 2017 12:02:59 -0500
changeset 444824 af82d0c3342786f2575a3bca847bd7e91d44b1d0
parent 444823 63888c62a7937154e9cbf8ebecdecadb734aa8ae
child 444825 ed210b4ed61446769a587772b18d22a18d61a714
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)
reviewerstnikkel, gchang
bugs1416774
milestone58.0
Bug 1416774 - Ensure that imgRequestProxy::CancelAndForgetObserver removes itself from the cache validator. r=tnikkel a=gchang 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;