From b2712b400e31dd36d83fb609ef440ba240a5fe9c Mon Sep 17 00:00:00 2001 From: Joe Drew Date: Fri, 18 Jan 2013 16:47:18 -0500 Subject: [PATCH] Bug 716140 - Create a clone imgStatusTracker on every asynchronous decoding event, and replay the difference between that imgStatusTracker and the image's current status tracker when decoding completes. r=seth * * * [mq]: test-fixes * * * [mq]: undefer-on-remove * * * imported patch notify-in-sequence --HG-- extra : rebase_source : 01a495a64cb4b6a8076a3550c254ddd804f53e80 --- image/src/Decoder.cpp | 16 +- image/src/Decoder.h | 2 + image/src/RasterImage.cpp | 279 +++++++++++++++++------- image/src/RasterImage.h | 60 ++++- image/src/imgStatusTracker.cpp | 146 ++++++++++--- image/src/imgStatusTracker.h | 18 +- image/test/unit/async_load_tests.js | 11 + image/test/unit/image_load_helpers.js | 8 +- image/test/unit/test_private_channel.js | 9 + 9 files changed, 419 insertions(+), 130 deletions(-) diff --git a/image/src/Decoder.cpp b/image/src/Decoder.cpp index 476781120988..517c278b2d0d 100644 --- a/image/src/Decoder.cpp +++ b/image/src/Decoder.cpp @@ -135,6 +135,13 @@ Decoder::Finish(RasterImage::eShutdownIntent aShutdownIntent) } } } + + // Set image metadata before calling DecodingComplete, because DecodingComplete calls Optimize(). + mImageMetadata.SetOnImage(&mImage); + + if (mDecodeDone) { + mImage.DecodingComplete(); + } } void @@ -155,9 +162,6 @@ Decoder::FlushInvalidations() if (mInvalidRect.IsEmpty()) return; - // Tell the image that it's been updated - mImage.FrameUpdated(mFrameCount - 1, mInvalidRect); - if (mObserver) { #ifdef XP_MACOSX // Bug 703231 @@ -285,15 +289,9 @@ Decoder::PostDecodeDone(int32_t aLoopCount /* = 0 */) NS_ABORT_IF_FALSE(!mDecodeDone, "Decode already done!"); mDecodeDone = true; - // Set metadata before DecodingComplete(), since DecodingComplete() calls Optimize() mImageMetadata.SetLoopCount(aLoopCount); mImageMetadata.SetIsNonPremultiplied(GetDecodeFlags() & DECODER_NO_PREMULTIPLY_ALPHA); - // Sync metadata to image - mImageMetadata.SetOnImage(&mImage); - - // Notify - mImage.DecodingComplete(); if (mObserver) { mObserver->OnStopDecode(NS_OK); } diff --git a/image/src/Decoder.h b/image/src/Decoder.h index 7f60b7dee42b..14ecbdfaad24 100644 --- a/image/src/Decoder.h +++ b/image/src/Decoder.h @@ -132,6 +132,8 @@ public: // Use HistogramCount as an invalid Histogram ID virtual Telemetry::ID SpeedHistogram() { return Telemetry::HistogramCount; } + ImageMetadata& GetImageMetadata() { return mImageMetadata; } + protected: /* diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index 8097cc0e874b..a8566b5d455b 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -648,8 +648,11 @@ RasterImage::RequestRefresh(const mozilla::TimeStamp& aTime) #endif UpdateImageContainer(); + + // Explicitly call this on mStatusTracker so we're sure to not interfere + // with the decoding process if (mStatusTracker) - mStatusTracker->GetDecoderObserver()->FrameChanged(&dirtyRect); + mStatusTracker->FrameChanged(&dirtyRect); } } @@ -1683,8 +1686,9 @@ RasterImage::ResetAnimation() // we fix bug 500402. // Update display if we were animating before - if (mAnimating && mStatusTracker) - mStatusTracker->GetDecoderObserver()->FrameChanged(&(mAnim->firstFrameRefreshArea)); + if (mAnimating && mStatusTracker) { + mStatusTracker->FrameChanged(&(mAnim->firstFrameRefreshArea)); + } if (ShouldAnimate()) { StartAnimation(); @@ -1844,8 +1848,7 @@ RasterImage::DoImageDataComplete() // directly to the decoder in the AddSourceData() calls. This means we're // done, so we can shut down the decoder. if (!StoringSourceData()) { - nsresult rv = ShutdownDecoder(eShutdownIntent_Done); - CONTAINER_ENSURE_SUCCESS(rv); + RasterImage::FinishedSomeDecoding(this); } // If there's a decoder open, synchronously decode the beginning of the image @@ -2575,8 +2578,6 @@ RasterImage::InitDecoder(bool aDoSizeDecode) CONTAINER_ENSURE_TRUE(type != eDecoderType_unknown, NS_IMAGELIB_ERROR_NO_DECODER); // Instantiate the appropriate decoder - imgDecoderObserver* observer = mStatusTracker ? mStatusTracker->GetDecoderObserver() - : nullptr; switch (type) { case eDecoderType_png: mDecoder = new nsPNGDecoder(*this); @@ -2608,7 +2609,10 @@ RasterImage::InitDecoder(bool aDoSizeDecode) } // Initialize the decoder - mDecoder->SetObserver(observer); + if (!mDecodeRequest) { + mDecodeRequest = new DecodeRequest(this); + } + mDecoder->SetObserver(mDecodeRequest->mStatusTracker->GetDecoderObserver()); mDecoder->SetSizeDecode(aDoSizeDecode); mDecoder->SetDecodeFlags(mFrameDecodeFlags); mDecoder->Init(); @@ -2644,6 +2648,8 @@ RasterImage::InitDecoder(bool aDoSizeDecode) nsresult RasterImage::ShutdownDecoder(eShutdownIntent aIntent) { + MOZ_ASSERT(NS_IsMainThread()); + // Ensure that our intent is valid NS_ABORT_IF_FALSE((aIntent >= 0) && (aIntent < eShutdownIntent_AllCount), "Invalid shutdown intent"); @@ -2820,19 +2826,19 @@ RasterImage::RequestDecodeCore(RequestDecodeType aDecodeType) if (mDecoder && (mDecoder->IsSizeDecode() || mDecoder->GetDecodeFlags() != mFrameDecodeFlags)) { - rv = ShutdownDecoder(eShutdownIntent_NotNeeded); - CONTAINER_ENSURE_SUCCESS(rv); + RasterImage::FinishedSomeDecoding(this, eShutdownIntent_NotNeeded); } // If we don't have a decoder, create one if (!mDecoder) { NS_ABORT_IF_FALSE(mFrames.IsEmpty(), "Trying to decode to non-empty frame-array"); rv = InitDecoder(/* aDoSizeDecode = */ false); + CONTAINER_ENSURE_SUCCESS(rv); } // If we've read all the data we have, we're done - if (mBytesDecoded == mSourceData.Length()) + if (mHasSourceData && mBytesDecoded == mSourceData.Length()) return NS_OK; // If we can do decoding now, do so. Small images will decode completely, @@ -2879,8 +2885,7 @@ RasterImage::SyncDecode() if (mDecoder && (mDecoder->IsSizeDecode() || mDecoder->GetDecodeFlags() != mFrameDecodeFlags)) { - rv = ShutdownDecoder(eShutdownIntent_NotNeeded); - CONTAINER_ENSURE_SUCCESS(rv); + RasterImage::FinishedSomeDecoding(this, eShutdownIntent_NotNeeded); } // If we don't have a decoder, create one @@ -2906,8 +2911,12 @@ RasterImage::SyncDecode() // If we finished the decode, shutdown the decoder if (mDecoder && IsDecodeFinished()) { - rv = ShutdownDecoder(eShutdownIntent_Done); + // We have to shut down the decoder *now*, so we explicitly shut down the + // decoder, and let FinishedSomeDecoding handle the rest for us. + nsRefPtr request = mDecodeRequest; + nsresult rv = ShutdownDecoder(eShutdownIntent_Done); CONTAINER_ENSURE_SUCCESS(rv); + RasterImage::FinishedSomeDecoding(this, eShutdownIntent_Done, request); } // All good if no errors! @@ -2966,7 +2975,7 @@ RasterImage::ScalingDone(ScaleRequest* request, ScaleStatus status) if (mStatusTracker) { imgFrame *scaledFrame = request->dstFrame.get(); scaledFrame->ImageUpdated(scaledFrame->GetRect()); - mStatusTracker->GetDecoderObserver()->FrameChanged(&request->srcRect); + mStatusTracker->FrameChanged(&request->srcRect); } mScaleResult.status = SCALE_DONE; @@ -3113,7 +3122,7 @@ RasterImage::Draw(gfxContext *aContext, // that case we use our animation consumers count as a proxy for lock count. if (mLockCount == 0 || (mAnim && mAnimationConsumers == 0)) { if (mStatusTracker) - mStatusTracker->GetDecoderObserver()->OnUnlockedDraw(); + mStatusTracker->OnUnlockedDraw(); } // We use !mDecoded && mHasSourceData to mean discarded. @@ -3196,7 +3205,7 @@ RasterImage::UnlockImage() PR_LOG(GetCompressedImageAccountingLog(), PR_LOG_DEBUG, ("RasterImage[0x%p] canceling decode because image " "is now unlocked.", this)); - ShutdownDecoder(eShutdownIntent_NotNeeded); + RasterImage::FinishedSomeDecoding(this, eShutdownIntent_NotNeeded); ForceDiscard(); return NS_OK; } @@ -3254,14 +3263,15 @@ RasterImage::IsDecodeFinished() // Precondition NS_ABORT_IF_FALSE(mDecoder, "Can't call IsDecodeFinished() without decoder!"); + // If we aren't storing source data, just presume that it's OK to shut down + // the decoder as we always write all the data we get. + if (!StoringSourceData()) { + return true; + } + // Assume it's not finished bool decodeFinished = false; - // There shouldn't be any reason to call this if we're not storing - // source data - NS_ABORT_IF_FALSE(StoringSourceData(), - "just shut down on SourceDataComplete!"); - // The decode is complete if we got what we wanted... if (mDecoder->IsSizeDecode()) { if (mHasSize) @@ -3292,8 +3302,9 @@ RasterImage::DoError() return; // If we're mid-decode, shut down the decoder. - if (mDecoder) - ShutdownDecoder(eShutdownIntent_Error); + if (mDecoder) { + FinishedSomeDecoding(this, eShutdownIntent_Error); + } // Put the container in an error state mError = true; @@ -3352,6 +3363,86 @@ RasterImage::GetFramesNotified(uint32_t *aFramesNotified) } #endif +/* static */ void +RasterImage::FinishedSomeDecoding(RasterImage* aImage, + eShutdownIntent aIntent /* = eShutdownIntent_Done */, + DecodeRequest* aRequest /* = nullptr */) +{ + MOZ_ASSERT(NS_IsMainThread()); + + nsRefPtr request; + if (aRequest) { + request = aRequest; + } else { + request = aImage->mDecodeRequest; + } + + // Ensure that, if the decoder is the last reference to the image, we don't + // destroy it by destroying the decoder. + nsRefPtr image = aImage; + + bool done = false; + + if (image->mDecoder) { + if (request && request->mChunkCount && !image->mDecoder->IsSizeDecode()) { + Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, request->mChunkCount); + } + + // If the decode finished, tell the image and shut down the decoder. + if (image->mDecoder->GetDecodeDone() || image->IsDecodeFinished() || + aIntent != eShutdownIntent_Done) { + done = true; + + // Hold on to a reference to the decoder until we're done with it + nsRefPtr decoder = image->mDecoder; + + // We need to shut down the decoder first, in order to ensure all + // decoding routines have been finished. + nsresult rv = image->ShutdownDecoder(aIntent); + if (NS_FAILED(rv)) { + image->DoError(); + } + + // Do some telemetry if this isn't a size decode. + if (request && !decoder->IsSizeDecode()) { + Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME, + int32_t(request->mDecodeTime.ToMicroseconds())); + + // We record the speed for only some decoders. The rest have + // SpeedHistogram return HistogramCount. + Telemetry::ID id = decoder->SpeedHistogram(); + if (id < Telemetry::HistogramCount) { + int32_t KBps = int32_t(request->mImage->mBytesDecoded / + (1024 * request->mDecodeTime.ToSeconds())); + Telemetry::Accumulate(id, KBps); + } + } + } + } + + // If it have any frames, tell the image what happened to the most recent + // one. This will be better when frames are published to decoders instead of + // decoders requesting them. + if (image->GetNumFrames()) { + nsIntRect rect; + if (request) { + rect = request->mStatusTracker->GetInvalidRect(); + } else { + rect = image->CurrentStatusTracker().GetInvalidRect(); + } + aImage->FrameUpdated(image->GetNumFrames() - 1, rect); + } + + // Then, tell the observers what happened in the decoder. + // If we have no request, we have not yet created a decoder, but we still + // need to send out notifications. + if (request) { + image->mStatusTracker->SyncAndSyncNotifyDifference(request->mStatusTracker); + } else { + image->mStatusTracker->SyncNotifyDecodeState(); + } +} + /* static */ RasterImage::DecodeWorker* RasterImage::DecodeWorker::Singleton() { @@ -3363,13 +3454,33 @@ RasterImage::DecodeWorker::Singleton() return sSingleton; } +RasterImage::DecodeWorker::~DecodeWorker() +{ + MOZ_ASSERT(NS_IsMainThread(), "Must shut down DecodeWorker on main thread!"); + + // Shut down all the decoders since we're going away. + DecodeRequest* request = mASAPDecodeRequests.getFirst(); + while (request) { + RasterImage::FinishedSomeDecoding(request->mImage, eShutdownIntent_NotNeeded); + + request = request->getNext(); + } + + request = mNormalDecodeRequests.getFirst(); + while (request) { + RasterImage::FinishedSomeDecoding(request->mImage, eShutdownIntent_NotNeeded); + + request = request->getNext(); + } +} + void RasterImage::DecodeWorker::MarkAsASAP(RasterImage* aImg) { // We can be marked as ASAP before we've been asked to decode. If we are, // create the request so we have somewhere to write down our status. - if (aImg->mDecodeRequest) { - CreateRequestForImage(aImg); + if (!aImg->mDecodeRequest) { + aImg->mDecodeRequest = new DecodeRequest(aImg); } DecodeRequest* request = aImg->mDecodeRequest; @@ -3401,7 +3512,7 @@ RasterImage::DecodeWorker::MarkAsASAP(RasterImage* aImg) } void -RasterImage::DecodeWorker::AddDecodeRequest(DecodeRequest* aRequest) +RasterImage::DecodeWorker::AddDecodeRequest(DecodeRequest* aRequest, uint32_t bytesToDecode) { if (aRequest->isInList()) { // The image is already in our list of images to decode, so we don't have @@ -3409,6 +3520,8 @@ RasterImage::DecodeWorker::AddDecodeRequest(DecodeRequest* aRequest) return; } + aRequest->mBytesToDecode = bytesToDecode; + if (aRequest->mIsASAP) { mASAPDecodeRequests.insertBack(aRequest); } else { @@ -3416,27 +3529,22 @@ RasterImage::DecodeWorker::AddDecodeRequest(DecodeRequest* aRequest) } } -void -RasterImage::DecodeWorker::CreateRequestForImage(RasterImage* aImg) -{ - aImg->mDecodeRequest = new DecodeRequest(aImg); -} - void RasterImage::DecodeWorker::RequestDecode(RasterImage* aImg) { - if (!aImg->mDecodeRequest) { - CreateRequestForImage(aImg); - } + MOZ_ASSERT(aImg->mDecoder); - AddDecodeRequest(aImg->mDecodeRequest); + AddDecodeRequest(aImg->mDecodeRequest, aImg->mSourceData.Length() - aImg->mBytesDecoded); EnsurePendingInEventLoop(); } void RasterImage::DecodeWorker::DecodeABitOf(RasterImage* aImg) { + MOZ_ASSERT(NS_IsMainThread()); + DecodeSomeOfImage(aImg); + RasterImage::FinishedSomeDecoding(aImg); // If we aren't yet finished decoding and we have more data in hand, add // this request to the back of the priority list. @@ -3466,7 +3574,6 @@ RasterImage::DecodeWorker::StopDecoding(RasterImage* aImg) if (aImg->mDecodeRequest->isInList()) { aImg->mDecodeRequest->remove(); } - aImg->mDecodeRequest = nullptr; } } @@ -3492,16 +3599,29 @@ RasterImage::DecodeWorker::Run() // This has to be a strong pointer, because DecodeSomeOfImage may destroy // image->mDecoder, which may be holding the only other reference to image. - RefPtr image = request->mImage; - DecodeSomeOfImage(image); + nsRefPtr image = request->mImage; + uint32_t oldCount = image->mDecoder->GetFrameCount(); + uint32_t oldByteCount = image->mBytesDecoded; + + DecodeSomeOfImage(image, DECODE_TYPE_NORMAL, request->mBytesToDecode); + + uint32_t bytesDecoded = image->mBytesDecoded - oldByteCount; // If we aren't yet finished decoding and we have more data in hand, add // this request to the back of the list. if (image->mDecoder && !image->mError && !image->IsDecodeFinished() && - image->mSourceData.Length() > image->mBytesDecoded) { - AddDecodeRequest(request); + bytesDecoded < request->mBytesToDecode) { + AddDecodeRequest(request, request->mBytesToDecode - bytesDecoded); + + // If we have a new frame, let everybody know about it. + if (image->mDecoder->GetFrameCount() != oldCount) { + DecodeDoneWorker::DidSomeDecoding(image, request); + } + } else { + // Nothing more for us to do - let everyone know what happened. + DecodeDoneWorker::DidSomeDecoding(image, request); } } while ((TimeStamp::Now() - eventStart).ToMilliseconds() <= gMaxMSBeforeYield); @@ -3520,13 +3640,19 @@ RasterImage::DecodeWorker::Run() nsresult RasterImage::DecodeWorker::DecodeUntilSizeAvailable(RasterImage* aImg) { - return DecodeSomeOfImage(aImg, DECODE_TYPE_UNTIL_SIZE); + MOZ_ASSERT(NS_IsMainThread()); + + nsresult rv = DecodeSomeOfImage(aImg, DECODE_TYPE_UNTIL_SIZE); + + RasterImage::FinishedSomeDecoding(aImg); + + return rv; } nsresult -RasterImage::DecodeWorker::DecodeSomeOfImage( - RasterImage* aImg, - DecodeType aDecodeType /* = DECODE_TYPE_NORMAL */) +RasterImage::DecodeWorker::DecodeSomeOfImage(RasterImage* aImg, + DecodeType aDecodeType /* = DECODE_TYPE_NORMAL */, + uint32_t bytesToDecode /* = 0 */) { NS_ABORT_IF_FALSE(aImg->mInitialized, "Worker active for uninitialized container!"); @@ -3555,6 +3681,10 @@ RasterImage::DecodeWorker::DecodeSomeOfImage( maxBytes = gDecodeBytesAtATime; } + if (bytesToDecode == 0) { + bytesToDecode = aImg->mSourceData.Length() - aImg->mBytesDecoded; + } + int32_t chunkCount = 0; TimeStamp start = TimeStamp::Now(); TimeStamp deadline = start + TimeDuration::FromMilliseconds(gMaxMSBeforeYield); @@ -3565,31 +3695,28 @@ RasterImage::DecodeWorker::DecodeSomeOfImage( // * we're an UNTIL_SIZE decode and we get the size, or // * we run out of time. while (aImg->mSourceData.Length() > aImg->mBytesDecoded && + bytesToDecode > 0 && !aImg->IsDecodeFinished() && !(aDecodeType == DECODE_TYPE_UNTIL_SIZE && aImg->mHasSize)) { chunkCount++; - nsresult rv = aImg->DecodeSomeData(maxBytes); + uint32_t chunkSize = std::min(bytesToDecode, maxBytes); + nsresult rv = aImg->DecodeSomeData(chunkSize); if (NS_FAILED(rv)) { aImg->DoError(); return rv; } + bytesToDecode -= chunkSize; + // Yield if we've been decoding for too long. We check this _after_ decoding // a chunk to ensure that we don't yield without doing any decoding. if (TimeStamp::Now() >= deadline) break; } - if (!aImg->mDecodeRequest) { - MOZ_ASSERT(aDecodeType == DECODE_TYPE_UNTIL_SIZE); - } - if (aImg->mDecodeRequest) { aImg->mDecodeRequest->mDecodeTime += (TimeStamp::Now() - start); - } - - if (chunkCount && !aImg->mDecoder->IsSizeDecode()) { - Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, chunkCount); + aImg->mDecodeRequest->mChunkCount += chunkCount; } // Flush invalidations (and therefore paint) now that we've decoded all the @@ -3617,30 +3744,30 @@ RasterImage::DecodeWorker::DecodeSomeOfImage( aImg->mInDecoder = false; } - // If the decode finished, shut down the decoder. - if (aImg->mDecoder && aImg->IsDecodeFinished()) { + return NS_OK; +} - // Do some telemetry if this isn't a size decode. - DecodeRequest* request = aImg->mDecodeRequest; - if (request && !aImg->mDecoder->IsSizeDecode()) { - Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME, - int32_t(request->mDecodeTime.ToMicroseconds())); +RasterImage::DecodeDoneWorker::DecodeDoneWorker(RasterImage* image, DecodeRequest* request) + : mImage(image) + , mRequest(request) +{} - // We record the speed for only some decoders. The rest have - // SpeedHistogram return HistogramCount. - Telemetry::ID id = aImg->mDecoder->SpeedHistogram(); - if (id < Telemetry::HistogramCount) { - int32_t KBps = int32_t(request->mImage->mBytesDecoded / - (1024 * request->mDecodeTime.ToSeconds())); - Telemetry::Accumulate(id, KBps); - } - } +void +RasterImage::DecodeDoneWorker::DidSomeDecoding(RasterImage* image, DecodeRequest* request) +{ + nsCOMPtr worker = new DecodeDoneWorker(image, request); + NS_DispatchToMainThread(worker); +} - nsresult rv = aImg->ShutdownDecoder(RasterImage::eShutdownIntent_Done); - if (NS_FAILED(rv)) { - aImg->DoError(); - return rv; - } +NS_IMETHODIMP +RasterImage::DecodeDoneWorker::Run() +{ + RasterImage::FinishedSomeDecoding(mImage, eShutdownIntent_Done, mRequest); + + // If we didn't finish decoding yet, try again + if (mImage->mDecoder && !mImage->IsDecodeFinished() && + mImage->mSourceData.Length() > mImage->mBytesDecoded) { + DecodeWorker::Singleton()->RequestDecode(mImage); } return NS_OK; diff --git a/image/src/RasterImage.h b/image/src/RasterImage.h index 80c45af90226..f419025c1036 100644 --- a/image/src/RasterImage.h +++ b/image/src/RasterImage.h @@ -334,6 +334,15 @@ public: }; private: + imgStatusTracker& CurrentStatusTracker() + { + if (mDecodeRequest) { + return *mDecodeRequest->mStatusTracker; + } else { + return *mStatusTracker; + } + } + struct Anim { //! Area of the first frame that needs to be redrawn on subsequent loops. @@ -381,16 +390,28 @@ private: { DecodeRequest(RasterImage* aImage) : mImage(aImage) + , mBytesToDecode(0) + , mChunkCount(0) , mIsASAP(false) { + mStatusTracker = aImage->mStatusTracker->CloneForRecording(); } + // The status tracker that is associated with a given decode request, to + // ensure their lifetimes are linked. + nsRefPtr mStatusTracker; + RasterImage* mImage; + uint32_t mBytesToDecode; + /* Keeps track of how much time we've burned decoding this particular decode * request. */ TimeDuration mDecodeTime; + /* The number of chunks it took to decode this image. */ + int32_t mChunkCount; + /* True if we need to handle this decode as soon as possible. */ bool mIsASAP; }; @@ -459,6 +480,8 @@ private: NS_IMETHOD Run(); + virtual ~DecodeWorker(); + private: /* statics */ static StaticRefPtr sSingleton; @@ -472,7 +495,7 @@ private: /* Add the given request to the appropriate list of decode requests, but * don't ensure that we're pending in the event loop. */ - void AddDecodeRequest(DecodeRequest* aRequest); + void AddDecodeRequest(DecodeRequest* aRequest, uint32_t bytesToDecode); enum DecodeType { DECODE_TYPE_NORMAL, @@ -480,9 +503,11 @@ private: }; /* Decode some chunks of the given image. If aDecodeType is UNTIL_SIZE, - * decode until we have the image's size, then stop. */ + * decode until we have the image's size, then stop. If bytesToDecode is + * non-0, at most bytesToDecode bytes will be decoded. */ nsresult DecodeSomeOfImage(RasterImage* aImg, - DecodeType aDecodeType = DECODE_TYPE_NORMAL); + DecodeType aDecodeType = DECODE_TYPE_NORMAL, + uint32_t bytesToDecode = 0); /* Create a new DecodeRequest suitable for doing some decoding and set it * as aImg's mDecodeRequest. */ @@ -498,6 +523,33 @@ private: bool mPendingInEventLoop; }; + class DecodeDoneWorker : public nsRunnable + { + public: + /** + * Called by the DecodeWorker with an image when it's done some significant + * portion of decoding that needs to be notified about. + * + * Ensures the decode state accumulated by the decoding process gets + * applied to the image. + */ + static void DidSomeDecoding(RasterImage* image, DecodeRequest* request); + + NS_IMETHOD Run(); + + private: /* methods */ + DecodeDoneWorker(RasterImage* image, DecodeRequest* request); + + private: /* members */ + + nsRefPtr mImage; + nsRefPtr mRequest; + }; + + static void FinishedSomeDecoding(RasterImage* image, + eShutdownIntent intent = eShutdownIntent_Done, + DecodeRequest* request = nullptr); + void DrawWithPreDownscaleIfNeeded(imgFrame *aFrame, gfxContext *aContext, gfxPattern::GraphicsFilter aFilter, @@ -567,7 +619,7 @@ private: LockImage(); // Notify our observers that we are starting animation. - mStatusTracker->RecordImageIsAnimated(); + CurrentStatusTracker().RecordImageIsAnimated(); } } diff --git a/image/src/imgStatusTracker.cpp b/image/src/imgStatusTracker.cpp index 41e6d16608f3..62cc3788d98c 100644 --- a/image/src/imgStatusTracker.cpp +++ b/image/src/imgStatusTracker.cpp @@ -285,17 +285,17 @@ private: imgStatusTracker::imgStatusTracker(Image* aImage) : mImage(aImage), - mTrackerObserver(new imgStatusTrackerNotifyingObserver(this)), mState(0), mImageStatus(imgIRequest::STATUS_NONE), mIsMultipart(false), mHadLastPart(false) -{} +{ + mTrackerObserver = new imgStatusTrackerObserver(this); +} // Private, used only by CloneForRecording. imgStatusTracker::imgStatusTracker(const imgStatusTracker& aOther) : mImage(aOther.mImage), - mTrackerObserver(new imgStatusTrackerNotifyingObserver(this)), mState(aOther.mState), mImageStatus(aOther.mImageStatus), mIsMultipart(aOther.mIsMultipart), @@ -307,7 +307,9 @@ imgStatusTracker::imgStatusTracker(const imgStatusTracker& aOther) // object // - mConsumers, because we don't need to talk to consumers // - mInvalidRect, because the point of it is to be fired off and reset -{} +{ + mTrackerObserver = new imgStatusTrackerObserver(this); +} imgStatusTracker::~imgStatusTracker() {} @@ -361,6 +363,11 @@ class imgRequestNotifyRunnable : public nsRunnable mProxies.AppendElement(aRequestProxy); } + void RemoveProxy(imgRequestProxy* aRequestProxy) + { + mProxies.RemoveElement(aRequestProxy); + } + private: friend class imgStatusTracker; @@ -440,26 +447,37 @@ imgStatusTracker::NotifyCurrentState(imgRequestProxy* proxy) NS_DispatchToCurrentThread(ev); } -/* static */ void -imgStatusTracker::SyncNotifyState(imgRequestProxy* proxy, bool hasImage, uint32_t state, nsIntRect& dirtyRect, bool hadLastPart) -{ - nsCOMPtr kungFuDeathGrip(proxy); +#define NOTIFY_IMAGE_OBSERVERS(func) \ + do { \ + nsTObserverArray::ForwardIterator iter(proxies); \ + while (iter.HasMore()) { \ + nsRefPtr proxy = iter.GetNext(); \ + if (!proxy->NotificationsDeferred()) { \ + proxy->func; \ + } \ + } \ + } while (false); +/* static */ void +imgStatusTracker::SyncNotifyState(nsTObserverArray& proxies, + bool hasImage, uint32_t state, + nsIntRect& dirtyRect, bool hadLastPart) +{ // OnStartRequest if (state & stateRequestStarted) - proxy->OnStartRequest(); + NOTIFY_IMAGE_OBSERVERS(OnStartRequest()); // OnStartContainer if (state & stateHasSize) - proxy->OnStartContainer(); + NOTIFY_IMAGE_OBSERVERS(OnStartContainer()); // OnStartDecode if (state & stateDecodeStarted) - proxy->OnStartDecode(); + NOTIFY_IMAGE_OBSERVERS(OnStartDecode()); // BlockOnload if (state & stateBlockingOnload) - proxy->BlockOnload(); + NOTIFY_IMAGE_OBSERVERS(BlockOnload()); if (hasImage) { // OnFrameUpdate @@ -467,60 +485,79 @@ imgStatusTracker::SyncNotifyState(imgRequestProxy* proxy, bool hasImage, uint32_ // vector images, true for raster images that have decoded at // least one frame) then send OnFrameUpdate. if (!dirtyRect.IsEmpty()) - proxy->OnFrameUpdate(&dirtyRect); + NOTIFY_IMAGE_OBSERVERS(OnFrameUpdate(&dirtyRect)); if (state & stateFrameStopped) - proxy->OnStopFrame(); + NOTIFY_IMAGE_OBSERVERS(OnStopFrame()); // OnImageIsAnimated if (state & stateImageIsAnimated) - proxy->OnImageIsAnimated(); + NOTIFY_IMAGE_OBSERVERS(OnImageIsAnimated()); } if (state & stateDecodeStopped) { NS_ABORT_IF_FALSE(hasImage, "stopped decoding without ever having an image?"); - proxy->OnStopDecode(); + NOTIFY_IMAGE_OBSERVERS(OnStopDecode()); } if (state & stateRequestStopped) { - proxy->OnStopRequest(hadLastPart); + NOTIFY_IMAGE_OBSERVERS(OnStopRequest(hadLastPart)); } } void imgStatusTracker::SyncAndSyncNotifyDifference(imgStatusTracker* other) { - uint32_t diffState = ~mState & other->mState; + // We must not modify or notify for the begin-load state, which happens from Necko callbacks. + uint32_t loadState = mState & stateRequestStarted; + uint32_t diffState = ~mState & other->mState & ~stateRequestStarted; bool unblockedOnload = mState & stateBlockingOnload && !(other->mState & stateBlockingOnload); - bool foundError = mImageStatus == imgIRequest::STATUS_ERROR; + bool foundError = (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. // First, actually synchronize our state. mInvalidRect = mInvalidRect.Union(other->mInvalidRect); - mState |= other->mState; + mState |= diffState | loadState; + if (unblockedOnload) { + mState &= ~stateBlockingOnload; + } mImageStatus = other->mImageStatus; mIsMultipart = other->mIsMultipart; mHadLastPart = other->mHadLastPart; - // Now that we've updated our state, notify all the consumers about the state - // that's changed. - nsTObserverArray::ForwardIterator iter(mConsumers); - while (iter.HasMore()) { - imgRequestProxy* proxy = iter.GetNext(); + // The error state is sticky and overrides all other bits. + if (mImageStatus == imgIRequest::STATUS_ERROR || + other->mImageStatus == imgIRequest::STATUS_ERROR) { + mImageStatus = imgIRequest::STATUS_ERROR; + } else { + mImageStatus |= other->mImageStatus; - if (!proxy->NotificationsDeferred()) { - SyncNotifyState(proxy, mImage, diffState, mInvalidRect, other->mHadLastPart); + // 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; + } + } - if (unblockedOnload) { + SyncNotifyState(mConsumers, !!mImage, diffState, mInvalidRect, mHadLastPart); + + if (unblockedOnload) { + nsTObserverArray::ForwardIterator iter(mConsumers); + while (iter.HasMore()) { + // Hold on to a reference to this proxy, since notifying the state can + // cause it to disappear. + nsRefPtr proxy = iter.GetNext(); + + if (!proxy->NotificationsDeferred()) { SendUnblockOnload(proxy); } } } - // Reset the other rectangle for another go, if it's going to have one. + // Reset the invalid rectangles for another go. other->mInvalidRect.SetEmpty(); + mInvalidRect.SetEmpty(); if (foundError) { FireFailureNotification(); @@ -552,7 +589,17 @@ imgStatusTracker::SyncNotify(imgRequestProxy* proxy) r = mImage->FrameRect(imgIContainer::FRAME_CURRENT); } - SyncNotifyState(proxy, !!mImage, mState, r, mHadLastPart); + nsTObserverArray array; + array.AppendElement(proxy); + SyncNotifyState(array, !!mImage, mState, r, mHadLastPart); +} + +void +imgStatusTracker::SyncNotifyDecodeState() +{ + SyncNotifyState(mConsumers, !!mImage, mState & ~stateRequestStarted, mInvalidRect, mHadLastPart); + + mInvalidRect.SetEmpty(); } void @@ -591,8 +638,18 @@ imgStatusTracker::RemoveConsumer(imgRequestProxy* aConsumer, nsresult aStatus) // Consumers can get confused if they don't get all the proper teardown // notifications. Part ways on good terms. - if (removed) + if (removed && !aConsumer->NotificationsDeferred()) { EmulateRequestFinished(aConsumer, aStatus); + } + + // Make sure we don't give callbacks to a consumer that isn't interested in + // them any more. + imgRequestNotifyRunnable* runnable = static_cast(mRequestRunnable.get()); + if (aConsumer->NotificationsDeferred() && runnable) { + runnable->RemoveProxy(aConsumer); + aConsumer->SetNotificationsDeferred(false); + } + return removed; } @@ -737,7 +794,7 @@ imgStatusTracker::RecordImageIsAnimated() { NS_ABORT_IF_FALSE(mImage, "RecordImageIsAnimated called before we have an Image"); - mImageStatus |= stateImageIsAnimated; + mState |= stateImageIsAnimated; } void @@ -754,6 +811,16 @@ imgStatusTracker::SendUnlockedDraw(imgRequestProxy* aProxy) aProxy->OnUnlockedDraw(); } +void +imgStatusTracker::OnUnlockedDraw() +{ + RecordUnlockedDraw(); + nsTObserverArray::ForwardIterator iter(mConsumers); + while (iter.HasMore()) { + SendUnlockedDraw(iter.GetNext()); + } +} + void imgStatusTracker::RecordFrameChanged(const nsIntRect* aDirtyRect) { @@ -861,6 +928,18 @@ imgStatusTracker::OnDiscard() } } +void +imgStatusTracker::FrameChanged(const nsIntRect* aDirtyRect) +{ + RecordFrameChanged(aDirtyRect); + + /* notify the kids */ + nsTObserverArray::ForwardIterator iter(mConsumers); + while (iter.HasMore()) { + SendFrameChanged(iter.GetNext(), aDirtyRect); + } +} + void imgStatusTracker::OnDataAvailable() { @@ -889,7 +968,6 @@ imgStatusTracker::SendBlockOnload(imgRequestProxy* aProxy) void imgStatusTracker::RecordUnblockOnload() { - MOZ_ASSERT(mState & stateBlockingOnload); mState &= ~stateBlockingOnload; } @@ -919,6 +997,8 @@ imgStatusTracker::MaybeUnblockOnload() void imgStatusTracker::FireFailureNotification() { + MOZ_ASSERT(NS_IsMainThread()); + // Some kind of problem has happened with image decoding. // Report the URI to net:failed-to-process-uri-conent observers. nsCOMPtr uri = GetImage()->GetURI(); diff --git a/image/src/imgStatusTracker.h b/image/src/imgStatusTracker.h index 10c0f80d858a..49ed28a952ac 100644 --- a/image/src/imgStatusTracker.h +++ b/image/src/imgStatusTracker.h @@ -89,6 +89,11 @@ public: // OnStartRequest). void SyncNotify(imgRequestProxy* proxy); + // "Replays" all of the decode notifications (i.e., not + // OnStartRequest/OnStopRequest) that have happened to us to all of our + // non-deferred proxies. + void SyncNotifyDecodeState(); + // Send some notifications that would be necessary to make |proxy| believe // the request is finished downloading and decoding. We only send // OnStopRequest and UnblockOnload, and only if necessary. @@ -161,6 +166,8 @@ public: void OnDataAvailable(); void OnStopRequest(bool aLastPart, nsresult aStatus); void OnDiscard(); + void FrameChanged(const nsIntRect* aDirtyRect); + void OnUnlockedDraw(); /* non-virtual imgIOnloadBlocker methods */ // NB: If UnblockOnload is sent, and then we are asked to replay the @@ -181,6 +188,9 @@ public: inline imgDecoderObserver* GetDecoderObserver() { return mTrackerObserver.get(); } imgStatusTracker* CloneForRecording(); + void SyncAndSyncNotifyDifference(imgStatusTracker* other); + + nsIntRect GetInvalidRect() const { return mInvalidRect; } private: friend class imgStatusNotifyRunnable; @@ -191,11 +201,9 @@ private: void FireFailureNotification(); - static void SyncNotifyState(imgRequestProxy* proxy, bool hasImage, - uint32_t state, nsIntRect& dirtyRect, - bool hadLastPart); - - void SyncAndSyncNotifyDifference(imgStatusTracker* other); + static void SyncNotifyState(nsTObserverArray& proxies, + bool hasImage, uint32_t state, + nsIntRect& dirtyRect, bool hadLastPart); nsCOMPtr mRequestRunnable; diff --git a/image/test/unit/async_load_tests.js b/image/test/unit/async_load_tests.js index e5b824c83277..c1dac0025f58 100644 --- a/image/test/unit/async_load_tests.js +++ b/image/test/unit/async_load_tests.js @@ -48,6 +48,7 @@ function checkClone(other_listener, aRequest) var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools) .createScriptedObserver(listener); var clone = aRequest.clone(outer); + requests.push(clone); } // Ensure that all the callbacks were called on aRequest. @@ -73,6 +74,7 @@ function secondLoadDone(oldlistener, aRequest) var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools) .createScriptedObserver(listener); var staticrequestclone = staticrequest.clone(outer); + requests.push(staticrequestclone); } catch(e) { // We can't create a static request. Most likely the request we started // with didn't load successfully. @@ -205,8 +207,17 @@ function startImageCallback(otherCb) var gCurrentLoader; +function cleanup() +{ + for (var i = 0; i < requests.length; ++i) { + requests[i].cancelAndForgetObserver(0); + } +} + function run_test() { + do_register_cleanup(cleanup); + gCurrentLoader = Cc["@mozilla.org/image/loader;1"].createInstance(Ci.imgILoader); do_test_pending(); diff --git a/image/test/unit/image_load_helpers.js b/image/test/unit/image_load_helpers.js index db9aec17c35f..e2210bcc9dfa 100644 --- a/image/test/unit/image_load_helpers.js +++ b/image/test/unit/image_load_helpers.js @@ -45,9 +45,11 @@ function ImageListener(start_callback, stop_callback) { do_check_false(this.synchronous); - // We have to cancel the request when we're done with it to break any - // reference loops! - aRequest.cancelAndForgetObserver(0); + try { + aRequest.requestDecode(); + } catch (e) { + do_print("requestDecode threw " + e); + } this.state |= LOAD_COMPLETE; diff --git a/image/test/unit/test_private_channel.js b/image/test/unit/test_private_channel.js index b9a9cc464501..78ab8f48da1f 100644 --- a/image/test/unit/test_private_channel.js +++ b/image/test/unit/test_private_channel.js @@ -94,7 +94,16 @@ function run_loadImage_tests() { }); } +function cleanup() +{ + for (var i = 0; i < requests.length; ++i) { + requests[i].cancelAndForgetObserver(0); + } +} + function run_test() { + do_register_cleanup(cleanup); + do_test_pending(); // We create a public channel that loads an image, then an identical