Bug 1097405 - Clean up imgStatusTracker's Difference APIs. r=tn
authorSeth Fowler <seth@mozilla.com>
Fri, 14 Nov 2014 20:10:47 -0800
changeset 215887 9a3e7bca8051f7ad8af18b9ac8461bf76ea36caa
parent 215886 02d3440cd34b569b21842a27350d45612297fe3d
child 215888 1d1810ba0ebf323cff97648732547786df12cc87
push id27829
push usergszorc@mozilla.com
push dateSat, 15 Nov 2014 22:34:49 +0000
treeherderautoland@19f75e1211e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1097405
milestone36.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 1097405 - Clean up imgStatusTracker's Difference APIs. r=tn
image/src/RasterImage.cpp
image/src/VectorImage.cpp
image/src/imgRequest.cpp
image/src/imgStatusTracker.cpp
image/src/imgStatusTracker.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -2992,41 +2992,40 @@ RasterImage::FinishedSomeDecoding(eShutd
     invalidRect = nsIntRect();
   }
   if (mHasBeenDecoded && !invalidRect.IsEmpty()) {
     // Don't send partial invalidations if we've been decoded before.
     invalidRect = mDecoded ? GetFirstFrameRect()
                            : nsIntRect();
   }
 
-  diff = image->mStatusTracker->Difference(diff);
-  image->mStatusTracker->ApplyDifference(diff);
-
   if (mNotifying) {
     // Accumulate the status changes. We don't permit recursive notifications
     // because they cause subtle concurrency bugs, so we'll delay sending out
     // the notifications until we pop back to the lowest invocation of
     // FinishedSomeDecoding on the stack.
     NS_WARNING("Recursively notifying in RasterImage::FinishedSomeDecoding!");
     mStatusDiff.Combine(diff);
     mInvalidRect.Union(invalidRect);
   } else {
     MOZ_ASSERT(mStatusDiff.IsNoChange(), "Shouldn't have an accumulated change at this point");
     MOZ_ASSERT(mInvalidRect.IsEmpty(), "Shouldn't have an accumulated invalidation rect here");
 
+    diff = image->mStatusTracker->Difference(diff);
+
     while (!diff.IsNoChange() || !invalidRect.IsEmpty()) {
       // Tell the observers what happened.
       mNotifying = true;
       image->mStatusTracker->SyncNotifyDifference(diff, invalidRect);
       mNotifying = false;
 
       // Gather any status changes that may have occurred as a result of sending
       // out the previous notifications. If there were any, we'll send out
       // notifications for them next.
-      diff = mStatusDiff;
+      diff = image->mStatusTracker->Difference(mStatusDiff);
       mStatusDiff = ImageStatusDiff::NoChange();
       invalidRect = mInvalidRect;
       mInvalidRect = nsIntRect();
     }
   }
 
   return RequestDecodeIfNeeded(rv, aIntent, done, wasSize);
 }
--- a/image/src/VectorImage.cpp
+++ b/image/src/VectorImage.cpp
@@ -439,17 +439,16 @@ VectorImage::OnImageDataComplete(nsIRequ
   // Give precedence to Necko failure codes.
   if (NS_FAILED(aStatus))
     finalStatus = aStatus;
 
   // Actually fire OnStopRequest.
   if (mStatusTracker) {
     ImageStatusDiff diff =
       ImageStatusDiff::ForOnStopRequest(aLastPart, mError, finalStatus);
-    mStatusTracker->ApplyDifference(diff);
     mStatusTracker->SyncNotifyDifference(diff);
   }
   return finalStatus;
 }
 
 nsresult
 VectorImage::OnImageDataAvailable(nsIRequest* aRequest,
                                   nsISupports* aContext,
@@ -564,17 +563,16 @@ VectorImage::SendInvalidationNotificatio
   // SVG document may not be 100% ready to render at that time. In those cases
   // we would miss the subsequent invalidations if we didn't send out the
   // notifications directly in |InvalidateObservers...|.
 
   if (mStatusTracker) {
     SurfaceCache::Discard(this);
     ImageStatusDiff diff;
     diff.diffState = FLAG_FRAME_STOPPED;
-    mStatusTracker->ApplyDifference(diff);
     mStatusTracker->SyncNotifyDifference(diff, nsIntRect::GetMaxSizedIntRect());
   }
 }
 
 NS_IMETHODIMP_(nsIntRect)
 VectorImage::GetImageSpaceInvalidationRect(const nsIntRect& aRect)
 {
   return aRect;
@@ -1030,17 +1028,16 @@ VectorImage::OnStartRequest(nsIRequest* 
   }
 
   // Sending StartDecode will block page load until the document's ready.  (We
   // unblock it by sending StopDecode in OnSVGDocumentLoaded or
   // OnSVGDocumentError.)
   if (mStatusTracker) {
     ImageStatusDiff diff;
     diff.diffState |= FLAG_DECODE_STARTED | FLAG_ONLOAD_BLOCKED;
-    mStatusTracker->ApplyDifference(diff);
     mStatusTracker->SyncNotifyDifference(diff);
   }
 
   // Create a listener to wait until the SVG document is fully loaded, which
   // will signal that this image is ready to render. Certain error conditions
   // will prevent us from ever getting this notification, so we also create a
   // listener that waits for parsing to complete and cancels the
   // SVGLoadEventListener if needed. The listeners are automatically attached
@@ -1113,17 +1110,16 @@ VectorImage::OnSVGDocumentLoaded()
   // Start listening to our image for rendering updates.
   mRenderingObserver = new SVGRootRenderingObserver(mSVGDocumentWrapper, this);
 
   // Tell *our* observers that we're done loading.
   if (mStatusTracker) {
     ImageStatusDiff diff;
     diff.diffState = FLAG_HAS_SIZE | FLAG_FRAME_STOPPED | FLAG_DECODE_STOPPED |
                      FLAG_ONLOAD_UNBLOCKED;
-    mStatusTracker->ApplyDifference(diff);
     mStatusTracker->SyncNotifyDifference(diff, nsIntRect::GetMaxSizedIntRect());
   }
 
   EvaluateAnimation();
 }
 
 void
 VectorImage::OnSVGDocumentError()
@@ -1135,17 +1131,16 @@ VectorImage::OnSVGDocumentError()
   // "broken image" icon.  See bug 594505.
   mError = true;
 
   if (mStatusTracker) {
     // Unblock page load.
     ImageStatusDiff diff;
     diff.diffState |= FLAG_DECODE_STOPPED | FLAG_ONLOAD_UNBLOCKED |
                       FLAG_HAS_ERROR;
-    mStatusTracker->ApplyDifference(diff);
     mStatusTracker->SyncNotifyDifference(diff);
   }
 }
 
 //------------------------------------------------------------------------------
 // nsIStreamListener method
 
 //******************************************************************************
--- a/image/src/imgRequest.cpp
+++ b/image/src/imgRequest.cpp
@@ -795,17 +795,16 @@ NS_IMETHODIMP imgRequest::OnStopRequest(
 
   if (!mImage) {
     // We have to fire imgStatusTracker::OnStopRequest ourselves because there's
     // no image capable of doing so.
     ImageStatusDiff diff =
       ImageStatusDiff::ForOnStopRequest(lastPart, /* aError = */ false, status);
 
     nsRefPtr<imgStatusTracker> statusTracker = GetStatusTracker();
-    statusTracker->ApplyDifference(diff);
     statusTracker->SyncNotifyDifference(diff);
   }
 
   mTimedChannel = nullptr;
   return NS_OK;
 }
 
 struct mimetype_closure
--- a/image/src/imgStatusTracker.cpp
+++ b/image/src/imgStatusTracker.cpp
@@ -291,42 +291,39 @@ imgStatusTracker::SyncNotifyState(ProxyA
   }
 }
 
 ImageStatusDiff
 imgStatusTracker::Difference(const ImageStatusDiff& aOther) const
 {
   ImageStatusDiff diff;
   diff.diffState = ~mState & aOther.diffState;
-
-  // Don't unblock onload if we're not blocked.
-  if (!((mState | diff.diffState) & FLAG_ONLOAD_BLOCKED)) {
-    diff.diffState &= ~FLAG_ONLOAD_UNBLOCKED;
-  }
-
   return diff;
 }
 
 void
-imgStatusTracker::ApplyDifference(const ImageStatusDiff& aDiff)
-{
-  LOG_SCOPE(GetImgLog(), "imgStatusTracker::ApplyDifference");
-  mState |= aDiff.diffState;
-}
-
-void
 imgStatusTracker::SyncNotifyDifference(const ImageStatusDiff& aDiff,
                                        const nsIntRect& aInvalidRect /* = nsIntRect() */)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Use mConsumers on main thread only");
   LOG_SCOPE(GetImgLog(), "imgStatusTracker::SyncNotifyDifference");
 
-  SyncNotifyState(mConsumers, !!mImage, aDiff.diffState, aInvalidRect);
+  // Don't unblock onload if we're not blocked.
+  ImageStatusDiff diff = Difference(aDiff);
+  if (!((mState | diff.diffState) & FLAG_ONLOAD_BLOCKED)) {
+    diff.diffState &= ~FLAG_ONLOAD_UNBLOCKED;
+  }
 
-  if (aDiff.diffState & FLAG_HAS_ERROR) {
+  // Apply the changes.
+  mState |= diff.diffState;
+
+  // Send notifications.
+  SyncNotifyState(mConsumers, !!mImage, diff.diffState, aInvalidRect);
+
+  if (diff.diffState & FLAG_HAS_ERROR) {
     FireFailureNotification();
   }
 }
 
 void
 imgStatusTracker::SyncNotify(imgRequestProxy* proxy)
 {
   MOZ_ASSERT(NS_IsMainThread(), "imgRequestProxy is not threadsafe");
--- a/image/src/imgStatusTracker.h
+++ b/image/src/imgStatusTracker.h
@@ -224,19 +224,16 @@ public:
     nsRefPtr<mozilla::image::Image> image = mImage;
     return image.forget();
   }
   inline bool HasImage() { return mImage; }
 
   // Compute the difference between this status tracker and aOther.
   mozilla::image::ImageStatusDiff Difference(const mozilla::image::ImageStatusDiff& aOther) const;
 
-  // Update our state to incorporate the changes in aDiff.
-  void ApplyDifference(const mozilla::image::ImageStatusDiff& aDiff);
-
   // Notify for the changes captured in an ImageStatusDiff. Because this may
   // result in recursive notifications, no decoding locks may be held.
   // Called on the main thread only.
   void SyncNotifyDifference(const mozilla::image::ImageStatusDiff& aDiff,
                             const nsIntRect& aInvalidRect = nsIntRect());
 
 private:
   typedef nsTObserverArray<mozilla::WeakPtr<imgRequestProxy>> ProxyArray;