Bug 784591. Part 1.5. Remove the SHOULD_BE_TRACKED bit and just always track non-null requests. r=joe,khuey
authorTimothy Nikkel <tnikkel@gmail.com>
Wed, 13 Feb 2013 14:18:08 -0600
changeset 131665 eb62e8cc4ff1193ff6f69cd7603221b3c1c9d3b0
parent 131664 ee1c037c0e1c0365a431cbac5f252d0b6e2d32b8
child 131666 7559ef8dc1bbebfc411f8c311eaaa867a82256f9
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe, khuey
bugs784591
milestone21.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 784591. Part 1.5. Remove the SHOULD_BE_TRACKED bit and just always track non-null requests. r=joe,khuey Also assert that the request is null when we do not track it.
content/base/src/nsImageLoadingContent.cpp
content/base/src/nsImageLoadingContent.h
--- a/content/base/src/nsImageLoadingContent.cpp
+++ b/content/base/src/nsImageLoadingContent.cpp
@@ -493,16 +493,17 @@ nsImageLoadingContent::LoadImageWithChan
   nsresult rv = nsContentUtils::GetImgLoaderForChannel(aChannel)->
     LoadImageWithChannel(aChannel, this, doc,
                          getter_AddRefs(listener),
                          getter_AddRefs(req));
   if (NS_SUCCEEDED(rv)) {
     TrackImage(req);
     ResetAnimationIfNeeded();
   } else {
+    MOZ_ASSERT(!req, "Shouldn't have non-null request here");
     // If we don't have a current URI, we might as well store this URI so people
     // know what we tried (and failed) to load.
     if (!mCurrentRequest)
       aChannel->GetURI(getter_AddRefs(mCurrentURI));
     FireEvent(NS_LITERAL_STRING("error"));
     aError.Throw(rv);
   }
   return listener.forget();
@@ -730,16 +731,17 @@ nsImageLoadingContent::LoadImage(nsIURI*
 
         nsImageFrame *f = do_QueryFrame(GetOurPrimaryFrame());
         if (f) {
           f->NotifyNewCurrentRequest(mCurrentRequest, NS_OK);
         }
       }
     }
   } else {
+    MOZ_ASSERT(!req, "Shouldn't have non-null request here");
     // If we don't have a current URI, we might as well store this URI so people
     // know what we tried (and failed) to load.
     if (!mCurrentRequest)
       mCurrentURI = aNewURI;
     FireEvent(NS_LITERAL_STRING("error"));
     return NS_OK;
   }
 
@@ -842,20 +844,22 @@ nsImageLoadingContent::UseAsPrimaryReque
 
   // Get rid if our existing images
   ClearPendingRequest(NS_BINDING_ABORTED);
   ClearCurrentRequest(NS_BINDING_ABORTED);
 
   // Clone the request we were given.
   nsRefPtr<imgRequestProxy>& req = PrepareNextRequest();
   nsresult rv = aRequest->Clone(this, getter_AddRefs(req));
-  if (NS_SUCCEEDED(rv))
+  if (NS_SUCCEEDED(rv)) {
     TrackImage(req);
-  else
+  } else {
+    MOZ_ASSERT(!req, "Shouldn't have non-null request here");
     return rv;
+  }
 
   return NS_OK;
 }
 
 nsIDocument*
 nsImageLoadingContent::GetOurOwnerDoc()
 {
   nsCOMPtr<nsIContent> thisContent =
@@ -1144,19 +1148,19 @@ nsImageLoadingContent::BindToTree(nsIDoc
   if (!aDocument)
     return;
 
   // Push a null JSContext on the stack so that callbacks triggered by the
   // below code won't think they're being called from JS.
   nsCxPusher pusher;
   pusher.PushNull();
 
-  if (mCurrentRequestFlags & REQUEST_SHOULD_BE_TRACKED)
+  if (mCurrentRequest)
     aDocument->AddImage(mCurrentRequest);
-  if (mPendingRequestFlags & REQUEST_SHOULD_BE_TRACKED)
+  if (mPendingRequest)
     aDocument->AddImage(mPendingRequest);
 
   if (mCurrentRequestFlags & REQUEST_BLOCKS_ONLOAD)
     aDocument->BlockOnload();
 }
 
 void
 nsImageLoadingContent::UnbindFromTree(bool aDeep, bool aNullParent)
@@ -1166,58 +1170,48 @@ nsImageLoadingContent::UnbindFromTree(bo
   if (!doc)
     return;
 
   // Push a null JSContext on the stack so that callbacks triggered by the
   // below code won't think they're being called from JS.
   nsCxPusher pusher;
   pusher.PushNull();
 
-  if (mCurrentRequestFlags & REQUEST_SHOULD_BE_TRACKED)
+  if (mCurrentRequest)
     doc->RemoveImage(mCurrentRequest);
-  if (mPendingRequestFlags & REQUEST_SHOULD_BE_TRACKED)
+  if (mPendingRequest)
     doc->RemoveImage(mPendingRequest);
 
   if (mCurrentRequestFlags & REQUEST_BLOCKS_ONLOAD)
     doc->UnblockOnload(false);
 }
 
 nsresult
 nsImageLoadingContent::TrackImage(imgIRequest* aImage)
 {
   if (!aImage)
     return NS_OK;
 
   MOZ_ASSERT(aImage == mCurrentRequest || aImage == mPendingRequest,
              "Why haven't we heard of this request?");
-  if (aImage == mCurrentRequest) {
-    mCurrentRequestFlags |= REQUEST_SHOULD_BE_TRACKED;
-  } else {
-    mPendingRequestFlags |= REQUEST_SHOULD_BE_TRACKED;
-  }
 
   nsIDocument* doc = GetOurCurrentDoc();
   if (doc)
     return doc->AddImage(aImage);
   return NS_OK;
 }
 
 nsresult
 nsImageLoadingContent::UntrackImage(imgIRequest* aImage)
 {
   if (!aImage)
     return NS_OK;
 
   MOZ_ASSERT(aImage == mCurrentRequest || aImage == mPendingRequest,
              "Why haven't we heard of this request?");
-  if (aImage == mCurrentRequest) {
-    mCurrentRequestFlags &= ~REQUEST_SHOULD_BE_TRACKED;
-  } else {
-    mPendingRequestFlags &= ~REQUEST_SHOULD_BE_TRACKED;
-  }
 
   // If GetOurDocument() returns null here, we've outlived our document.
   // That's fine, because the document empties out the tracker and unlocks
   // all locked images on destruction.
   nsIDocument* doc = GetOurCurrentDoc();
   if (doc)
     return doc->RemoveImage(aImage, nsIDocument::REQUEST_DISCARD);
   return NS_OK;
--- a/content/base/src/nsImageLoadingContent.h
+++ b/content/base/src/nsImageLoadingContent.h
@@ -334,21 +334,18 @@ protected:
   nsRefPtr<imgRequestProxy> mCurrentRequest;
   nsRefPtr<imgRequestProxy> mPendingRequest;
   uint32_t mCurrentRequestFlags;
   uint32_t mPendingRequestFlags;
 
   enum {
     // Set if the request needs 
     REQUEST_NEEDS_ANIMATION_RESET = 0x00000001U,
-    // Set if the request should be tracked.  This is true if the request is
-    // not tracked iff this node is not in the document.
-    REQUEST_SHOULD_BE_TRACKED = 0x00000002U,
     // Set if the request is blocking onload.
-    REQUEST_BLOCKS_ONLOAD = 0x00000004U
+    REQUEST_BLOCKS_ONLOAD = 0x00000002U
   };
 
   // If the image was blocked or if there was an error loading, it's nice to
   // still keep track of what the URI was despite not having an imgIRequest.
   // We only maintain this in those situations (in the common case, this is
   // always null).
   nsCOMPtr<nsIURI>      mCurrentURI;