From 6a9ab9bb484f403ca0e1053137e204bfd358dbb5 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Tue, 2 Aug 2016 16:45:22 -0700 Subject: [PATCH] Bug 1291071 (Part 4) - Clean up Decoder::SpeedHistogram() and related code. r=edwin --- image/Decoder.cpp | 9 +-------- image/Decoder.h | 19 +++++++++++++------ image/RasterImage.cpp | 7 ++----- image/decoders/nsGIFDecoder2.cpp | 6 +++--- image/decoders/nsGIFDecoder2.h | 4 +++- image/decoders/nsJPEGDecoder.cpp | 6 +++--- image/decoders/nsJPEGDecoder.h | 6 ++++-- image/decoders/nsPNGDecoder.cpp | 6 +++--- image/decoders/nsPNGDecoder.h | 8 +++++--- 9 files changed, 37 insertions(+), 34 deletions(-) diff --git a/image/Decoder.cpp b/image/Decoder.cpp index 94f9be3923c99..24b58fe83b3b7 100644 --- a/image/Decoder.cpp +++ b/image/Decoder.cpp @@ -268,7 +268,7 @@ Decoder::TakeCompleteFrameCount() } DecoderTelemetry -Decoder::Telemetry() +Decoder::Telemetry() const { MOZ_ASSERT(mIterator); return DecoderTelemetry(SpeedHistogram(), @@ -543,12 +543,5 @@ Decoder::PostError() } } -Telemetry::ID -Decoder::SpeedHistogram() -{ - // Use HistogramCount as an invalid Histogram ID. - return Telemetry::HistogramCount; -} - } // namespace image } // namespace mozilla diff --git a/image/Decoder.h b/image/Decoder.h index c10d0596e63b2..89f82b1e79135 100644 --- a/image/Decoder.h +++ b/image/Decoder.h @@ -30,7 +30,7 @@ namespace image { struct DecoderTelemetry final { - DecoderTelemetry(Telemetry::ID aSpeedHistogram, + DecoderTelemetry(Maybe aSpeedHistogram, size_t aBytesDecoded, uint32_t aChunkCount, TimeDuration aDecodeTime) @@ -49,8 +49,9 @@ struct DecoderTelemetry final /// @return our decoder's decode time, in microseconds. int32_t DecodeTimeMicros() { return mDecodeTime.ToMicroseconds(); } - /// The per-image-format telemetry ID for recording our decoder's speed. - const Telemetry::ID mSpeedHistogram; + /// The per-image-format telemetry ID for recording our decoder's speed, or + /// Nothing() if we don't record speed telemetry for this kind of decoder. + const Maybe mSpeedHistogram; /// The number of bytes of input our decoder processed. const size_t mBytesDecoded; @@ -336,13 +337,11 @@ class Decoder return gfx::IntRect(gfx::IntPoint(), OutputSize()); } - virtual Telemetry::ID SpeedHistogram(); - /// @return the metadata we collected about this image while decoding. const ImageMetadata& GetImageMetadata() { return mImageMetadata; } /// @return performance telemetry we collected while decoding. - DecoderTelemetry Telemetry(); + DecoderTelemetry Telemetry() const; /** * @return a weak pointer to the image associated with this decoder. Illegal @@ -387,6 +386,14 @@ class Decoder virtual nsresult FinishInternal(); virtual nsresult FinishWithErrorInternal(); + /** + * @return the per-image-format telemetry ID for recording this decoder's + * speed, or Nothing() if we don't record speed telemetry for this kind of + * decoder. + */ + virtual Maybe SpeedHistogram() const { return Nothing(); } + + /* * Progress notifications. */ diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 92a6b90ae6bff..857c93c54aff0 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -1619,11 +1619,8 @@ RasterImage::FinalizeDecoder(Decoder* aDecoder, Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME, int32_t(aTelemetry.mDecodeTime.ToMicroseconds())); - // We record the speed for only some decoders. The rest have - // SpeedHistogram return HistogramCount. - Telemetry::ID id = aTelemetry.mSpeedHistogram; - if (id < Telemetry::HistogramCount) { - Telemetry::Accumulate(id, aTelemetry.Speed()); + if (aTelemetry.mSpeedHistogram) { + Telemetry::Accumulate(*aTelemetry.mSpeedHistogram, aTelemetry.Speed()); } } diff --git a/image/decoders/nsGIFDecoder2.cpp b/image/decoders/nsGIFDecoder2.cpp index 8f28c4cf870b1..ea5f1023154b3 100644 --- a/image/decoders/nsGIFDecoder2.cpp +++ b/image/decoders/nsGIFDecoder2.cpp @@ -1073,10 +1073,10 @@ nsGIFDecoder2::SkipSubBlocks(const char* aData) nextSubBlockLength); } -Telemetry::ID -nsGIFDecoder2::SpeedHistogram() +Maybe +nsGIFDecoder2::SpeedHistogram() const { - return Telemetry::IMAGE_DECODE_SPEED_GIF; + return Some(Telemetry::IMAGE_DECODE_SPEED_GIF); } } // namespace image diff --git a/image/decoders/nsGIFDecoder2.h b/image/decoders/nsGIFDecoder2.h index 925c574a402e2..c903ce890af03 100644 --- a/image/decoders/nsGIFDecoder2.h +++ b/image/decoders/nsGIFDecoder2.h @@ -24,10 +24,12 @@ class nsGIFDecoder2 : public Decoder public: ~nsGIFDecoder2(); +protected: LexerResult DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume) override; nsresult FinishInternal() override; - virtual Telemetry::ID SpeedHistogram() override; + + Maybe SpeedHistogram() const override; private: friend class DecoderFactory; diff --git a/image/decoders/nsJPEGDecoder.cpp b/image/decoders/nsJPEGDecoder.cpp index 071d8859e085a..05f69934f3f18 100644 --- a/image/decoders/nsJPEGDecoder.cpp +++ b/image/decoders/nsJPEGDecoder.cpp @@ -123,10 +123,10 @@ nsJPEGDecoder::~nsJPEGDecoder() this)); } -Telemetry::ID -nsJPEGDecoder::SpeedHistogram() +Maybe +nsJPEGDecoder::SpeedHistogram() const { - return Telemetry::IMAGE_DECODE_SPEED_JPEG; + return Some(Telemetry::IMAGE_DECODE_SPEED_JPEG); } nsresult diff --git a/image/decoders/nsJPEGDecoder.h b/image/decoders/nsJPEGDecoder.h index fb8262574a299..260e9130317e9 100644 --- a/image/decoders/nsJPEGDecoder.h +++ b/image/decoders/nsJPEGDecoder.h @@ -57,13 +57,15 @@ class nsJPEGDecoder : public Decoder mSampleSize = aSampleSize; } + void NotifyDone(); + +protected: nsresult InitInternal() override; LexerResult DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume) override; nsresult FinishInternal() override; - virtual Telemetry::ID SpeedHistogram() override; - void NotifyDone(); + Maybe SpeedHistogram() const override; protected: Orientation ReadOrientationFromEXIF(); diff --git a/image/decoders/nsPNGDecoder.cpp b/image/decoders/nsPNGDecoder.cpp index a249c45571872..adc0d7175f2cf 100644 --- a/image/decoders/nsPNGDecoder.cpp +++ b/image/decoders/nsPNGDecoder.cpp @@ -1034,10 +1034,10 @@ nsPNGDecoder::warning_callback(png_structp png_ptr, png_const_charp warning_msg) MOZ_LOG(sPNGLog, LogLevel::Warning, ("libpng warning: %s\n", warning_msg)); } -Telemetry::ID -nsPNGDecoder::SpeedHistogram() +Maybe +nsPNGDecoder::SpeedHistogram() const { - return Telemetry::IMAGE_DECODE_SPEED_PNG; + return Some(Telemetry::IMAGE_DECODE_SPEED_PNG); } bool diff --git a/image/decoders/nsPNGDecoder.h b/image/decoders/nsPNGDecoder.h index 51e73e0a9a788..4b5c50ac505d2 100644 --- a/image/decoders/nsPNGDecoder.h +++ b/image/decoders/nsPNGDecoder.h @@ -22,13 +22,15 @@ class nsPNGDecoder : public Decoder public: virtual ~nsPNGDecoder(); + /// @return true if this PNG is a valid ICO resource. + bool IsValidICO() const; + +protected: nsresult InitInternal() override; LexerResult DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume) override; - virtual Telemetry::ID SpeedHistogram() override; - /// @return true if this PNG is a valid ICO resource. - bool IsValidICO() const; + Maybe SpeedHistogram() const override; private: friend class DecoderFactory;