[blink] Use //base/metrics for recording enum metrics for bitmap images.

Miscellaneous fixes:

- Simplify the unit tests by unparameterizing them. Instead, the tests
  simply iterate over an array of test cases, using SCOPED_TRACE() to
  make it easier to debug failures.
- Convert metrics enums to be a scoped enumeration, which allows:
  - clang to enforce kMaxValue correctness
  - autodeduction of the max value by UMA_HISTOGRAM_ENUMERATION()
- Remove the unused Gamma enum. The metric itself was removed in
  https://crrev.com/c/1684563.

Bug: 742517, 1047547
Change-Id: I0a5b4aeaa8540505a7b0596a4d0df6f871fef8b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527341Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826393}

third_party/blink/renderer/core/imagebitmap/image_bitmap.cc

@@ -978,7 +978,7 @@ void ImageBitmap::RasterizeImageOnBackgroundThread(
   PostCrossThreadTask(
       *task_runner, FROM_HERE,
       CrossThreadBindOnce(std::move(callback), std::move(skia_image),
-                          kDefaultImageOrientation));
+                          ImageOrientationEnum::kDefault));
 }
 
 ScriptPromise ImageBitmap::CreateAsync(ImageElementBase* image,

third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc

@@ -309,7 +309,7 @@ void DecodeImageOnDecoderThread(
       data_complete, alpha_option, ImageDecoder::kDefaultBitDepth,
       color_behavior);
   sk_sp<SkImage> frame;
-  ImageOrientationEnum orientation = kDefaultImageOrientation;
+  ImageOrientationEnum orientation = ImageOrientationEnum::kDefault;
   if (decoder) {
     orientation = decoder->Orientation().Orientation();
     frame = ImageBitmap::GetSkImageFromDecoder(std::move(decoder));

third_party/blink/renderer/modules/webcodecs/image_decoder_external.cc

@@ -376,7 +376,8 @@ void ImageDecoderExternal::MaybeSatisfyPendingDecodes() {
         base::nullopt, options_));
     result->setDuration(
         decoder_->FrameDurationAtIndex(request->frame_index).InMicroseconds());
-    result->setOrientation(decoder_->Orientation().Orientation());
+    result->setOrientation(
+        static_cast<uint32_t>(decoder_->Orientation().Orientation()));
     result->setComplete(is_complete);
     request->result = result;
   }

third_party/blink/renderer/platform/graphics/accelerated_static_bitmap_image.cc

@@ -57,7 +57,7 @@ AcceleratedStaticBitmapImage::CreateFromCanvasMailbox(
     std::unique_ptr<viz::SingleReleaseCallback> release_callback) {
   return base::AdoptRef(new AcceleratedStaticBitmapImage(
       mailbox, sync_token, shared_image_texture_id, sk_image_info,
-      texture_target, is_origin_top_left, kDefaultImageOrientation,
+      texture_target, is_origin_top_left, ImageOrientationEnum::kDefault,
       std::move(context_provider_wrapper), context_thread_ref,
       std::move(context_task_runner), std::move(release_callback)));
 }

third_party/blink/renderer/platform/graphics/bitmap_image.cc

@@ -166,7 +166,7 @@ IntSize BitmapImage::PreferredDisplaySize() const {
 
 bool BitmapImage::HasDefaultOrientation() const {
   ImageOrientation orientation = CurrentFrameOrientation();
-  return orientation == kDefaultImageOrientation;
+  return orientation == ImageOrientationEnum::kDefault;
 }
 
 bool BitmapImage::GetHotSpot(IntPoint& hot_spot) const {
@@ -288,13 +288,13 @@ void BitmapImage::Draw(
   if (adjusted_src_rect.IsEmpty() || dst_rect.IsEmpty())
     return;  // Nothing to draw.
 
-  ImageOrientation orientation = kDefaultImageOrientation;
+  ImageOrientation orientation = ImageOrientationEnum::kDefault;
   if (should_respect_image_orientation == kRespectImageOrientation)
     orientation = CurrentFrameOrientation();
 
   PaintCanvasAutoRestore auto_restore(canvas, false);
   FloatRect adjusted_dst_rect = dst_rect;
-  if (orientation != kDefaultImageOrientation) {
+  if (orientation != ImageOrientationEnum::kDefault) {
     canvas->save();
 
     // ImageOrientation expects the origin to be at (0, 0)
@@ -349,9 +349,6 @@ bool BitmapImage::IsSizeAvailable() {
   if (size_available_ && HasVisibleImageSize(Size())) {
     BitmapImageMetrics::CountDecodedImageType(decoder_->FilenameExtension());
     if (decoder_->FilenameExtension() == "jpg") {
-      BitmapImageMetrics::CountImageOrientation(
-          decoder_->OrientationAtIndex(0).Orientation());
-
       IntSize correctedSize = decoder_->DensityCorrectedSizeAtIndex(0);
       BitmapImageMetrics::CountImageDensityCorrection(
         !correctedSize.IsEmpty() && correctedSize != decoder_->Size());
@@ -419,7 +416,7 @@ bool BitmapImage::CurrentFrameIsLazyDecoded() {
 
 ImageOrientation BitmapImage::CurrentFrameOrientation() const {
   return decoder_ ? decoder_->OrientationAtIndex(PaintImage::kDefaultFrameIndex)
-                  : kDefaultImageOrientation;
+                  : ImageOrientationEnum::kDefault;
 }
 
 IntSize BitmapImage::CurrentFrameDensityCorrectedSize() const {

third_party/blink/renderer/platform/graphics/bitmap_image_metrics.cc

@@ -19,35 +19,24 @@ namespace blink {
 void BitmapImageMetrics::CountDecodedImageType(const String& type) {
   DecodedImageType decoded_image_type =
       type == "jpg"
-          ? kImageJPEG
+          ? DecodedImageType::kJPEG
           : type == "png"
-                ? kImagePNG
+                ? DecodedImageType::kPNG
                 : type == "gif"
-                      ? kImageGIF
+                      ? DecodedImageType::kGIF
                       : type == "webp"
-                            ? kImageWebP
+                            ? DecodedImageType::kWebP
                             : type == "ico"
-                                  ? kImageICO
+                                  ? DecodedImageType::kICO
                                   : type == "bmp"
-                                        ? kImageBMP
+                                        ? DecodedImageType::kBMP
 #if BUILDFLAG(ENABLE_AV1_DECODER)
                                         : type == "avif"
-                                              ? kImageAVIF
+                                              ? DecodedImageType::kAVIF
 #endif
-                                              : DecodedImageType::kImageUnknown;
+                                              : DecodedImageType::kUnknown;
 
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(
-      EnumerationHistogram, decoded_image_type_histogram,
-      ("Blink.DecodedImageType", kDecodedImageTypeEnumEnd));
-  decoded_image_type_histogram.Count(decoded_image_type);
-}
-
-void BitmapImageMetrics::CountImageOrientation(
-    const ImageOrientationEnum orientation) {
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(
-      EnumerationHistogram, orientation_histogram,
-      ("Blink.DecodedImage.Orientation", kImageOrientationEnumEnd));
-  orientation_histogram.Count(orientation);
+  UMA_HISTOGRAM_ENUMERATION("Blink.DecodedImageType", decoded_image_type);
 }
 
 void BitmapImageMetrics::CountImageDensityCorrection(bool density_correction_present) {
@@ -90,10 +79,7 @@ void BitmapImageMetrics::CountJpegArea(const IntSize& size) {
 }
 
 void BitmapImageMetrics::CountJpegColorSpace(JpegColorSpace color_space) {
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(
-      EnumerationHistogram, color_space_histogram,
-      ("Blink.ImageDecoders.Jpeg.ColorSpace", JpegColorSpace::kMaxValue));
-  color_space_histogram.Count(color_space);
+  UMA_HISTOGRAM_ENUMERATION("Blink.ImageDecoders.Jpeg.ColorSpace", color_space);
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/bitmap_image_metrics.h

@@ -21,41 +21,23 @@ class PLATFORM_EXPORT BitmapImageMetrics {
   // Values synced with 'DecodedImageType' in
   // src/tools/metrics/histograms/enums.xml. These values are persisted to logs.
   // Entries should not be renumbered and numeric values should never be reused.
-  enum DecodedImageType {
-    kImageUnknown = 0,
-    kImageJPEG = 1,
-    kImagePNG = 2,
-    kImageGIF = 3,
-    kImageWebP = 4,
-    kImageICO = 5,
-    kImageBMP = 6,
-    kImageAVIF = 7,
-    kDecodedImageTypeEnumEnd = kImageAVIF + 1
-  };
-
-  // Values synced with 'Gamma' in src/tools/metrics/histograms/enums.xml. These
-  // values are persisted to logs. Entries should not be renumbered and numeric
-  // values should never be reused.
-  enum Gamma {
-    kGammaLinear = 0,
-    kGammaSRGB = 1,
-    kGamma2Dot2 = 2,
-    kGammaNonStandard = 3,
-    kGammaNull = 4,
-    kGammaFail = 5,
-    kGammaInvalid = 6,
-    kGammaExponent = 7,
-    kGammaTable = 8,
-    kGammaParametric = 9,
-    kGammaNamed = 10,
-    kGammaEnd = kGammaNamed + 1,
+  enum class DecodedImageType {
+    kUnknown = 0,
+    kJPEG = 1,
+    kPNG = 2,
+    kGIF = 3,
+    kWebP = 4,
+    kICO = 5,
+    kBMP = 6,
+    kAVIF = 7,
+    kMaxValue = kAVIF,
   };
 
   // Categories for the JPEG color space histogram. Synced with 'JpegColorSpace'
   // in src/tools/metrics/histograms/enums.xml. These values are persisted to
   // logs. Entries should not be renumbered and numeric values should never be
   // reused.
-  enum JpegColorSpace {
+  enum class JpegColorSpace {
     kUnknown = 0,
     kGrayscale = 1,
     kRGB = 2,
@@ -73,7 +55,6 @@ class PLATFORM_EXPORT BitmapImageMetrics {
 
   // |type| is the return value of ImageDecoder::FilenameExtension().
   static void CountDecodedImageType(const String& type);
-  static void CountImageOrientation(const ImageOrientationEnum);
   static void CountImageDensityCorrection(bool densityCorrectionPresent);
   // Report the JPEG compression density in 0.01 bits per pixel for an image
   // with a smallest side (width or length) of |image_min_side| and total size

third_party/blink/renderer/platform/graphics/canvas_resource_provider.h

@@ -136,7 +136,7 @@ class PLATFORM_EXPORT CanvasResourceProvider
   // should be derived from the source of the bitmap data.
   virtual scoped_refptr<CanvasResource> ProduceCanvasResource() = 0;
   virtual scoped_refptr<StaticBitmapImage> Snapshot(
-      const ImageOrientation& = kDefaultImageOrientation) = 0;
+      const ImageOrientation& = ImageOrientationEnum::kDefault) = 0;
 
   // WebGraphicsContext3DProvider::DestructionObserver implementation.
   void OnContextDestroyed() override;

third_party/blink/renderer/platform/graphics/deferred_image_decoder.cc

@@ -120,7 +120,7 @@ struct DeferredFrameData {
 
  public:
   DeferredFrameData()
-      : orientation_(kDefaultImageOrientation), is_received_(false) {}
+      : orientation_(ImageOrientationEnum::kDefault), is_received_(false) {}
 
   ImageOrientation orientation_;
   IntSize density_corrected_size_;
@@ -361,7 +361,7 @@ ImageOrientation DeferredImageDecoder::OrientationAtIndex(size_t index) const {
     return metadata_decoder_->Orientation();
   if (index < frame_data_.size())
     return frame_data_[index].orientation_;
-  return kDefaultImageOrientation;
+  return ImageOrientationEnum::kDefault;
 }
 
 IntSize DeferredImageDecoder::DensityCorrectedSizeAtIndex(size_t index) const {

third_party/blink/renderer/platform/graphics/image.cc

@@ -117,7 +117,7 @@ PaintImage Image::ResizeAndOrientImage(
   IntSize size(image.width(), image.height());
   size.Scale(image_scale.Width(), image_scale.Height());
   AffineTransform transform;
-  if (orientation != kDefaultImageOrientation) {
+  if (orientation != ImageOrientationEnum::kDefault) {
     if (orientation.UsesWidthAsHeight())
       size = size.TransposedSize();
     transform *= orientation.TransformFromDefault(FloatSize(size));
@@ -369,7 +369,7 @@ SkBitmap Image::AsSkBitmapForCurrentFrame(
     ImageOrientation orientation =
         respect_image_orientation == kRespectImageOrientation
             ? bitmap_image->CurrentFrameOrientation()
-            : kDefaultImageOrientation;
+            : ImageOrientationEnum::kDefault;
 
     FloatSize image_scale(1, 1);
     if (density_corrected_size != bitmap_image->Size()) {
@@ -402,7 +402,7 @@ DarkModeImageCache* Image::GetDarkModeImageCache() {
 FloatRect Image::CorrectSrcRectForImageOrientation(FloatSize image_size,
                                                    FloatRect src_rect) const {
   ImageOrientation orientation = CurrentFrameOrientation();
-  DCHECK(orientation != kDefaultImageOrientation);
+  DCHECK(orientation != ImageOrientationEnum::kDefault);
   AffineTransform forward_map = orientation.TransformFromDefault(image_size);
   AffineTransform inverse_map = forward_map.Inverse();
   return inverse_map.MapRect(src_rect);

third_party/blink/renderer/platform/graphics/image.h

@@ -212,7 +212,7 @@ class PLATFORM_EXPORT Image : public ThreadSafeRefCounted<Image> {
   // Most image types have the default orientation. Only bitmap derived image
   // types need to override this method.
   virtual ImageOrientation CurrentFrameOrientation() const {
-    return kDefaultImageOrientation;
+    return ImageOrientationEnum::kDefault;
   }
 
   virtual IntSize CurrentFrameDensityCorrectedSize() const { return IntSize(); }

third_party/blink/renderer/platform/graphics/image_orientation.cc

@@ -36,24 +36,22 @@ AffineTransform ImageOrientation::TransformFromDefault(
   float h = drawn_size.Height();
 
   switch (orientation_) {
-    case kOriginTopLeft:
+    case ImageOrientationEnum::kOriginTopLeft:
       return AffineTransform();
-    case kOriginTopRight:
+    case ImageOrientationEnum::kOriginTopRight:
       return AffineTransform(-1, 0, 0, 1, w, 0);
-    case kOriginBottomRight:
+    case ImageOrientationEnum::kOriginBottomRight:
       return AffineTransform(-1, 0, 0, -1, w, h);
-    case kOriginBottomLeft:
+    case ImageOrientationEnum::kOriginBottomLeft:
       return AffineTransform(1, 0, 0, -1, 0, h);
-    case kOriginLeftTop:
+    case ImageOrientationEnum::kOriginLeftTop:
       return AffineTransform(0, 1, 1, 0, 0, 0);
-    case kOriginRightTop:
+    case ImageOrientationEnum::kOriginRightTop:
       return AffineTransform(0, 1, -1, 0, w, 0);
-    case kOriginRightBottom:
+    case ImageOrientationEnum::kOriginRightBottom:
       return AffineTransform(0, -1, -1, 0, w, h);
-    case kOriginLeftBottom:
+    case ImageOrientationEnum::kOriginLeftBottom:
       return AffineTransform(0, -1, 1, 0, 0, h);
-    default:
-      NOTREACHED();
   }
 
   NOTREACHED();

third_party/blink/renderer/platform/graphics/image_orientation.h

@@ -26,6 +26,8 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_IMAGE_ORIENTATION_H_
 #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_IMAGE_ORIENTATION_H_
 
+#include <stdint.h>
+
 #include "third_party/blink/renderer/platform/platform_export.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 
@@ -36,7 +38,9 @@ class FloatSize;
 
 // This enum intentionally matches the orientation values from the EXIF spec.
 // See JEITA CP-3451, page 18. http://www.exif.org/Exif2-2.PDF
-enum ImageOrientationEnum {
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class ImageOrientationEnum : int8_t {
   // "TopLeft" means that the 0 row starts at the Top, the 0 column starts at
   // the Left.
   kOriginTopLeft = 1,      // default
@@ -48,8 +52,8 @@ enum ImageOrientationEnum {
   kOriginRightBottom = 7,  // mirror along x-axis + 90 degree CW rotation
   kOriginLeftBottom = 8,   // 270 degree CW rotation
   // All other values are "reserved" as of EXIF 2.2
-  kDefaultImageOrientation = kOriginTopLeft,
-  kImageOrientationEnumEnd = kOriginLeftBottom + 1,
+  kDefault = kOriginTopLeft,
+  kMaxValue = kOriginLeftBottom,
 };
 
 enum RespectImageOrientationEnum {
@@ -61,12 +65,13 @@ class PLATFORM_EXPORT ImageOrientation final {
   DISALLOW_NEW();
 
  public:
-  ImageOrientation(ImageOrientationEnum orientation = kDefaultImageOrientation)
+  ImageOrientation(
+      ImageOrientationEnum orientation = ImageOrientationEnum::kDefault)
       : orientation_(orientation) {}
 
   bool UsesWidthAsHeight() const {
     // Values 5 through 8 all flip the width/height.
-    return orientation_ >= kOriginLeftTop;
+    return orientation_ >= ImageOrientationEnum::kOriginLeftTop;
   }
 
   // ImageOrientationEnum currently matches EXIF values, however code outside
@@ -74,8 +79,9 @@ class PLATFORM_EXPORT ImageOrientation final {
   static ImageOrientation FromEXIFValue(int exif_value) {
     // Values direct from images may be invalid, in which case we use the
     // default.
-    if (exif_value < kOriginTopLeft || exif_value > kOriginLeftBottom)
-      return kDefaultImageOrientation;
+    if (exif_value < static_cast<int>(ImageOrientationEnum::kOriginTopLeft) ||
+        exif_value > static_cast<int>(ImageOrientationEnum::kOriginLeftBottom))
+      return ImageOrientationEnum::kDefault;
     return static_cast<ImageOrientationEnum>(exif_value);
   }
 
@@ -93,7 +99,6 @@ class PLATFORM_EXPORT ImageOrientation final {
   ImageOrientationEnum Orientation() const { return orientation_; }
 
  private:
-  // FIXME: This only needs to be one byte.
   ImageOrientationEnum orientation_;
 };
 

third_party/blink/renderer/platform/graphics/static_bitmap_image.cc

@@ -58,7 +58,7 @@ void StaticBitmapImage::DrawHelper(
 
   cc::PaintCanvasAutoRestore auto_restore(canvas, false);
   FloatRect adjusted_dst_rect = dst_rect;
-  if (respect_orientation && orientation_ != kDefaultImageOrientation) {
+  if (respect_orientation && orientation_ != ImageOrientationEnum::kDefault) {
     canvas->save();
 
     // ImageOrientation expects the origin to be at (0, 0)

third_party/blink/renderer/platform/graphics/static_bitmap_image.h

@@ -27,11 +27,11 @@ class PLATFORM_EXPORT StaticBitmapImage : public Image {
   // The ImageOrientation should be derived from the source of the image data.
   static scoped_refptr<StaticBitmapImage> Create(
       PaintImage,
-      ImageOrientation = kDefaultImageOrientation);
+      ImageOrientation = ImageOrientationEnum::kDefault);
   static scoped_refptr<StaticBitmapImage> Create(
       sk_sp<SkData> data,
       const SkImageInfo&,
-      ImageOrientation = kDefaultImageOrientation);
+      ImageOrientation = ImageOrientationEnum::kDefault);
 
   StaticBitmapImage(ImageOrientation orientation) : orientation_(orientation) {}
 
@@ -93,7 +93,7 @@ class PLATFORM_EXPORT StaticBitmapImage : public Image {
     return orientation_;
   }
   bool HasDefaultOrientation() const override {
-    return orientation_ == kDefaultImageOrientation;
+    return orientation_ == ImageOrientationEnum::kDefault;
   }
 
   static base::CheckedNumeric<size_t> GetSizeInBytes(
@@ -122,7 +122,7 @@ class PLATFORM_EXPORT StaticBitmapImage : public Image {
   // static image is created and the underlying representations do not store
   // the information. The property is set at construction based on the source of
   // the image data.
-  ImageOrientation orientation_ = kDefaultImageOrientation;
+  ImageOrientation orientation_ = ImageOrientationEnum::kDefault;
 
   // The following property is here because the SkImage API doesn't expose the
   // info. It is applied to both UnacceleratedStaticBitmapImage and

third_party/blink/renderer/platform/graphics/unaccelerated_static_bitmap_image.h

@@ -21,10 +21,10 @@ class PLATFORM_EXPORT UnacceleratedStaticBitmapImage final
   // The ImageOrientation should be derived from the source of the image data.
   static scoped_refptr<UnacceleratedStaticBitmapImage> Create(
       sk_sp<SkImage>,
-      ImageOrientation orientation = kDefaultImageOrientation);
+      ImageOrientation orientation = ImageOrientationEnum::kDefault);
   static scoped_refptr<UnacceleratedStaticBitmapImage> Create(
       PaintImage,
-      ImageOrientation orientation = kDefaultImageOrientation);
+      ImageOrientation orientation = ImageOrientationEnum::kDefault);
 
   bool CurrentFrameKnownToBeOpaque() override;
   IntSize Size() const override;

third_party/blink/renderer/platform/image-decoders/jpeg/jpeg_image_decoder_test.cc

@@ -34,13 +34,13 @@
 #include <memory>
 #include <string>
 
+#include "base/test/metrics/histogram_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/platform/web_data.h"
 #include "third_party/blink/public/platform/web_size.h"
 #include "third_party/blink/renderer/platform/graphics/bitmap_image_metrics.h"
 #include "third_party/blink/renderer/platform/image-decoders/image_animation.h"
 #include "third_party/blink/renderer/platform/image-decoders/image_decoder_test_helpers.h"
-#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
 #include "third_party/blink/renderer/platform/wtf/shared_buffer.h"
 
 namespace blink {
@@ -414,7 +414,7 @@ class ColorSpaceUMATest
 // Tests that the JPEG color space/subsampling is recorded correctly as a UMA
 // for a variety of images. When the decode fails, no UMA should be recorded.
 TEST_P(ColorSpaceUMATest, CorrectColorSpaceRecorded) {
-  HistogramTester histogram_tester;
+  base::HistogramTester histogram_tester;
   scoped_refptr<SharedBuffer> data =
       ReadFile(("/images/resources/" + GetParam().file).c_str());
   ASSERT_TRUE(data);

tools/metrics/histograms/histograms_xml/blink/histograms.xml

@@ -552,6 +552,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 
 <histogram name="Blink.DecodedImage.Orientation" enum="DecodedImageOrientation"
     expires_after="M81">
+  <obsolete>
+    Removed in M88.
+  </obsolete>
   <owner>andrescj@chromium.org</owner>
   <owner>rob.buis@samsung.org</owner>
   <summary>Image orientation inferred during decode.</summary>