Bug 905468 (Part 1) - Make imgStatusTracker state diffing a cleaner process. r=jdm, a=bajaj
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -2949,19 +2949,20 @@ RasterImage::FinishedSomeDecoding(eShutd
// decoding routines have been finished.
rv = image->ShutdownDecoder(aIntent);
if (NS_FAILED(rv)) {
image->DoError();
}
}
}
- imgStatusTracker::StatusDiff diff;
+ ImageStatusDiff diff;
if (request) {
- diff = image->mStatusTracker->CalculateAndApplyDifference(request->mStatusTracker);
+ diff = image->mStatusTracker->Difference(request->mStatusTracker);
+ image->mStatusTracker->ApplyDifference(diff);
}
{
// Notifications can't go out with the decoding lock held.
MutexAutoUnlock unlock(mDecodingMutex);
// Then, tell the observers what happened in the decoder.
// If we have no request, we have not yet created a decoder, but we still
--- a/image/src/VectorImage.cpp
+++ b/image/src/VectorImage.cpp
@@ -384,20 +384,22 @@ VectorImage::OnImageDataComplete(nsIRequ
nsresult finalStatus = OnStopRequest(aRequest, aContext, aStatus);
// Give precedence to Necko failure codes.
if (NS_FAILED(aStatus))
finalStatus = aStatus;
// Actually fire OnStopRequest.
if (mStatusTracker) {
+ // XXX(seth): Is this seriously the least insane way to do this?
nsRefPtr<imgStatusTracker> clone = mStatusTracker->CloneForRecording();
imgDecoderObserver* observer = clone->GetDecoderObserver();
observer->OnStopRequest(aLastPart, finalStatus);
- imgStatusTracker::StatusDiff diff = mStatusTracker->CalculateAndApplyDifference(clone);
+ ImageStatusDiff diff = mStatusTracker->Difference(clone);
+ mStatusTracker->ApplyDifference(diff);
mStatusTracker->SyncNotifyDifference(diff);
}
return finalStatus;
}
nsresult
VectorImage::OnImageDataAvailable(nsIRequest* aRequest,
nsISupports* aContext,
@@ -848,17 +850,18 @@ 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) {
nsRefPtr<imgStatusTracker> clone = mStatusTracker->CloneForRecording();
imgDecoderObserver* observer = clone->GetDecoderObserver();
observer->OnStartDecode();
- imgStatusTracker::StatusDiff diff = mStatusTracker->CalculateAndApplyDifference(clone);
+ ImageStatusDiff diff = mStatusTracker->Difference(clone);
+ 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
@@ -936,17 +939,18 @@ VectorImage::OnSVGDocumentLoaded()
nsRefPtr<imgStatusTracker> clone = mStatusTracker->CloneForRecording();
imgDecoderObserver* observer = clone->GetDecoderObserver();
observer->OnStartContainer(); // Signal that width/height are available.
observer->FrameChanged(&nsIntRect::GetMaxSizedIntRect());
observer->OnStopFrame();
observer->OnStopDecode(NS_OK); // Unblock page load.
- imgStatusTracker::StatusDiff diff = mStatusTracker->CalculateAndApplyDifference(clone);
+ ImageStatusDiff diff = mStatusTracker->Difference(clone);
+ mStatusTracker->ApplyDifference(diff);
mStatusTracker->SyncNotifyDifference(diff);
}
EvaluateAnimation();
}
void
VectorImage::OnSVGDocumentError()
@@ -959,17 +963,18 @@ VectorImage::OnSVGDocumentError()
mError = true;
if (mStatusTracker) {
nsRefPtr<imgStatusTracker> clone = mStatusTracker->CloneForRecording();
imgDecoderObserver* observer = clone->GetDecoderObserver();
// Unblock page load.
observer->OnStopDecode(NS_ERROR_FAILURE);
- imgStatusTracker::StatusDiff diff = mStatusTracker->CalculateAndApplyDifference(clone);
+ ImageStatusDiff diff = mStatusTracker->Difference(clone);
+ mStatusTracker->ApplyDifference(diff);
mStatusTracker->SyncNotifyDifference(diff);
}
}
//------------------------------------------------------------------------------
// nsIStreamListener method
//******************************************************************************
--- a/image/src/imgStatusTracker.cpp
+++ b/image/src/imgStatusTracker.cpp
@@ -514,91 +514,105 @@ imgStatusTracker::SyncNotifyState(nsTObs
NOTIFY_IMAGE_OBSERVERS(OnStopDecode());
}
if (state & stateRequestStopped) {
NOTIFY_IMAGE_OBSERVERS(OnStopRequest(hadLastPart));
}
}
-imgStatusTracker::StatusDiff
-imgStatusTracker::CalculateAndApplyDifference(imgStatusTracker* other)
+ImageStatusDiff
+imgStatusTracker::Difference(imgStatusTracker* aOther) const
{
- LOG_SCOPE(GetImgLog(), "imgStatusTracker::SyncAndCalculateDifference");
-
- // We must not modify or notify for the start-load state, which happens from Necko callbacks.
- uint32_t loadState = mState & stateRequestStarted;
-
- StatusDiff diff;
- diff.mDiffState = ~mState & other->mState & ~stateRequestStarted;
- diff.mUnblockedOnload = mState & stateBlockingOnload && !(other->mState & stateBlockingOnload);
- diff.mFoundError = (mImageStatus != imgIRequest::STATUS_ERROR) && (other->mImageStatus == imgIRequest::STATUS_ERROR);
-
- // Now that we've calculated the difference in state, synchronize our state
- // with the other tracker.
+ ImageStatusDiff diff;
+ diff.diffState = ~mState & aOther->mState & ~stateRequestStarted;
+ diff.diffImageStatus = ~mImageStatus & aOther->mImageStatus;
+ diff.unblockedOnload = mState & stateBlockingOnload && !(aOther->mState & stateBlockingOnload);
+ diff.unsetDecodeStarted = mImageStatus & imgIRequest::STATUS_DECODE_STARTED
+ && !(aOther->mImageStatus & imgIRequest::STATUS_DECODE_STARTED);
+ diff.foundError = (mImageStatus != imgIRequest::STATUS_ERROR)
+ && (aOther->mImageStatus == imgIRequest::STATUS_ERROR);
- // First, actually synchronize our state.
- mState |= diff.mDiffState | loadState;
- if (diff.mUnblockedOnload) {
- mState &= ~stateBlockingOnload;
- }
+ MOZ_ASSERT(!mIsMultipart || aOther->mIsMultipart, "mIsMultipart should be monotonic");
+ diff.foundIsMultipart = !mIsMultipart && aOther->mIsMultipart;
+ MOZ_ASSERT(!mHadLastPart || aOther->mHadLastPart, "mHadLastPart should be monotonic");
+ diff.foundLastPart = !mHadLastPart && aOther->mHadLastPart;
- mIsMultipart = other->mIsMultipart;
- mHadLastPart = other->mHadLastPart;
- mImageStatus |= other->mImageStatus;
- mHasBeenDecoded = mHasBeenDecoded || other->mHasBeenDecoded;
-
- // The error state is sticky and overrides all other bits.
- if (mImageStatus & imgIRequest::STATUS_ERROR) {
- mImageStatus = imgIRequest::STATUS_ERROR;
- } else {
- // Unset the bits that can get unset as part of the decoding process.
- if (!(other->mImageStatus & imgIRequest::STATUS_DECODE_STARTED)) {
- mImageStatus &= ~imgIRequest::STATUS_DECODE_STARTED;
- }
- }
+ diff.gotDecoded = !mHasBeenDecoded && aOther->mHasBeenDecoded;
// Only record partial invalidations if we haven't been decoded before.
// When images are re-decoded after discarding, we don't want to display
// partially decoded versions to the user.
- bool doInvalidations = !mHasBeenDecoded
- || mImageStatus & imgIRequest::STATUS_ERROR
- || mImageStatus & imgIRequest::STATUS_DECODE_COMPLETE;
+ const uint32_t combinedStatus = mImageStatus | aOther->mImageStatus;
+ const bool doInvalidations = !(mHasBeenDecoded || aOther->mHasBeenDecoded)
+ || combinedStatus & imgIRequest::STATUS_ERROR
+ || combinedStatus & imgIRequest::STATUS_DECODE_COMPLETE;
- // Record the invalid rectangles and reset them for another go.
+ // Record and reset the invalid rectangle.
+ // XXX(seth): We shouldn't be resetting anything here; see bug 910441.
if (doInvalidations) {
- diff.mInvalidRect = mInvalidRect.Union(other->mInvalidRect);
- other->mInvalidRect.SetEmpty();
- mInvalidRect.SetEmpty();
+ diff.invalidRect = aOther->mInvalidRect;
+ aOther->mInvalidRect.SetEmpty();
}
return diff;
}
void
-imgStatusTracker::SyncNotifyDifference(imgStatusTracker::StatusDiff diff)
+imgStatusTracker::ApplyDifference(const ImageStatusDiff& aDiff)
+{
+ LOG_SCOPE(GetImgLog(), "imgStatusTracker::ApplyDifference");
+
+ // We must not modify or notify for the start-load state, which happens from Necko callbacks.
+ uint32_t loadState = mState & stateRequestStarted;
+
+ // Synchronize our state.
+ mState |= aDiff.diffState | loadState;
+ if (aDiff.unblockedOnload)
+ mState &= ~stateBlockingOnload;
+
+ mIsMultipart = mIsMultipart || aDiff.foundIsMultipart;
+ mHadLastPart = mHadLastPart || aDiff.foundLastPart;
+ mHasBeenDecoded = mHasBeenDecoded || aDiff.gotDecoded;
+
+ // Update the image status. There are some subtle points which are handled below.
+ mImageStatus |= aDiff.diffImageStatus;
+
+ // Unset bits which can get unset as part of the decoding process.
+ if (aDiff.unsetDecodeStarted)
+ mImageStatus &= ~imgIRequest::STATUS_DECODE_STARTED;
+
+ // The error state is sticky and overrides all other bits.
+ if (mImageStatus & imgIRequest::STATUS_ERROR)
+ mImageStatus = imgIRequest::STATUS_ERROR;
+}
+
+void
+imgStatusTracker::SyncNotifyDifference(const ImageStatusDiff& diff)
{
LOG_SCOPE(GetImgLog(), "imgStatusTracker::SyncNotifyDifference");
- SyncNotifyState(mConsumers, !!mImage, diff.mDiffState, diff.mInvalidRect, mHadLastPart);
+ nsIntRect invalidRect = mInvalidRect.Union(diff.invalidRect);
+ mInvalidRect.SetEmpty();
+ SyncNotifyState(mConsumers, !!mImage, diff.diffState, invalidRect, mHadLastPart);
- if (diff.mUnblockedOnload) {
+ if (diff.unblockedOnload) {
nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mConsumers);
while (iter.HasMore()) {
// Hold on to a reference to this proxy, since notifying the state can
// cause it to disappear.
nsRefPtr<imgRequestProxy> proxy = iter.GetNext();
if (!proxy->NotificationsDeferred()) {
SendUnblockOnload(proxy);
}
}
}
- if (diff.mFoundError) {
+ if (diff.foundError) {
FireFailureNotification();
}
}
imgStatusTracker*
imgStatusTracker::CloneForRecording()
{
imgStatusTracker* clone = new imgStatusTracker(*this);
--- a/image/src/imgStatusTracker.h
+++ b/image/src/imgStatusTracker.h
@@ -9,42 +9,97 @@
class imgIContainer;
class imgRequestProxy;
class imgStatusNotifyRunnable;
class imgRequestNotifyRunnable;
class imgStatusTrackerObserver;
class imgStatusTrackerNotifyingObserver;
struct nsIntRect;
-namespace mozilla {
-namespace image {
-class Image;
-} // namespace image
-} // namespace mozilla
-
#include "mozilla/RefPtr.h"
#include "nsCOMPtr.h"
#include "nsTObserverArray.h"
#include "nsIRunnable.h"
#include "nscore.h"
#include "imgDecoderObserver.h"
#include "nsISupportsImpl.h"
+namespace mozilla {
+namespace image {
+
+class Image;
+
+struct ImageStatusDiff
+{
+ ImageStatusDiff()
+ : invalidRect()
+ , diffState(0)
+ , diffImageStatus(0)
+ , unblockedOnload(false)
+ , unsetDecodeStarted(false)
+ , foundError(false)
+ , foundIsMultipart(false)
+ , foundLastPart(false)
+ , gotDecoded(false)
+ { }
+
+ static ImageStatusDiff NoChange() { return ImageStatusDiff(); }
+ bool IsNoChange() const { return *this == NoChange(); }
+
+ bool operator!=(const ImageStatusDiff& aOther) const { return !(*this == aOther); }
+ bool operator==(const ImageStatusDiff& aOther) const {
+ return aOther.invalidRect == invalidRect
+ && aOther.diffState == diffState
+ && aOther.diffImageStatus == diffImageStatus
+ && aOther.unblockedOnload == unblockedOnload
+ && aOther.unsetDecodeStarted == unsetDecodeStarted
+ && aOther.foundError == foundError
+ && aOther.foundIsMultipart == foundIsMultipart
+ && aOther.foundLastPart == foundLastPart
+ && aOther.gotDecoded == gotDecoded;
+ }
+
+ void Combine(const ImageStatusDiff& aOther) {
+ invalidRect = invalidRect.Union(aOther.invalidRect);
+ diffState |= aOther.diffState;
+ diffImageStatus |= aOther.diffImageStatus;
+ unblockedOnload = unblockedOnload || aOther.unblockedOnload;
+ unsetDecodeStarted = unsetDecodeStarted || aOther.unsetDecodeStarted;
+ foundError = foundError || aOther.foundError;
+ foundIsMultipart = foundIsMultipart || aOther.foundIsMultipart;
+ foundLastPart = foundLastPart || aOther.foundLastPart;
+ gotDecoded = gotDecoded || aOther.gotDecoded;
+ }
+
+ nsIntRect invalidRect;
+ uint32_t diffState;
+ uint32_t diffImageStatus;
+ bool unblockedOnload : 1;
+ bool unsetDecodeStarted : 1;
+ bool foundError : 1;
+ bool foundIsMultipart : 1;
+ bool foundLastPart : 1;
+ bool gotDecoded : 1;
+};
+
enum {
stateRequestStarted = 1u << 0,
stateHasSize = 1u << 1,
stateDecodeStarted = 1u << 2,
stateDecodeStopped = 1u << 3,
stateFrameStopped = 1u << 4,
stateRequestStopped = 1u << 5,
stateBlockingOnload = 1u << 6,
stateImageIsAnimated = 1u << 7
};
+} // namespace image
+} // namespace mozilla
+
/*
* The image status tracker is a class that encapsulates all the loading and
* decoding status about an Image, and makes it possible to send notifications
* to imgRequestProxys, both synchronously (i.e., the status now) and
* asynchronously (the status later).
*
* When a new proxy needs to be notified of the current state of an image, call
* the Notify() method on this class with the relevant proxy as its argument,
@@ -191,31 +246,25 @@ public:
// Weak pointer getters - no AddRefs.
inline mozilla::image::Image* GetImage() const { return mImage; }
inline imgDecoderObserver* GetDecoderObserver() { return mTrackerObserver.get(); }
imgStatusTracker* CloneForRecording();
- struct StatusDiff
- {
- uint32_t mDiffState;
- bool mUnblockedOnload;
- bool mFoundError;
- nsIntRect mInvalidRect;
- };
+ // Compute the difference between this status tracker and aOther.
+ mozilla::image::ImageStatusDiff Difference(imgStatusTracker* aOther) const;
- // Calculate the difference between this and other, apply that difference to
- // ourselves, and return it for passing to SyncNotifyDifference.
- StatusDiff CalculateAndApplyDifference(imgStatusTracker* other);
+ // Update our state to incorporate the changes in aDiff.
+ void ApplyDifference(const mozilla::image::ImageStatusDiff& aDiff);
- // Notify for the difference found in CalculateAndApplyDifference. No
- // decoding locks may be held.
- void SyncNotifyDifference(StatusDiff diff);
+ // Notify for the changes captured in an ImageStatusDiff. Because this may
+ // result in recursive notifications, no decoding locks may be held.
+ void SyncNotifyDifference(const mozilla::image::ImageStatusDiff& aDiff);
nsIntRect GetInvalidRect() const { return mInvalidRect; }
private:
friend class imgStatusNotifyRunnable;
friend class imgRequestNotifyRunnable;
friend class imgStatusTrackerObserver;
friend class imgStatusTrackerNotifyingObserver;