Bug 1291071 (Part 4) - Clean up Decoder::SpeedHistogram() and related code. r=edwin

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

image/Decoder.h

@@ -30,7 +30,7 @@ namespace image {
 
 struct DecoderTelemetry final
 {
-  DecoderTelemetry(Telemetry::ID aSpeedHistogram,
+  DecoderTelemetry(Maybe<Telemetry::ID> 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<Telemetry::ID> mSpeedHistogram;
 
   /// The number of bytes of input our decoder processed.
   const size_t mBytesDecoded;
@@ -336,13 +337,11 @@ public:
     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 @@ protected:
   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<Telemetry::ID> SpeedHistogram() const { return Nothing(); }
+
+
   /*
    * Progress notifications.
    */

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());
       }
     }
 

image/decoders/nsGIFDecoder2.cpp

@@ -1073,10 +1073,10 @@ nsGIFDecoder2::SkipSubBlocks(const char* aData)
                                   nextSubBlockLength);
 }
 
-Telemetry::ID
-nsGIFDecoder2::SpeedHistogram()
+Maybe<Telemetry::ID>
+nsGIFDecoder2::SpeedHistogram() const
 {
-  return Telemetry::IMAGE_DECODE_SPEED_GIF;
+  return Some(Telemetry::IMAGE_DECODE_SPEED_GIF);
 }
 
 } // namespace image

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<Telemetry::ID> SpeedHistogram() const override;
 
 private:
   friend class DecoderFactory;

image/decoders/nsJPEGDecoder.cpp

@@ -123,10 +123,10 @@ nsJPEGDecoder::~nsJPEGDecoder()
           this));
 }
 
-Telemetry::ID
-nsJPEGDecoder::SpeedHistogram()
+Maybe<Telemetry::ID>
+nsJPEGDecoder::SpeedHistogram() const
 {
-  return Telemetry::IMAGE_DECODE_SPEED_JPEG;
+  return Some(Telemetry::IMAGE_DECODE_SPEED_JPEG);
 }
 
 nsresult

image/decoders/nsJPEGDecoder.h

@@ -57,13 +57,15 @@ public:
     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<Telemetry::ID> SpeedHistogram() const override;
 
 protected:
   Orientation ReadOrientationFromEXIF();

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<Telemetry::ID>
+nsPNGDecoder::SpeedHistogram() const
 {
-  return Telemetry::IMAGE_DECODE_SPEED_PNG;
+  return Some(Telemetry::IMAGE_DECODE_SPEED_PNG);
 }
 
 bool

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<Telemetry::ID> SpeedHistogram() const override;
 
 private:
   friend class DecoderFactory;