Bug 716140 - Heap-allocate DecodeRequests so we know when we're still decoding an image. r=seth
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -363,26 +363,16 @@ RasterImage::RasterImage(imgStatusTracke
nsIURI* aURI /* = nullptr */) :
ImageResource(aStatusTracker, aURI), // invoke superclass's constructor
mSize(0,0),
mFrameDecodeFlags(DECODE_FLAGS_DEFAULT),
mAnim(nullptr),
mLoopCount(-1),
mLockCount(0),
mDecoder(nullptr),
-// We know DecodeRequest won't touch members of RasterImage
-// until this constructor completes
-#ifdef _MSC_VER
-#pragma warning(push)
-#pragma warning(disable : 4355)
-#endif
- mDecodeRequest(this),
-#ifdef _MSC_VER
-#pragma warning(pop)
-#endif
mBytesDecoded(0),
mDecodeCount(0),
#ifdef DEBUG
mFramesNotified(0),
#endif
mHasSize(false),
mDecodeOnDraw(false),
mMultipart(false),
@@ -2629,17 +2619,17 @@ RasterImage::InitDecoder(bool aDoSizeDec
mDecodeCount++;
Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(mDecodeCount);
if (mDecodeCount > sMaxDecodeCount) {
// Don't subtract out 0 from the histogram, because that causes its count
// to go negative, which is not kosher.
if (sMaxDecodeCount > 0) {
Telemetry::GetHistogramById(Telemetry::IMAGE_MAX_DECODE_COUNT)->Subtract(sMaxDecodeCount);
- }
+ }
sMaxDecodeCount = mDecodeCount;
Telemetry::GetHistogramById(Telemetry::IMAGE_MAX_DECODE_COUNT)->Add(sMaxDecodeCount);
}
}
return NS_OK;
}
@@ -3371,17 +3361,23 @@ RasterImage::DecodeWorker::Singleton()
}
return sSingleton;
}
void
RasterImage::DecodeWorker::MarkAsASAP(RasterImage* aImg)
{
- DecodeRequest* request = &aImg->mDecodeRequest;
+ // 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);
+ }
+
+ DecodeRequest* request = aImg->mDecodeRequest;
// If we're already an ASAP request, there's nothing to do here.
if (request->mIsASAP) {
return;
}
request->mIsASAP = true;
@@ -3416,19 +3412,29 @@ RasterImage::DecodeWorker::AddDecodeRequ
if (aRequest->mIsASAP) {
mASAPDecodeRequests.insertBack(aRequest);
} else {
mNormalDecodeRequests.insertBack(aRequest);
}
}
void
+RasterImage::DecodeWorker::CreateRequestForImage(RasterImage* aImg)
+{
+ aImg->mDecodeRequest = new DecodeRequest(aImg);
+}
+
+void
RasterImage::DecodeWorker::RequestDecode(RasterImage* aImg)
{
- AddDecodeRequest(&aImg->mDecodeRequest);
+ if (!aImg->mDecodeRequest) {
+ CreateRequestForImage(aImg);
+ }
+
+ AddDecodeRequest(aImg->mDecodeRequest);
EnsurePendingInEventLoop();
}
void
RasterImage::DecodeWorker::DecodeABitOf(RasterImage* aImg)
{
DecodeSomeOfImage(aImg);
@@ -3449,22 +3455,24 @@ RasterImage::DecodeWorker::EnsurePending
mPendingInEventLoop = true;
NS_DispatchToCurrentThread(this);
}
}
void
RasterImage::DecodeWorker::StopDecoding(RasterImage* aImg)
{
- DecodeRequest* request = &aImg->mDecodeRequest;
- if (request->isInList()) {
- request->remove();
+ // If we haven't got a decode request, we're not currently decoding. (Having
+ // a decode request doesn't imply we *are* decoding, though.)
+ if (aImg->mDecodeRequest) {
+ if (aImg->mDecodeRequest->isInList()) {
+ aImg->mDecodeRequest->remove();
+ }
+ aImg->mDecodeRequest = nullptr;
}
- request->mDecodeTime = TimeDuration(0);
- request->mIsASAP = false;
}
NS_IMETHODIMP
RasterImage::DecodeWorker::Run()
{
// We just got called back by the event loop; therefore, we're no longer
// pending.
mPendingInEventLoop = false;
@@ -3479,17 +3487,17 @@ RasterImage::DecodeWorker::Run()
DecodeRequest* request = mASAPDecodeRequests.popFirst();
if (!request)
request = mNormalDecodeRequests.popFirst();
if (!request)
break;
// This has to be a strong pointer, because DecodeSomeOfImage may destroy
// image->mDecoder, which may be holding the only other reference to image.
- nsRefPtr<RasterImage> image = request->mImage;
+ RefPtr<RasterImage> image = request->mImage;
DecodeSomeOfImage(image);
// 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) {
@@ -3567,17 +3575,23 @@ RasterImage::DecodeWorker::DecodeSomeOfI
}
// 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;
}
- aImg->mDecodeRequest.mDecodeTime += (TimeStamp::Now() - start);
+ 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);
}
// Flush invalidations (and therefore paint) now that we've decoded all the
// chunks we're going to.
//
@@ -3602,18 +3616,18 @@ RasterImage::DecodeWorker::DecodeSomeOfI
aImg->mDecoder->FlushInvalidations();
aImg->mInDecoder = false;
}
// If the decode finished, shut down the decoder.
if (aImg->mDecoder && aImg->IsDecodeFinished()) {
// Do some telemetry if this isn't a size decode.
- DecodeRequest* request = &aImg->mDecodeRequest;
- if (!aImg->mDecoder->IsSizeDecode()) {
+ DecodeRequest* request = aImg->mDecodeRequest;
+ if (request && !aImg->mDecoder->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 = aImg->mDecoder->SpeedHistogram();
if (id < Telemetry::HistogramCount) {
int32_t KBps = int32_t(request->mImage->mBytesDecoded /
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -28,16 +28,17 @@
#include "imgFrame.h"
#include "nsThreadUtils.h"
#include "DiscardTracker.h"
#include "mozilla/TimeStamp.h"
#include "mozilla/Telemetry.h"
#include "mozilla/LinkedList.h"
#include "mozilla/StaticPtr.h"
#include "mozilla/WeakPtr.h"
+#include "mozilla/RefPtr.h"
#include "gfx2DGlue.h"
#ifdef DEBUG
#include "imgIContainerDebug.h"
#endif
class nsIInputStream;
#define NS_RASTERIMAGE_CID \
@@ -367,27 +368,29 @@ private:
lastCompositedFrameIndex(-1) {}
~Anim() {}
};
/**
* DecodeWorker keeps a linked list of DecodeRequests to keep track of the
* images it needs to decode.
*
- * Each RasterImage has a single DecodeRequest member.
+ * Each RasterImage has a pointer to one or zero heap-allocated
+ * DecodeRequests.
*/
- struct DecodeRequest : public LinkedListElement<DecodeRequest>
+ struct DecodeRequest : public LinkedListElement<DecodeRequest>,
+ public RefCounted<DecodeRequest>
{
DecodeRequest(RasterImage* aImage)
: mImage(aImage)
, mIsASAP(false)
{
}
- RasterImage* const mImage;
+ RasterImage* mImage;
/* Keeps track of how much time we've burned decoding this particular decode
* request. */
TimeDuration mDecodeTime;
/* True if we need to handle this decode as soon as possible. */
bool mIsASAP;
};
@@ -476,16 +479,20 @@ private:
DECODE_TYPE_UNTIL_SIZE
};
/* Decode some chunks of the given image. If aDecodeType is UNTIL_SIZE,
* decode until we have the image's size, then stop. */
nsresult DecodeSomeOfImage(RasterImage* aImg,
DecodeType aDecodeType = DECODE_TYPE_NORMAL);
+ /* Create a new DecodeRequest suitable for doing some decoding and set it
+ * as aImg's mDecodeRequest. */
+ void CreateRequestForImage(RasterImage* aImg);
+
private: /* members */
LinkedList<DecodeRequest> mASAPDecodeRequests;
LinkedList<DecodeRequest> mNormalDecodeRequests;
/* True if we've posted ourselves to the event loop and expect Run() to
* be called sometime in the future. */
bool mPendingInEventLoop;
@@ -662,17 +669,17 @@ private: // data
// Source data members
FallibleTArray<char> mSourceData;
nsCString mSourceDataMimeType;
friend class DiscardTracker;
// Decoder and friends
nsRefPtr<Decoder> mDecoder;
- DecodeRequest mDecodeRequest;
+ nsRefPtr<DecodeRequest> mDecodeRequest;
uint32_t mBytesDecoded;
// How many times we've decoded this image.
// This is currently only used for statistics
int32_t mDecodeCount;
// Cached value for GetImageContainer.
nsRefPtr<mozilla::layers::ImageContainer> mImageContainer;