From 289c979c45373e6d26042add71a920e41c4c41b5 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Fri, 14 Nov 2014 20:06:19 -0800 Subject: [PATCH] Bug 1089046 (Part 1) - Remove imgDecoderObserver and related code. r=tn --- image/decoders/nsICODecoder.cpp | 5 +- image/src/Decoder.cpp | 39 ++-- image/src/Decoder.h | 11 +- image/src/RasterImage.cpp | 35 ++- image/src/RasterImage.h | 24 +-- image/src/VectorImage.cpp | 22 +- image/src/imgDecoderObserver.h | 118 ---------- image/src/imgRequest.cpp | 9 +- image/src/imgRequestProxy.cpp | 2 - image/src/imgRequestProxy.h | 1 - image/src/imgStatusTracker.cpp | 370 +++----------------------------- image/src/imgStatusTracker.h | 68 ++---- 12 files changed, 103 insertions(+), 601 deletions(-) delete mode 100644 image/src/imgDecoderObserver.h diff --git a/image/decoders/nsICODecoder.cpp b/image/decoders/nsICODecoder.cpp index 71539d5c2c5..4c4bb71bb1f 100644 --- a/image/decoders/nsICODecoder.cpp +++ b/image/decoders/nsICODecoder.cpp @@ -83,6 +83,7 @@ nsICODecoder::FinishInternal() if (mContainedDecoder) { mContainedDecoder->FinishSharedDecoder(); mDecodeDone = mContainedDecoder->GetDecodeDone(); + mDiff = mContainedDecoder->GetDiff(); } } @@ -339,7 +340,6 @@ nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount, PNGSIGNATURESIZE); if (mIsPNG) { mContainedDecoder = new nsPNGDecoder(mImage); - mContainedDecoder->SetObserver(mObserver); mContainedDecoder->SetSizeDecode(IsSizeDecode()); mContainedDecoder->InitSharedDecoder(mImageData, mImageDataLength, mColormap, mColormapSize, @@ -418,7 +418,6 @@ nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount, nsBMPDecoder* bmpDecoder = new nsBMPDecoder(mImage); mContainedDecoder = bmpDecoder; bmpDecoder->SetUseAlphaData(true); - mContainedDecoder->SetObserver(mObserver); mContainedDecoder->SetSizeDecode(IsSizeDecode()); mContainedDecoder->InitSharedDecoder(mImageData, mImageDataLength, mColormap, mColormapSize, @@ -588,6 +587,7 @@ nsICODecoder::WriteToContainedDecoder(const char* aBuffer, uint32_t aCount, DecodeStrategy aStrategy) { mContainedDecoder->Write(aBuffer, aCount, aStrategy); + mDiff = mContainedDecoder->GetDiff(); if (mContainedDecoder->HasDataError()) { mDataError = mContainedDecoder->HasDataError(); } @@ -632,6 +632,7 @@ nsICODecoder::AllocateFrame() if (mContainedDecoder) { nsresult rv = mContainedDecoder->AllocateFrame(); mCurrentFrame = mContainedDecoder->GetCurrentFrame(); + mDiff = mContainedDecoder->GetDiff(); return rv; } diff --git a/image/src/Decoder.cpp b/image/src/Decoder.cpp index 85d10d63ebe..7ae4a55fdf2 100644 --- a/image/src/Decoder.cpp +++ b/image/src/Decoder.cpp @@ -30,8 +30,7 @@ Decoder::Decoder(RasterImage &aImage) , mSizeDecode(false) , mInFrame(false) , mIsAnimated(false) -{ -} +{ } Decoder::~Decoder() { @@ -47,11 +46,11 @@ Decoder::Init() { // No re-initializing NS_ABORT_IF_FALSE(!mInitialized, "Can't re-initialize a decoder!"); - NS_ABORT_IF_FALSE(mObserver, "Need an observer!"); // Fire OnStartDecode at init time to support bug 512435. - if (!IsSizeDecode()) - mObserver->OnStartDecode(); + if (!IsSizeDecode()) { + mDiff.diffState |= FLAG_DECODE_STARTED | FLAG_ONLOAD_BLOCKED; + } // Implementation-specific initialization InitInternal(); @@ -68,7 +67,6 @@ Decoder::InitSharedDecoder(uint8_t* imageData, uint32_t imageDataLength, { // No re-initializing NS_ABORT_IF_FALSE(!mInitialized, "Can't re-initialize a decoder!"); - NS_ABORT_IF_FALSE(mObserver, "Need an observer!"); mImageData = imageData; mImageDataLength = imageDataLength; @@ -177,9 +175,8 @@ Decoder::Finish(RasterImage::eShutdownIntent aShutdownIntent) } PostDecodeDone(); } else { - if (mObserver) { - mObserver->OnStopDecode(NS_ERROR_FAILURE); - } + mDiff.diffState |= FLAG_DECODE_STOPPED | FLAG_ONLOAD_UNBLOCKED | + FLAG_HAS_ERROR; } } @@ -280,9 +277,8 @@ Decoder::PostSize(int32_t aWidth, // Tell the image mImageMetadata.SetSize(aWidth, aHeight, aOrientation); - // Notify the observer - if (mObserver) - mObserver->OnStartContainer(); + // Record this notification. + mDiff.diffState |= FLAG_HAS_SIZE; } void @@ -295,6 +291,12 @@ Decoder::PostFrameStart() mFrameCount++; mInFrame = true; + // If we just became animated, record that fact. + if (mFrameCount > 1) { + mIsAnimated = true; + mDiff.diffState |= FLAG_IS_ANIMATED; + } + // Decoder implementations should only call this method if they successfully // appended the frame to the image. So mFrameCount should always match that // reported by the Image. @@ -324,14 +326,7 @@ Decoder::PostFrameStop(FrameBlender::FrameAlpha aFrameAlpha /* = FrameBlender::k mCurrentFrame->SetBlendMethod(aBlendMethod); mCurrentFrame->ImageUpdated(mCurrentFrame->GetRect()); - // Fire notifications - if (mObserver) { - mObserver->OnStopFrame(); - if (mFrameCount > 1 && !mIsAnimated) { - mIsAnimated = true; - mObserver->OnImageIsAnimated(); - } - } + mDiff.diffState |= FLAG_FRAME_STOPPED | FLAG_ONLOAD_UNBLOCKED; } void @@ -357,9 +352,7 @@ Decoder::PostDecodeDone(int32_t aLoopCount /* = 0 */) mImageMetadata.SetLoopCount(aLoopCount); mImageMetadata.SetIsNonPremultiplied(GetDecodeFlags() & DECODER_NO_PREMULTIPLY_ALPHA); - if (mObserver) { - mObserver->OnStopDecode(NS_OK); - } + mDiff.diffState |= FLAG_DECODE_STOPPED; } void diff --git a/image/src/Decoder.h b/image/src/Decoder.h index f8b6ef76f38..4229f9c1447 100644 --- a/image/src/Decoder.h +++ b/image/src/Decoder.h @@ -7,7 +7,6 @@ #define MOZILLA_IMAGELIB_DECODER_H_ #include "RasterImage.h" -#include "imgDecoderObserver.h" #include "mozilla/RefPtr.h" #include "DecodeStrategy.h" #include "ImageMetadata.h" @@ -98,14 +97,10 @@ public: mSizeDecode = aSizeDecode; } - void SetObserver(imgDecoderObserver* aObserver) - { - MOZ_ASSERT(aObserver); - mObserver = aObserver; - } - size_t BytesDecoded() const { return mBytesDecoded; } + ImageStatusDiff GetDiff() const { return mDiff; } + // The number of frames we have, including anything in-progress. Thus, this // is only 0 if we haven't begun any frames. uint32_t GetFrameCount() { return mFrameCount; } @@ -231,8 +226,8 @@ protected: */ RasterImage &mImage; nsRefPtr mCurrentFrame; - RefPtr mObserver; ImageMetadata mImageMetadata; + ImageStatusDiff mDiff; uint8_t* mImageData; // Pointer to image data in either Cairo or 8bit format uint32_t mImageDataLength; diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index 0c0326bd775..9ba627f1b27 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -11,7 +11,6 @@ #include "base/histogram.h" #include "gfxPlatform.h" #include "nsComponentManagerUtils.h" -#include "imgDecoderObserver.h" #include "nsError.h" #include "Decoder.h" #include "nsAutoPtr.h" @@ -1043,10 +1042,6 @@ RasterImage::EnsureAnimExists() // since we didn't kill the source data in the old world either, locking // is acceptable for the moment. LockImage(); - - // Notify our observers that we are starting animation. - nsRefPtr statusTracker = CurrentStatusTracker(); - statusTracker->RecordImageIsAnimated(); } } @@ -1696,14 +1691,13 @@ RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*, nsresult aStatus, bo if (NS_FAILED(aStatus)) finalStatus = aStatus; + ImageStatusDiff diff = + ImageStatusDiff::ForOnStopRequest(aLastPart, mError, finalStatus); + // We just recorded OnStopRequest; we need to inform our listeners. { ReentrantMonitorAutoEnter lock(mDecodingMonitor); - - nsRefPtr statusTracker = CurrentStatusTracker(); - statusTracker->GetDecoderObserver()->OnStopRequest(aLastPart, finalStatus); - - FinishedSomeDecoding(); + FinishedSomeDecoding(eShutdownIntent_Done, nullptr, diff); } return finalStatus; @@ -1993,9 +1987,6 @@ RasterImage::InitDecoder(bool aDoSizeDecode) if (!mDecodeRequest) { mDecodeRequest = new DecodeRequest(this); } - MOZ_ASSERT(mDecodeRequest->mStatusTracker); - MOZ_ASSERT(mDecodeRequest->mStatusTracker->GetDecoderObserver()); - mDecoder->SetObserver(mDecodeRequest->mStatusTracker->GetDecoderObserver()); mDecoder->SetSizeDecode(aDoSizeDecode); mDecoder->SetDecodeFlags(mFrameDecodeFlags); if (!aDoSizeDecode) { @@ -2853,8 +2844,7 @@ RasterImage::DoError() return; } - // Calling FinishedSomeDecoding and CurrentStatusTracker requires us to be in - // the decoding monitor. + // Calling FinishedSomeDecoding requires us to be in the decoding monitor. ReentrantMonitorAutoEnter lock(mDecodingMonitor); // If we're mid-decode, shut down the decoder. @@ -2865,9 +2855,6 @@ RasterImage::DoError() // Put the container in an error state. mError = true; - nsRefPtr statusTracker = CurrentStatusTracker(); - statusTracker->GetDecoderObserver()->OnError(); - // Log our error LOG_CONTAINER_ERROR; } @@ -2975,7 +2962,8 @@ RasterImage::RequestDecodeIfNeeded(nsresult aStatus, nsresult RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_Done */, - DecodeRequest* aRequest /* = nullptr */) + DecodeRequest* aRequest /* = nullptr */, + const ImageStatusDiff& aDiff /* = ImageStatusDiff::NoChange() */) { MOZ_ASSERT(NS_IsMainThread()); @@ -2996,9 +2984,11 @@ RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_D bool wasSize = false; nsIntRect invalidRect; nsresult rv = NS_OK; + ImageStatusDiff diff = aDiff; if (image->mDecoder) { invalidRect = image->mDecoder->TakeInvalidRect(); + diff.Combine(image->mDecoder->GetDiff()); if (request && request->mChunkCount && !image->mDecoder->IsSizeDecode()) { Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, request->mChunkCount); @@ -3039,6 +3029,9 @@ RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_D if (NS_FAILED(rv)) { image->DoError(); } + + // If there were any final state changes, grab them. + diff.Combine(decoder->GetDiff()); } } @@ -3053,9 +3046,7 @@ RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_D : nsIntRect(); } - ImageStatusDiff diff = - request ? image->mStatusTracker->Difference(request->mStatusTracker) - : image->mStatusTracker->DecodeStateAsDifference(); + diff = image->mStatusTracker->Difference(diff); image->mStatusTracker->ApplyDifference(diff); if (mNotifying) { diff --git a/image/src/RasterImage.h b/image/src/RasterImage.h index f1b17050c81..e43be4d9bc8 100644 --- a/image/src/RasterImage.h +++ b/image/src/RasterImage.h @@ -309,16 +309,6 @@ public: // Decode strategy private: - already_AddRefed CurrentStatusTracker() - { - mDecodingMonitor.AssertCurrentThreadIn(); - nsRefPtr statusTracker; - statusTracker = mDecodeRequest ? mDecodeRequest->mStatusTracker - : mStatusTracker; - MOZ_ASSERT(statusTracker); - return statusTracker.forget(); - } - nsresult OnImageDataCompleteCore(nsIRequest* aRequest, nsISupports*, nsresult aStatus); /** @@ -333,19 +323,10 @@ private: , mRequestStatus(REQUEST_INACTIVE) , mChunkCount(0) , mAllocatedNewFrame(false) - { - MOZ_ASSERT(aImage, "aImage cannot be null"); - MOZ_ASSERT(aImage->mStatusTracker, - "aImage should have an imgStatusTracker"); - mStatusTracker = aImage->mStatusTracker->CloneForRecording(); - } + { } NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodeRequest) - // The status tracker that is associated with a given decode request, to - // ensure their lifetimes are linked. - nsRefPtr mStatusTracker; - RasterImage* mImage; size_t mBytesToDecode; @@ -531,7 +512,8 @@ private: }; nsresult FinishedSomeDecoding(eShutdownIntent intent = eShutdownIntent_Done, - DecodeRequest* request = nullptr); + DecodeRequest* request = nullptr, + const ImageStatusDiff& aDiff = ImageStatusDiff::NoChange()); void DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef, gfxContext* aContext, diff --git a/image/src/VectorImage.cpp b/image/src/VectorImage.cpp index 2275ac08700..7d94ff014ef 100644 --- a/image/src/VectorImage.cpp +++ b/image/src/VectorImage.cpp @@ -10,7 +10,6 @@ #include "gfxDrawable.h" #include "gfxPlatform.h" #include "gfxUtils.h" -#include "imgDecoderObserver.h" #include "imgFrame.h" #include "mozilla/AutoRestore.h" #include "mozilla/MemoryReporting.h" @@ -443,11 +442,8 @@ VectorImage::OnImageDataComplete(nsIRequest* aRequest, // Actually fire OnStopRequest. if (mStatusTracker) { - // XXX(seth): Is this seriously the least insane way to do this? - nsRefPtr clone = mStatusTracker->CloneForRecording(); - imgDecoderObserver* observer = clone->GetDecoderObserver(); - observer->OnStopRequest(aLastPart, finalStatus); - ImageStatusDiff diff = mStatusTracker->Difference(clone); + ImageStatusDiff diff = + ImageStatusDiff::ForOnStopRequest(aLastPart, mError, finalStatus); mStatusTracker->ApplyDifference(diff); mStatusTracker->SyncNotifyDifference(diff); } @@ -1037,10 +1033,8 @@ VectorImage::OnStartRequest(nsIRequest* aRequest, nsISupports* aCtxt) // unblock it by sending StopDecode in OnSVGDocumentLoaded or // OnSVGDocumentError.) if (mStatusTracker) { - nsRefPtr clone = mStatusTracker->CloneForRecording(); - imgDecoderObserver* observer = clone->GetDecoderObserver(); - observer->OnStartDecode(); - ImageStatusDiff diff = mStatusTracker->Difference(clone); + ImageStatusDiff diff; + diff.diffState |= FLAG_DECODE_STARTED | FLAG_ONLOAD_BLOCKED; mStatusTracker->ApplyDifference(diff); mStatusTracker->SyncNotifyDifference(diff); } @@ -1142,12 +1136,10 @@ VectorImage::OnSVGDocumentError() mError = true; if (mStatusTracker) { - nsRefPtr clone = mStatusTracker->CloneForRecording(); - imgDecoderObserver* observer = clone->GetDecoderObserver(); - // Unblock page load. - observer->OnStopDecode(NS_ERROR_FAILURE); - ImageStatusDiff diff = mStatusTracker->Difference(clone); + ImageStatusDiff diff; + diff.diffState |= FLAG_DECODE_STOPPED | FLAG_ONLOAD_UNBLOCKED | + FLAG_HAS_ERROR; mStatusTracker->ApplyDifference(diff); mStatusTracker->SyncNotifyDifference(diff); } diff --git a/image/src/imgDecoderObserver.h b/image/src/imgDecoderObserver.h deleted file mode 100644 index 91ebf06af1c..00000000000 --- a/image/src/imgDecoderObserver.h +++ /dev/null @@ -1,118 +0,0 @@ -/** -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef MOZILLA_IMAGELIB_IMGDECODEROBSERVER_H_ -#define MOZILLA_IMAGELIB_IMGDECODEROBSERVER_H_ - -#include "nsRect.h" -#include "mozilla/WeakPtr.h" - -struct nsIntRect; - -/** - * imgDecoderObserver interface - * - * This interface is used to observe Decoder objects. - * - * We make the distinction here between "load" and "decode" notifications. Load - * notifications are fired as the image is loaded from the network or - * filesystem. Decode notifications are fired as the image is decoded. If an - * image is decoded on load and not visibly discarded, decode notifications are - * nested logically inside load notifications as one might expect. However, with - * decode-on-draw, the set of decode notifications can come completely _after_ - * the load notifications, and can come multiple times if the image is - * discardable. Moreover, they can be interleaved in various ways. In general, - * any presumed ordering between load and decode notifications should not be - * relied upon. - * - * Decode notifications may or may not be synchronous, depending on the - * situation. If imgIDecoder::FLAG_SYNC_DECODE is passed to a function that - * triggers a decode, all notifications that can be generated from the currently - * loaded data fire before the call returns. If FLAG_SYNC_DECODE is not passed, - * all, some, or none of the notifications may fire before the call returns. - */ -class imgDecoderObserver -{ -public: - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(imgDecoderObserver); - - /** - * Load notification. - * - * called at the same time that nsIRequestObserver::onStartRequest would be - * (used only for observers of imgIRequest objects, which are nsIRequests, - * not imgIDecoder objects) - */ - virtual void OnStartRequest() = 0; - - /** - * Decode notification. - * - * Called as soon as the image begins getting decoded. This does not include - * "header-only" decodes used by decode-on-draw to parse the width/height - * out of the image. Thus, it is a decode notification only. - */ - virtual void OnStartDecode() = 0; - - /** - * Load notification. - * - * Called once enough data has been loaded from the network that we were able - * to parse the width/height from the image. By the time this callback is been - * called, the size has been set on the container and STATUS_SIZE_AVAILABLE - * has been set on the associated imgRequest. - */ - virtual void OnStartContainer() = 0; - - /** - * Decode notification. - * - * called when a frame is finished decoding. - */ - virtual void OnStopFrame() = 0; - - /** - * Notification for when an image is known to be animated. This should be - * fired at the earliest possible time. - */ - virtual void OnImageIsAnimated() = 0; - - /** - * Decode notification. - * - * Called when all decoding has terminated. - */ - virtual void OnStopDecode(nsresult status) = 0; - - /** - * Load notification. - * - * called at the same time that nsIRequestObserver::onStopRequest would be - * (used only for observers of imgIRequest objects, which are nsIRequests, - * not imgIDecoder objects) - */ - virtual void OnStopRequest(bool aIsLastPart, nsresult aStatus) = 0; - - /** - * Called when we are asked to Draw an image that is not locked. - */ - virtual void OnUnlockedDraw() = 0; - - /** - * Called when an image is realized to be in error state. - */ - virtual void OnError() = 0; - -protected: - virtual ~imgDecoderObserver() = 0; -}; - -// We must define a destructor because derived classes call our destructor from -// theirs. Pure virtual destructors only requires that child classes implement -// a virtual destructor, not that we can't have one too! -inline imgDecoderObserver::~imgDecoderObserver() -{} - -#endif // MOZILLA_IMAGELIB_IMGDECODEROBSERVER_H diff --git a/image/src/imgRequest.cpp b/image/src/imgRequest.cpp index 533c46c2385..a8e23ab4c3b 100644 --- a/image/src/imgRequest.cpp +++ b/image/src/imgRequest.cpp @@ -293,7 +293,7 @@ void imgRequest::Cancel(nsresult aStatus) statusTracker->MaybeUnblockOnload(); - statusTracker->RecordCancel(); + statusTracker->RecordError(); if (NS_IsMainThread()) { ContinueCancel(aStatus); @@ -732,6 +732,7 @@ NS_IMETHODIMP imgRequest::OnStartRequest(nsIRequest *aRequest, nsISupports *ctxt NS_IMETHODIMP imgRequest::OnStopRequest(nsIRequest *aRequest, nsISupports *ctxt, nsresult status) { LOG_FUNC(GetImgLog(), "imgRequest::OnStopRequest"); + MOZ_ASSERT(NS_IsMainThread(), "Can't send notifications off-main-thread"); // XXXldb What if this is a non-last part of a multipart request? // xxx before we release our reference to mRequest, lets @@ -795,8 +796,12 @@ NS_IMETHODIMP imgRequest::OnStopRequest(nsIRequest *aRequest, nsISupports *ctxt, 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 statusTracker = GetStatusTracker(); - statusTracker->OnStopRequest(lastPart, status); + statusTracker->ApplyDifference(diff); + statusTracker->SyncNotifyDifference(diff); } mTimedChannel = nullptr; diff --git a/image/src/imgRequestProxy.cpp b/image/src/imgRequestProxy.cpp index a941554f318..38362211ebe 100644 --- a/image/src/imgRequestProxy.cpp +++ b/image/src/imgRequestProxy.cpp @@ -721,8 +721,6 @@ NS_IMETHODIMP imgRequestProxy::GetHasTransferredData(bool* hasData) return NS_OK; } -/** imgDecoderObserver methods **/ - void imgRequestProxy::OnStartDecode() { // This notification is deliberately not propagated since there are no diff --git a/image/src/imgRequestProxy.h b/image/src/imgRequestProxy.h index 1cc347e2f06..db894dc8441 100644 --- a/image/src/imgRequestProxy.h +++ b/image/src/imgRequestProxy.h @@ -145,7 +145,6 @@ protected: // class) imgStatusTracker is the only class allowed to send us // notifications. - /* non-virtual imgDecoderObserver methods */ void OnStartDecode (); void OnStartContainer (); void OnFrameUpdate (const nsIntRect * aRect); diff --git a/image/src/imgStatusTracker.cpp b/image/src/imgStatusTracker.cpp index 8e17938e8f8..6dcf24336e9 100644 --- a/image/src/imgStatusTracker.cpp +++ b/image/src/imgStatusTracker.cpp @@ -9,7 +9,6 @@ #include "imgIContainer.h" #include "imgRequestProxy.h" -#include "imgDecoderObserver.h" #include "Image.h" #include "nsNetUtil.h" #include "nsIObserverService.h" @@ -20,136 +19,6 @@ using namespace mozilla::image; using mozilla::WeakPtr; -class imgStatusTrackerObserver : public imgDecoderObserver -{ -public: - explicit imgStatusTrackerObserver(imgStatusTracker* aTracker) - : mTracker(aTracker) - { - MOZ_ASSERT(aTracker); - } - - void SetTracker(imgStatusTracker* aTracker) - { - MOZ_ASSERT(aTracker); - mTracker = aTracker; - } - - /** imgDecoderObserver methods **/ - - virtual void OnStartDecode() MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnStartDecode"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - tracker->RecordStartDecode(); - if (!tracker->IsMultipart()) { - tracker->RecordBlockOnload(); - } - } - - virtual void OnStartRequest() MOZ_OVERRIDE - { - NS_NOTREACHED("imgStatusTrackerObserver(imgDecoderObserver)::OnStartRequest"); - } - - virtual void OnStartContainer() MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnStartContainer"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - nsRefPtr image = tracker->GetImage();; - tracker->RecordStartContainer(image); - } - - virtual void OnStopFrame() MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnStopFrame"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - tracker->RecordStopFrame(); - tracker->RecordUnblockOnload(); - } - - virtual void OnStopDecode(nsresult aStatus) MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnStopDecode"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - tracker->RecordStopDecode(aStatus); - - // This is really hacky. We need to handle the case where we start decoding, - // block onload, but then hit an error before we get to our first frame. - tracker->RecordUnblockOnload(); - } - - virtual void OnStopRequest(bool aLastPart, nsresult aStatus) MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnStopRequest"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - tracker->RecordStopRequest(aLastPart, aStatus); - } - - virtual void OnUnlockedDraw() MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnUnlockedDraw"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - NS_ABORT_IF_FALSE(tracker->HasImage(), - "OnUnlockedDraw callback before we've created our image"); - tracker->RecordUnlockedDraw(); - } - - virtual void OnImageIsAnimated() MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnImageIsAnimated"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - tracker->RecordImageIsAnimated(); - } - - virtual void OnError() MOZ_OVERRIDE - { - LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnError"); - nsRefPtr tracker = mTracker.get(); - if (!tracker) { return; } - tracker->RecordError(); - } - -protected: - virtual ~imgStatusTrackerObserver() {} - -private: - WeakPtr mTracker; -}; - -// imgStatusTracker methods - -imgStatusTracker::imgStatusTracker(Image* aImage) - : mImage(aImage) - , mState(0) -{ - mTrackerObserver = new imgStatusTrackerObserver(this); -} - -// Private, used only by CloneForRecording. -imgStatusTracker::imgStatusTracker(const imgStatusTracker& aOther) - : mImage(aOther.mImage) - , mState(aOther.mState) - // Note: we explicitly don't copy several fields: - // - mRequestRunnable, because it won't be nulled out when the - // mRequestRunnable's Run function eventually gets called. - // - mProperties, because we don't need it and it'd just point at the same - // object - // - mConsumers, because we don't need to talk to consumers -{ - mTrackerObserver = new imgStatusTrackerObserver(this); -} - -imgStatusTracker::~imgStatusTracker() -{} - imgStatusTrackerInit::imgStatusTrackerInit(mozilla::image::Image* aImage, imgStatusTracker* aTracker) { @@ -188,6 +57,11 @@ imgStatusTracker::ResetImage() void imgStatusTracker::SetIsMultipart() { mState |= FLAG_IS_MULTIPART; + + // If we haven't already blocked onload, make sure we never do. + if (!(mState & FLAG_ONLOAD_BLOCKED)) { + mState |= FLAG_ONLOAD_BLOCKED | FLAG_ONLOAD_UNBLOCKED; + } } bool @@ -418,20 +292,16 @@ imgStatusTracker::SyncNotifyState(ProxyArray& aProxies, } ImageStatusDiff -imgStatusTracker::Difference(imgStatusTracker* aOther) const +imgStatusTracker::Difference(const ImageStatusDiff& aOther) const { - MOZ_ASSERT(aOther, "aOther cannot be null"); ImageStatusDiff diff; - diff.diffState = ~mState & aOther->mState; - return 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; + } -ImageStatusDiff -imgStatusTracker::DecodeStateAsDifference() const -{ - ImageStatusDiff diff; - // XXX(seth): Is FLAG_REQUEST_STARTED really the only non-"decode state" flag? - diff.diffState = mState & ~FLAG_REQUEST_STARTED; return diff; } @@ -456,15 +326,6 @@ imgStatusTracker::SyncNotifyDifference(const ImageStatusDiff& aDiff, } } -already_AddRefed -imgStatusTracker::CloneForRecording() -{ - // Grab a ref to this to ensure it isn't deleted. - nsRefPtr thisStatusTracker = this; - nsRefPtr clone = new imgStatusTracker(*thisStatusTracker); - return clone.forget(); -} - void imgStatusTracker::SyncNotify(imgRequestProxy* proxy) { @@ -559,33 +420,11 @@ imgStatusTracker::FirstConsumerIs(imgRequestProxy* aConsumer) } void -imgStatusTracker::RecordCancel() +imgStatusTracker::RecordError() { mState |= FLAG_HAS_ERROR; } -void -imgStatusTracker::RecordLoaded() -{ - NS_ABORT_IF_FALSE(mImage, "RecordLoaded called before we have an Image"); - mState |= FLAG_REQUEST_STARTED | FLAG_HAS_SIZE | - FLAG_REQUEST_STOPPED | FLAG_MULTIPART_STOPPED; -} - -void -imgStatusTracker::RecordDecoded() -{ - NS_ABORT_IF_FALSE(mImage, "RecordDecoded called before we have an Image"); - mState |= FLAG_DECODE_STARTED | FLAG_DECODE_STOPPED | FLAG_FRAME_STOPPED; -} - -void -imgStatusTracker::RecordStartDecode() -{ - NS_ABORT_IF_FALSE(mImage, "RecordStartDecode without an Image"); - mState |= FLAG_DECODE_STARTED; -} - void imgStatusTracker::SendStartDecode(imgRequestProxy* aProxy) { @@ -594,16 +433,6 @@ imgStatusTracker::SendStartDecode(imgRequestProxy* aProxy) aProxy->OnStartDecode(); } -void -imgStatusTracker::RecordStartContainer(imgIContainer* aContainer) -{ - NS_ABORT_IF_FALSE(mImage, - "RecordStartContainer called before we have an Image"); - NS_ABORT_IF_FALSE(mImage == aContainer, - "RecordStartContainer called with wrong Image"); - mState |= FLAG_HAS_SIZE; -} - void imgStatusTracker::SendStartContainer(imgRequestProxy* aProxy) { @@ -612,13 +441,6 @@ imgStatusTracker::SendStartContainer(imgRequestProxy* aProxy) aProxy->OnStartContainer(); } -void -imgStatusTracker::RecordStopFrame() -{ - NS_ABORT_IF_FALSE(mImage, "RecordStopFrame called before we have an Image"); - mState |= FLAG_FRAME_STOPPED; -} - void imgStatusTracker::SendStopFrame(imgRequestProxy* aProxy) { @@ -627,18 +449,6 @@ imgStatusTracker::SendStopFrame(imgRequestProxy* aProxy) aProxy->OnStopFrame(); } -void -imgStatusTracker::RecordStopDecode(nsresult aStatus) -{ - MOZ_ASSERT(mImage, "RecordStopDecode called before we have an Image"); - - mState |= FLAG_DECODE_STOPPED; - - if (NS_FAILED(aStatus)) { - mState |= FLAG_HAS_ERROR; - } -} - void imgStatusTracker::SendStopDecode(imgRequestProxy* aProxy, nsresult aStatus) @@ -657,21 +467,6 @@ imgStatusTracker::SendDiscard(imgRequestProxy* aProxy) } -void -imgStatusTracker::RecordUnlockedDraw() -{ - NS_ABORT_IF_FALSE(mImage, - "RecordUnlockedDraw called before we have an Image"); -} - -void -imgStatusTracker::RecordImageIsAnimated() -{ - NS_ABORT_IF_FALSE(mImage, - "RecordImageIsAnimated called before we have an Image"); - mState |= FLAG_IS_ANIMATED; -} - void imgStatusTracker::SendImageIsAnimated(imgRequestProxy* aProxy) { @@ -692,7 +487,6 @@ void imgStatusTracker::OnUnlockedDraw() { MOZ_ASSERT(NS_IsMainThread()); - RecordUnlockedDraw(); ProxyArray::ForwardIterator iter(mConsumers); while (iter.HasMore()) { nsRefPtr proxy = iter.GetNext().get(); @@ -703,23 +497,6 @@ imgStatusTracker::OnUnlockedDraw() } /* non-virtual sort-of-nsIRequestObserver methods */ -void -imgStatusTracker::RecordStartRequest() -{ - // We're starting a new load, so clear any status and state bits indicating - // load/decode. - // XXX(seth): Are these really the only flags we want to clear? - mState &= ~FLAG_REQUEST_STARTED; - mState &= ~FLAG_DECODE_STARTED; - mState &= ~FLAG_DECODE_STOPPED; - mState &= ~FLAG_REQUEST_STOPPED; - mState &= ~FLAG_ONLOAD_BLOCKED; - mState &= ~FLAG_ONLOAD_UNBLOCKED; - mState &= ~FLAG_IS_ANIMATED; - - mState |= FLAG_REQUEST_STARTED; -} - void imgStatusTracker::SendStartRequest(imgRequestProxy* aProxy) { @@ -732,7 +509,20 @@ void imgStatusTracker::OnStartRequest() { MOZ_ASSERT(NS_IsMainThread()); - RecordStartRequest(); + + // We're starting a new load, so clear any status and state bits indicating + // load/decode. + // XXX(seth): Are these really the only flags we want to clear? + mState &= ~FLAG_REQUEST_STARTED; + mState &= ~FLAG_DECODE_STARTED; + mState &= ~FLAG_DECODE_STOPPED; + mState &= ~FLAG_REQUEST_STOPPED; + mState &= ~FLAG_ONLOAD_BLOCKED; + mState &= ~FLAG_ONLOAD_UNBLOCKED; + mState &= ~FLAG_IS_ANIMATED; + + mState |= FLAG_REQUEST_STARTED; + ProxyArray::ForwardIterator iter(mConsumers); while (iter.HasMore()) { nsRefPtr proxy = iter.GetNext().get(); @@ -742,19 +532,6 @@ imgStatusTracker::OnStartRequest() } } -void -imgStatusTracker::RecordStopRequest(bool aLastPart, - nsresult aStatus) -{ - mState |= FLAG_REQUEST_STOPPED; - if (aLastPart) { - mState |= FLAG_MULTIPART_STOPPED; - } - if (NS_FAILED(aStatus)) { - mState |= FLAG_HAS_ERROR; - } -} - void imgStatusTracker::SendStopRequest(imgRequestProxy* aProxy, bool aLastPart, @@ -766,59 +543,6 @@ imgStatusTracker::SendStopRequest(imgRequestProxy* aProxy, } } -class OnStopRequestEvent : public nsRunnable -{ -public: - OnStopRequestEvent(imgStatusTracker* aTracker, - bool aLastPart, - nsresult aStatus) - : mTracker(aTracker) - , mLastPart(aLastPart) - , mStatus(aStatus) - { - MOZ_ASSERT(!NS_IsMainThread(), "Should be created off the main thread"); - MOZ_ASSERT(aTracker, "aTracker should not be null"); - } - - NS_IMETHOD Run() - { - MOZ_ASSERT(NS_IsMainThread(), "Should be running on the main thread"); - MOZ_ASSERT(mTracker, "mTracker should not be null"); - mTracker->OnStopRequest(mLastPart, mStatus); - return NS_OK; - } -private: - nsRefPtr mTracker; - bool mLastPart; - nsresult mStatus; -}; - -void -imgStatusTracker::OnStopRequest(bool aLastPart, - nsresult aStatus) -{ - if (!NS_IsMainThread()) { - NS_DispatchToMainThread( - new OnStopRequestEvent(this, aLastPart, aStatus)); - return; - } - bool preexistingError = mState & FLAG_HAS_ERROR; - - RecordStopRequest(aLastPart, aStatus); - /* notify the kids */ - ProxyArray::ForwardIterator srIter(mConsumers); - while (srIter.HasMore()) { - nsRefPtr proxy = srIter.GetNext().get(); - if (proxy) { - SendStopRequest(proxy, aLastPart, aStatus); - } - } - - if (NS_FAILED(aStatus) && !preexistingError) { - FireFailureNotification(); - } -} - void imgStatusTracker::OnDiscard() { @@ -834,22 +558,6 @@ imgStatusTracker::OnDiscard() } } -void -imgStatusTracker::OnStopFrame() -{ - MOZ_ASSERT(NS_IsMainThread()); - RecordStopFrame(); - - /* notify the kids */ - ProxyArray::ForwardIterator iter(mConsumers); - while (iter.HasMore()) { - nsRefPtr proxy = iter.GetNext().get(); - if (proxy) { - SendStopFrame(proxy); - } - } -} - void imgStatusTracker::OnDataAvailable() { @@ -871,12 +579,6 @@ imgStatusTracker::OnDataAvailable() } } -void -imgStatusTracker::RecordBlockOnload() -{ - mState |= FLAG_ONLOAD_BLOCKED; -} - void imgStatusTracker::SendBlockOnload(imgRequestProxy* aProxy) { @@ -886,16 +588,6 @@ imgStatusTracker::SendBlockOnload(imgRequestProxy* aProxy) } } -void -imgStatusTracker::RecordUnblockOnload() -{ - // We sometimes unblock speculatively, so only actually unblock if we've - // previously blocked. - if (mState & FLAG_ONLOAD_BLOCKED) { - mState |= FLAG_ONLOAD_UNBLOCKED; - } -} - void imgStatusTracker::SendUnblockOnload(imgRequestProxy* aProxy) { @@ -917,7 +609,7 @@ imgStatusTracker::MaybeUnblockOnload() return; } - RecordUnblockOnload(); + mState |= FLAG_ONLOAD_UNBLOCKED; ProxyArray::ForwardIterator iter(mConsumers); while (iter.HasMore()) { @@ -928,12 +620,6 @@ imgStatusTracker::MaybeUnblockOnload() } } -void -imgStatusTracker::RecordError() -{ - mState |= FLAG_HAS_ERROR; -} - void imgStatusTracker::FireFailureNotification() { diff --git a/image/src/imgStatusTracker.h b/image/src/imgStatusTracker.h index e4a37111e28..b6f8d55542f 100644 --- a/image/src/imgStatusTracker.h +++ b/image/src/imgStatusTracker.h @@ -7,7 +7,6 @@ #ifndef imgStatusTracker_h__ #define imgStatusTracker_h__ -class imgDecoderObserver; class imgIContainer; class imgStatusNotifyRunnable; class imgRequestNotifyRunnable; @@ -52,6 +51,21 @@ struct ImageStatusDiff static ImageStatusDiff NoChange() { return ImageStatusDiff(); } bool IsNoChange() const { return *this == NoChange(); } + static ImageStatusDiff ForOnStopRequest(bool aLastPart, + bool aError, + nsresult aStatus) + { + ImageStatusDiff diff; + diff.diffState |= FLAG_REQUEST_STOPPED; + if (aLastPart) { + diff.diffState |= FLAG_MULTIPART_STOPPED; + } + if (NS_FAILED(aStatus) || aError) { + diff.diffState |= FLAG_HAS_ERROR; + } + return diff; + } + bool operator!=(const ImageStatusDiff& aOther) const { return !(*this == aOther); } bool operator==(const ImageStatusDiff& aOther) const { return aOther.diffState == diffState; @@ -81,7 +95,7 @@ struct ImageStatusDiff class imgStatusTracker : public mozilla::SupportsWeakPtr { - virtual ~imgStatusTracker(); + virtual ~imgStatusTracker() { } public: MOZ_DECLARE_REFCOUNTED_TYPENAME(imgStatusTracker) @@ -90,7 +104,10 @@ public: // aImage is the image that this status tracker will pass to the // imgRequestProxys in SyncNotify() and EmulateRequestFinished(), and must be // alive as long as this instance is, because we hold a weak reference to it. - explicit imgStatusTracker(mozilla::image::Image* aImage); + explicit imgStatusTracker(mozilla::image::Image* aImage) + : mImage(aImage) + , mState(0) + { } // Image-setter, for imgStatusTrackers created by imgRequest::Init, which // are created before their Image is created. This method should only @@ -163,44 +180,20 @@ public: // Get the current image status (as in imgIRequest). uint32_t GetImageStatus() const; - // Following are all the notification methods. You must call the Record - // variant on this status tracker, then call the Send variant for each proxy - // you want to notify. - - // Call when the request is being cancelled. - void RecordCancel(); - - // Shorthand for recording all the load notifications: StartRequest, - // StartContainer, StopRequest. - void RecordLoaded(); - - // Shorthand for recording all the decode notifications: StartDecode, - // DataAvailable, StopFrame, StopDecode. - void RecordDecoded(); - - /* non-virtual imgDecoderObserver methods */ // Functions with prefix Send- are main thread only, since they contain calls // to imgRequestProxy functions, which are expected on the main thread. - void RecordStartDecode(); void SendStartDecode(imgRequestProxy* aProxy); - void RecordStartContainer(imgIContainer* aContainer); void SendStartContainer(imgRequestProxy* aProxy); - void RecordStopFrame(); void SendStopFrame(imgRequestProxy* aProxy); - void RecordStopDecode(nsresult statusg); void SendStopDecode(imgRequestProxy* aProxy, nsresult aStatus); void SendDiscard(imgRequestProxy* aProxy); - void RecordUnlockedDraw(); void SendUnlockedDraw(imgRequestProxy* aProxy); - void RecordImageIsAnimated(); void SendImageIsAnimated(imgRequestProxy *aProxy); /* non-virtual sort-of-nsIRequestObserver methods */ // Functions with prefix Send- are main thread only, since they contain calls // to imgRequestProxy functions, which are expected on the main thread. - void RecordStartRequest(); void SendStartRequest(imgRequestProxy* aProxy); - void RecordStopRequest(bool aLastPart, nsresult aStatus); void SendStopRequest(imgRequestProxy* aProxy, bool aLastPart, nsresult aStatus); // All main thread only because they call functions (like SendStartRequest) @@ -209,20 +202,14 @@ public: // OnDataAvailable will dispatch a call to itself onto the main thread if not // called there. void OnDataAvailable(); - void OnStopRequest(bool aLastPart, nsresult aStatus); void OnDiscard(); void OnUnlockedDraw(); - // This is called only by VectorImage, and only to ensure tests work - // properly. Do not use it. - void OnStopFrame(); /* non-virtual imgIOnloadBlocker methods */ // NB: If UnblockOnload is sent, and then we are asked to replay the // notifications, we will not send a BlockOnload/UnblockOnload pair. This // is different from all the other notifications. - void RecordBlockOnload(); void SendBlockOnload(imgRequestProxy* aProxy); - void RecordUnblockOnload(); void SendUnblockOnload(imgRequestProxy* aProxy); // Main thread only because mConsumers is not threadsafe. @@ -239,16 +226,8 @@ public: } inline bool HasImage() { return mImage; } - inline imgDecoderObserver* GetDecoderObserver() { return mTrackerObserver.get(); } - - already_AddRefed CloneForRecording(); - // Compute the difference between this status tracker and aOther. - mozilla::image::ImageStatusDiff Difference(imgStatusTracker* aOther) const; - - // Captures all of the decode notifications (i.e., not OnStartRequest / - // OnStopRequest) so far as an ImageStatusDiff. - mozilla::image::ImageStatusDiff DecodeStateAsDifference() const; + 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); @@ -265,7 +244,8 @@ private: friend class imgRequestNotifyRunnable; friend class imgStatusTrackerObserver; friend class imgStatusTrackerInit; - imgStatusTracker(const imgStatusTracker& aOther); + + imgStatusTracker(const imgStatusTracker& aOther) MOZ_DELETE; // Main thread only because it deals with the observer service. void FireFailureNotification(); @@ -286,8 +266,6 @@ private: // on the main thread. ProxyArray mConsumers; - mozilla::RefPtr mTrackerObserver; - uint32_t mState; };