From dd66965dd289d371293997cfead79a147bf2246d Mon Sep 17 00:00:00 2001 From: Jonathan Freed Date: Tue, 16 Oct 2018 23:25:56 +0000 Subject: [PATCH] [Explore Sites] Adding UMA to capture the result of the ImageDecoder service. Change-Id: I1a5cca85ac14fcec14a6536b57d8bfa5db211fd6 Reviewed-on: https://chromium-review.googlesource.com/c/1277525 Commit-Queue: Jonathan Freed Reviewed-by: Steven Holte Reviewed-by: Justin DeWitt Cr-Original-Commit-Position: refs/heads/master@{#599237}(cherry picked from commit 0ad8cbec6313fb12f99d972f39a97ff15b3ad212) Reviewed-on: https://chromium-review.googlesource.com/c/1284812 Reviewed-by: Cathy Li Cr-Commit-Position: refs/branch-heads/3578@{#73} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} --- .../android/explore_sites/image_helper.cc | 17 ++++++--- .../explore_sites/image_helper_unittest.cc | 35 +++++++++++++++++++ tools/metrics/histograms/histograms.xml | 5 +++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/chrome/browser/android/explore_sites/image_helper.cc b/chrome/browser/android/explore_sites/image_helper.cc index 7b417c4c5aa1..d85887c747e5 100644 --- a/chrome/browser/android/explore_sites/image_helper.cc +++ b/chrome/browser/android/explore_sites/image_helper.cc @@ -5,6 +5,7 @@ #include "chrome/browser/android/explore_sites/image_helper.h" #include "base/memory/weak_ptr.h" +#include "base/metrics/histogram_macros.h" #include "chrome/browser/android/explore_sites/explore_sites_types.h" #include "content/public/common/service_manager_connection.h" #include "services/data_decoder/public/cpp/decode_image.h" @@ -123,11 +124,17 @@ void ImageHelper::Job::DecodeImageBytes( std::move(callback)); } +void RecordImageDecodedUMA(bool decoded) { + UMA_HISTOGRAM_BOOLEAN("ExploreSites.ImageDecoded", decoded); +} + void ImageHelper::Job::OnDecodeSiteImageDone(const SkBitmap& decoded_image) { + bool decode_success = !decoded_image.isNull(); DVLOG(1) << "Decoded site image, result " - << (decoded_image.isNull() ? "null" : "non-null"); + << (decode_success ? "non-null" : "null"); + RecordImageDecodedUMA(decode_success); - if (decoded_image.isNull()) { + if (!decode_success) { std::move(bitmap_callback_).Run(nullptr); } else { std::move(bitmap_callback_).Run(std::make_unique(decoded_image)); @@ -137,10 +144,12 @@ void ImageHelper::Job::OnDecodeSiteImageDone(const SkBitmap& decoded_image) { void ImageHelper::Job::OnDecodeCategoryImageDone( const SkBitmap& decoded_image) { + bool decode_success = !decoded_image.isNull(); DVLOG(1) << "Decoded image for category, result " - << (decoded_image.isNull() ? "null" : "non-null"); + << (decode_success ? "non-null" : "null"); + RecordImageDecodedUMA(decode_success); - if (decoded_image.isNull()) { + if (!decode_success) { num_icons_--; } else { bitmaps_.push_back(decoded_image); diff --git a/chrome/browser/android/explore_sites/image_helper_unittest.cc b/chrome/browser/android/explore_sites/image_helper_unittest.cc index 7b23998d132c..02de1982e5b7 100644 --- a/chrome/browser/android/explore_sites/image_helper_unittest.cc +++ b/chrome/browser/android/explore_sites/image_helper_unittest.cc @@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/test/bind_test_util.h" +#include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_task_environment.h" #include "base/test/test_simple_task_runner.h" #include "services/data_decoder/public/cpp/test_data_decoder_service.h" @@ -61,6 +62,8 @@ class ExploreSitesImageHelperTest : public testing::Test { }); } + const base::HistogramTester& histograms() const { return histogram_tester_; } + std::vector> last_bitmap_list; protected: @@ -71,6 +74,7 @@ class ExploreSitesImageHelperTest : public testing::Test { } std::unique_ptr connector_factory_; + base::HistogramTester histogram_tester_; }; // 1x1 webp image with color value of 0. @@ -271,4 +275,35 @@ TEST_F(ExploreSitesImageHelperTest, TestImageHelper_CategoryImage_InvalidWebP) { } } +// Test that the ExploreSites.ImageDecoded UMA works. +TEST_F(ExploreSitesImageHelperTest, TestImageHelper_ImageDecodedUMA) { + ImageHelper image_helper; + + // Record one success UMA from CompseSiteImage. + image_helper.ComposeSiteImage(StoreBitmap(), GetEncodedImageList(1), + GetConnector()); + scoped_task_environment_.RunUntilIdle(); + + histograms().ExpectTotalCount("ExploreSites.ImageDecoded", 1); + histograms().ExpectBucketCount("ExploreSites.ImageDecoded", true, 1); + + // Record one failure UMA from ComposeSiteImage. + EncodedImageList image_list; + image_list.push_back(std::make_unique(kInvalidWebpBytes)); + image_helper.ComposeSiteImage(StoreBitmap(), std::move(image_list), + GetConnector()); + scoped_task_environment_.RunUntilIdle(); + + histograms().ExpectTotalCount("ExploreSites.ImageDecoded", 2); + histograms().ExpectBucketCount("ExploreSites.ImageDecoded", false, 1); + + // Record 2 samples from ComposeCategoryImage. + image_helper.ComposeCategoryImage(StoreBitmap(), kIconSize, + GetEncodedImageList(2), GetConnector()); + scoped_task_environment_.RunUntilIdle(); + + histograms().ExpectTotalCount("ExploreSites.ImageDecoded", 4); + histograms().ExpectBucketCount("ExploreSites.ImageDecoded", true, 3); +} + } // namespace explore_sites diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index c707819537c2..573f94b2e4ef 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -28145,6 +28145,11 @@ uploading your change for review. + + freedjm@chromium.org + Tracks the result of image decoding for the favicons. + + dimich@chromium.org