[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor-browser/esr24] Bug 940714 - Add a RAII class to make synchronous decoding safer. r=tn, a=abillings
commit 164ccdf7911a3d54b8d14643539e01a9da7a680c
Author: Seth Fowler <seth@xxxxxxxxxxx>
Date: Wed Nov 20 17:21:51 2013 -0800
Bug 940714 - Add a RAII class to make synchronous decoding safer. r=tn, a=abillings
---
image/src/Decoder.h | 44 +++++++++++++++++++++++++++++++++++++++-----
image/src/RasterImage.cpp | 40 +++++++++++++++++-----------------------
image/src/RasterImage.h | 2 +-
3 files changed, 57 insertions(+), 29 deletions(-)
diff --git a/image/src/Decoder.h b/image/src/Decoder.h
index 463e473..705cb36 100644
--- a/image/src/Decoder.h
+++ b/image/src/Decoder.h
@@ -92,11 +92,6 @@ public:
mSizeDecode = aSizeDecode;
}
- void SetSynchronous(bool aSynchronous)
- {
- mSynchronous = aSynchronous;
- }
-
bool IsSynchronous() const
{
return mSynchronous;
@@ -242,6 +237,17 @@ protected:
bool mDataError;
private:
+ // Decode in synchronous mode. This is unsafe off-main-thread since it may
+ // attempt to allocate frames. To ensure that we never accidentally leave the
+ // decoder in synchronous mode, this should only be called by
+ // AutoSetSyncDecode.
+ void SetSynchronous(bool aSynchronous)
+ {
+ mSynchronous = aSynchronous;
+ }
+
+ friend class AutoSetSyncDecode;
+
uint32_t mFrameCount; // Number of frames, including anything in-progress
nsIntRect mInvalidRect; // Tracks an invalidation region in the current frame.
@@ -281,6 +287,34 @@ private:
bool mSynchronous;
};
+// A RAII helper class to automatically pair a call to SetSynchronous(true)
+// with a call to SetSynchronous(false), since failing to do so can lead us
+// to try to allocate frames off-main-thread, which is unsafe. Synchronous
+// decoding may only happen within the scope of an AutoSetSyncDecode. Nested
+// AutoSetSyncDecode's are OK.
+class AutoSetSyncDecode
+{
+public:
+ AutoSetSyncDecode(Decoder* aDecoder)
+ : mDecoder(aDecoder)
+ {
+ MOZ_ASSERT(NS_IsMainThread());
+ MOZ_ASSERT(mDecoder);
+
+ mOriginalValue = mDecoder->IsSynchronous();
+ mDecoder->SetSynchronous(true);
+ }
+
+ ~AutoSetSyncDecode()
+ {
+ mDecoder->SetSynchronous(mOriginalValue);
+ }
+
+private:
+ nsRefPtr<Decoder> mDecoder;
+ bool mOriginalValue;
+};
+
} // namespace image
} // namespace mozilla
diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp
index 695df7b..65f8169 100644
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -1713,10 +1713,11 @@ RasterImage::AddSourceData(const char *aBuffer, uint32_t aCount)
// write the data directly to the decoder. (If we haven't gotten the size,
// we'll queue up the data and write it out when we do.)
if (!StoringSourceData() && mHasSize) {
- mDecoder->SetSynchronous(true);
- rv = WriteToDecoder(aBuffer, aCount);
- mDecoder->SetSynchronous(false);
- CONTAINER_ENSURE_SUCCESS(rv);
+ {
+ AutoSetSyncDecode syncDecode(mDecoder);
+ rv = WriteToDecoder(aBuffer, aCount);
+ CONTAINER_ENSURE_SUCCESS(rv);
+ }
// We're not storing source data, so this data is probably coming straight
// from the network. In this case, we want to display data as soon as we
@@ -2084,7 +2085,7 @@ RasterImage::StoringSourceData() const {
// Sets up a decoder for this image. It is an error to call this function
// when decoding is already in process (ie - when mDecoder is non-null).
nsresult
-RasterImage::InitDecoder(bool aDoSizeDecode, bool aIsSynchronous /* = false */)
+RasterImage::InitDecoder(bool aDoSizeDecode)
{
// Ensure that the decoder is not already initialized
NS_ABORT_IF_FALSE(!mDecoder, "Calling InitDecoder() while already decoding!");
@@ -2152,7 +2153,6 @@ RasterImage::InitDecoder(bool aDoSizeDecode, bool aIsSynchronous /* = false */)
mDecoder->SetObserver(mDecodeRequest->mStatusTracker->GetDecoderObserver());
mDecoder->SetSizeDecode(aDoSizeDecode);
mDecoder->SetDecodeFlags(mFrameDecodeFlags);
- mDecoder->SetSynchronous(aIsSynchronous);
if (!aDoSizeDecode) {
// We already have the size; tell the decoder so it can preallocate a
// frame. By default, we create an ARGB frame with no offset. If decoders
@@ -2435,14 +2435,8 @@ RasterImage::RequestDecodeCore(RequestDecodeType aDecodeType)
// to finish decoding.
if (!mDecoded && !mInDecoder && mHasSourceData && aDecodeType == SYNCHRONOUS_NOTIFY_AND_SOME_DECODE) {
PROFILER_LABEL_PRINTF("RasterImage", "DecodeABitOf", "%s", GetURIString().get());
- mDecoder->SetSynchronous(true);
-
+ AutoSetSyncDecode syncDecode(mDecoder);
DecodePool::Singleton()->DecodeABitOf(this);
-
- // DecodeABitOf can destroy mDecoder.
- if (mDecoder) {
- mDecoder->SetSynchronous(false);
- }
return NS_OK;
}
@@ -2520,15 +2514,17 @@ RasterImage::SyncDecode()
// If we don't have a decoder, create one
if (!mDecoder) {
- rv = InitDecoder(/* aDoSizeDecode = */ false, /* aIsSynchronous = */ true);
+ rv = InitDecoder(/* aDoSizeDecode = */ false);
CONTAINER_ENSURE_SUCCESS(rv);
- } else {
- mDecoder->SetSynchronous(true);
}
- // Write everything we have
- rv = DecodeSomeData(mSourceData.Length() - mBytesDecoded);
- CONTAINER_ENSURE_SUCCESS(rv);
+ {
+ AutoSetSyncDecode syncDecode(mDecoder);
+
+ // Write everything we have
+ rv = DecodeSomeData(mSourceData.Length() - mBytesDecoded);
+ CONTAINER_ENSURE_SUCCESS(rv);
+ }
// When we're doing a sync decode, we want to get as much information from the
// image as possible. We've send the decoder all of our data, so now's a good
@@ -2541,11 +2537,9 @@ RasterImage::SyncDecode()
rv = FinishedSomeDecoding();
CONTAINER_ENSURE_SUCCESS(rv);
-
+
+ // If our decoder's still open, there's still work to be done.
if (mDecoder) {
- mDecoder->SetSynchronous(false);
-
- // If our decoder's still open, there's still work to be done.
DecodePool::Singleton()->RequestDecode(this);
}
diff --git a/image/src/RasterImage.h b/image/src/RasterImage.h
index cb51c68..420a470 100644
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -750,7 +750,7 @@ private: // data
// Decoding
nsresult WantDecodedFrames();
nsresult SyncDecode();
- nsresult InitDecoder(bool aDoSizeDecode, bool aIsSynchronous = false);
+ nsresult InitDecoder(bool aDoSizeDecode);
nsresult WriteToDecoder(const char *aBuffer, uint32_t aCount);
nsresult DecodeSomeData(uint32_t aMaxBytes);
bool IsDecodeFinished();
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits