Add UMAs to collect EDID colorimetry failures.

This CL adds two metrics in drm_util.cc:

- DrmUtil.CreateDisplaySnapshot.HasEdidBlob (boolean): to detect whether an EDID
blob is detected in the CreateDisplaySnapshot function. There are no
existing unit tests for this function, so no tests are added for this metric.

- DrmUtil.GetColorSpaceFromEdid.ChecksOutcome (histogram): to detect
whether the sanity checks pass in the GetColorSpaceFromEdid function (or
which one fails). Existing unit tests are modified to test this metric.

Also, some unused includes are removed from drm_util_unittest.cc.

Bug: 829506
Change-Id: Ib1cfc14577630e6649640da9b0a0b663a36296d5
Reviewed-on: https://chromium-review.googlesource.com/1014522
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553773}

tools/metrics/histograms/enums.xml

@@ -11134,6 +11134,15 @@ Called by update_net_error_codes.py.-->
   <int value="29" label="X25519"/>
 </enum>
 
+<enum name="EdidColorSpaceChecksOutcome">
+  <int value="0" label="All checks passed"/>
+  <int value="1" label="Bad primaries coordinates"/>
+  <int value="2" label="Area of primaries' triangle too small"/>
+  <int value="3" label="Blue primary is broken"/>
+  <int value="4" label="Cannot extract toXYZD50 matrix"/>
+  <int value="5" label="Bad gamma value"/>
+</enum>
+
 <enum name="EGLDisplayType">
   <int value="0" label="Default"/>
   <int value="1" label="SwiftShader"/>

tools/metrics/histograms/histograms.xml

@@ -20065,6 +20065,26 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="DrmUtil.CreateDisplaySnapshot.HasEdidBlob" enum="Boolean">
+  <owner>andrescj@chromium.org</owner>
+  <owner>mcasas@chromium.org</owner>
+  <summary>
+    Whether an EDID blob was detected. This UMA is recorded whenever we attempt
+    to parse the EDID from a display.
+  </summary>
+</histogram>
+
+<histogram name="DrmUtil.GetColorSpaceFromEdid.ChecksOutcome"
+    enum="EdidColorSpaceChecksOutcome">
+  <owner>andrescj@chromium.org</owner>
+  <owner>mcasas@chromium.org</owner>
+  <summary>
+    When attempting to get the color space from an EDID blob, whether the sanity
+    checks passed (and if not, which check failed). This UMA is recorded
+    whenever the color space is extracted from an EDID blob.
+  </summary>
+</histogram>
+
 <histogram name="EasyUnlock.AuthenticationSuccess" enum="BooleanSuccess">
   <obsolete>
     Deprecated as of 03/2018.

ui/ozone/platform/drm/common/drm_util.cc

@@ -14,6 +14,7 @@
 #include <utility>
 
 #include "base/containers/flat_map.h"
+#include "base/metrics/histogram_macros.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/display/types/display_mode.h"
 #include "ui/display/util/edid_parser.h"
@@ -25,6 +26,13 @@ namespace {
 static const size_t kDefaultCursorWidth = 64;
 static const size_t kDefaultCursorHeight = 64;
 
+// Used in the GetColorSpaceFromEdid function to collect data on whether the
+// color space extracted from an EDID blob passed the sanity checks.
+void EmitEdidColorSpaceChecksOutcomeUma(EdidColorSpaceChecksOutcome outcome) {
+  UMA_HISTOGRAM_ENUMERATION("DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+                            outcome);
+}
+
 bool IsCrtcInUse(
     uint32_t crtc,
     const std::vector<std::unique_ptr<HardwareDisplayControllerInfo>>&
@@ -402,6 +410,8 @@ std::unique_ptr<display::DisplaySnapshot> CreateDisplaySnapshot(
 
   ScopedDrmPropertyBlobPtr edid_blob(
       GetDrmPropertyBlob(fd, info->connector(), "EDID"));
+  UMA_HISTOGRAM_BOOLEAN("DrmUtil.CreateDisplaySnapshot.HasEdidBlob",
+                        !!edid_blob);
   std::vector<uint8_t> edid;
   if (edid_blob) {
     edid.assign(static_cast<uint8_t*>(edid_blob->data),
@@ -642,6 +652,8 @@ gfx::ColorSpace GetColorSpaceFromEdid(const display::EdidParser& edid_parser) {
   // Rx, to guarantee that the R, G and B colors are each in the correct region.
   if (!(primaries.fBX <= primaries.fRX && primaries.fGX <= primaries.fRX &&
         primaries.fBY <= primaries.fRY && primaries.fRY <= primaries.fGY)) {
+    EmitEdidColorSpaceChecksOutcomeUma(
+        EdidColorSpaceChecksOutcome::kErrorBadCoordinates);
     return gfx::ColorSpace();
   }
 
@@ -652,8 +664,11 @@ gfx::ColorSpace GetColorSpaceFromEdid(const display::EdidParser& edid_parser) {
       (primaries.fRX * primaries.fGY) + (primaries.fBX * primaries.fRY) +
       (primaries.fGX * primaries.fBY) - (primaries.fBX * primaries.fGY) -
       (primaries.fGX * primaries.fRY) - (primaries.fRX * primaries.fBY);
-  if (primaries_area_twice < kBT709PrimariesArea)
+  if (primaries_area_twice < kBT709PrimariesArea) {
+    EmitEdidColorSpaceChecksOutcomeUma(
+        EdidColorSpaceChecksOutcome::kErrorPrimariesAreaTooSmall);
     return gfx::ColorSpace();
+  }
 
   // Sanity check: https://crbug.com/809909, the blue primary coordinates should
   // not be too far left/upwards of the expected location (namely [0.15, 0.06]
@@ -665,18 +680,28 @@ gfx::ColorSpace GetColorSpaceFromEdid(const display::EdidParser& edid_parser) {
   const bool is_blue_primary_broken =
       (std::abs(primaries.fBX - kExpectedBluePrimaryX) > kBluePrimaryXDelta) ||
       (std::abs(primaries.fBY - kExpectedBluePrimaryY) > kBluePrimaryYDelta);
-  if (is_blue_primary_broken)
+  if (is_blue_primary_broken) {
+    EmitEdidColorSpaceChecksOutcomeUma(
+        EdidColorSpaceChecksOutcome::kErrorBluePrimaryIsBroken);
     return gfx::ColorSpace();
+  }
 
   SkMatrix44 color_space_as_matrix;
-  if (!primaries.toXYZD50(&color_space_as_matrix))
+  if (!primaries.toXYZD50(&color_space_as_matrix)) {
+    EmitEdidColorSpaceChecksOutcomeUma(
+        EdidColorSpaceChecksOutcome::kErrorCannotExtractToXYZD50);
     return gfx::ColorSpace();
+  }
 
   const double gamma = edid_parser.gamma();
-  if (gamma < 1.0)
+  if (gamma < 1.0) {
+    EmitEdidColorSpaceChecksOutcomeUma(
+        EdidColorSpaceChecksOutcome::kErrorBadGamma);
     return gfx::ColorSpace();
+  }
 
   SkColorSpaceTransferFn transfer = {gamma, 1.f, 0.f, 0.f, 0.f, 0.f, 0.f};
+  EmitEdidColorSpaceChecksOutcomeUma(EdidColorSpaceChecksOutcome::kSuccess);
   return gfx::ColorSpace::CreateCustom(color_space_as_matrix, transfer);
 }
 

ui/ozone/platform/drm/common/drm_util.h

@@ -30,6 +30,18 @@ class Point;
 
 namespace ui {
 
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class EdidColorSpaceChecksOutcome {
+  kSuccess = 0,
+  kErrorBadCoordinates = 1,
+  kErrorPrimariesAreaTooSmall = 2,
+  kErrorBluePrimaryIsBroken = 3,
+  kErrorCannotExtractToXYZD50 = 4,
+  kErrorBadGamma = 5,
+  kMaxValue = kErrorBadGamma
+};
+
 // Representation of the information required to initialize and configure a
 // native display. |index| is the position of the connection and will be
 // used to generate a unique identifier for the display.

ui/ozone/platform/drm/common/drm_util_unittest.cc

@@ -8,9 +8,8 @@
 #include <xf86drmMode.h>
 
 #include <map>
-#include <string>
 
-#include "base/numerics/ranges.h"
+#include "base/test/histogram_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkColorSpace.h"
 #include "third_party/skia/include/core/SkMatrix44.h"
@@ -310,6 +309,8 @@ TEST_F(DrmUtilTest, OverlaySurfaceCandidate) {
 }
 
 TEST_F(DrmUtilTest, GetColorSpaceFromEdid) {
+  base::HistogramTester histogram_tester;
+
   // Test with HP z32x monitor.
   constexpr SkColorSpacePrimaries expected_hpz32x_primaries = {
       .fRX = 0.673828f,
@@ -330,6 +331,11 @@ TEST_F(DrmUtilTest, GetColorSpaceFromEdid) {
           SkColorSpaceTransferFn({2.2, 1, 0, 0, 0, 0, 0}));
   EXPECT_EQ(expected_hpz32x_color_space.ToString(),
             GetColorSpaceFromEdid(display::EdidParser(hpz32x_edid)).ToString());
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kSuccess),
+      1);
 
   // Test with Chromebook Samus internal display.
   constexpr SkColorSpacePrimaries expected_samus_primaries = {.fRX = 0.633789f,
@@ -349,6 +355,11 @@ TEST_F(DrmUtilTest, GetColorSpaceFromEdid) {
           SkColorSpaceTransferFn({2.5, 1, 0, 0, 0, 0, 0}));
   EXPECT_EQ(expected_samus_color_space.ToString(),
             GetColorSpaceFromEdid(display::EdidParser(samus_edid)).ToString());
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kSuccess),
+      2);
 
   // Test with Chromebook Eve internal display.
   constexpr SkColorSpacePrimaries expected_eve_primaries = {.fRX = 0.639648f,
@@ -368,6 +379,11 @@ TEST_F(DrmUtilTest, GetColorSpaceFromEdid) {
           SkColorSpaceTransferFn({2.2, 1, 0, 0, 0, 0, 0}));
   EXPECT_EQ(expected_eve_color_space.ToString(),
             GetColorSpaceFromEdid(display::EdidParser(eve_edid)).ToString());
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kSuccess),
+      3);
 
   // Test with gamma marked as non-existent.
   const std::vector<uint8_t> no_gamma_edid(
@@ -375,24 +391,47 @@ TEST_F(DrmUtilTest, GetColorSpaceFromEdid) {
   const gfx::ColorSpace no_gamma_color_space =
       GetColorSpaceFromEdid(display::EdidParser(no_gamma_edid));
   EXPECT_FALSE(no_gamma_color_space.IsValid());
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kErrorBadGamma),
+      1);
+  histogram_tester.ExpectTotalCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome", 4);
 }
 
 TEST_F(DrmUtilTest, GetInvalidColorSpaceFromEdid) {
+  base::HistogramTester histogram_tester;
   const std::vector<uint8_t> empty_edid;
   EXPECT_EQ(gfx::ColorSpace(),
             GetColorSpaceFromEdid(display::EdidParser(empty_edid)));
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kErrorPrimariesAreaTooSmall),
+      1);
 
   const std::vector<uint8_t> invalid_edid(
       kInvalidEdid, kInvalidEdid + arraysize(kInvalidEdid) - 1);
   const gfx::ColorSpace invalid_color_space =
       GetColorSpaceFromEdid(display::EdidParser(invalid_edid));
   EXPECT_FALSE(invalid_color_space.IsValid());
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kErrorPrimariesAreaTooSmall),
+      2);
 
   const std::vector<uint8_t> sst210_edid(kSST210,
                                          kSST210 + arraysize(kSST210) - 1);
   const gfx::ColorSpace sst210_color_space =
       GetColorSpaceFromEdid(display::EdidParser(sst210_edid));
   EXPECT_FALSE(sst210_color_space.IsValid()) << sst210_color_space.ToString();
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kErrorBadCoordinates),
+      1);
 
   const std::vector<uint8_t> sst210_edid_2(
       kSST210Corrected, kSST210Corrected + arraysize(kSST210Corrected) - 1);
@@ -400,6 +439,11 @@ TEST_F(DrmUtilTest, GetInvalidColorSpaceFromEdid) {
       GetColorSpaceFromEdid(display::EdidParser(sst210_edid_2));
   EXPECT_FALSE(sst210_color_space_2.IsValid())
       << sst210_color_space_2.ToString();
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kErrorPrimariesAreaTooSmall),
+      3);
 
   const std::vector<uint8_t> broken_blue_edid(
       kBrokenBluePrimaries,
@@ -408,6 +452,13 @@ TEST_F(DrmUtilTest, GetInvalidColorSpaceFromEdid) {
       GetColorSpaceFromEdid(display::EdidParser(broken_blue_edid));
   EXPECT_FALSE(broken_blue_color_space.IsValid())
       << broken_blue_color_space.ToString();
+  histogram_tester.ExpectBucketCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
+      static_cast<base::HistogramBase::Sample>(
+          EdidColorSpaceChecksOutcome::kErrorBluePrimaryIsBroken),
+      1);
+  histogram_tester.ExpectTotalCount(
+      "DrmUtil.GetColorSpaceFromEdid.ChecksOutcome", 5);
 }
 
 TEST_F(DrmUtilTest, TestDisplayModesExtraction) {