Skip to content

Commit dd66965

Browse files
Jonathan FreedCathy Li
Jonathan Freed
authored and
Cathy Li
committed
[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 <[email protected]> Reviewed-by: Steven Holte <[email protected]> Reviewed-by: Justin DeWitt <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#599237}(cherry picked from commit 0ad8cbe) Reviewed-on: https://chromium-review.googlesource.com/c/1284812 Reviewed-by: Cathy Li <[email protected]> Cr-Commit-Position: refs/branch-heads/3578@{#73} Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
1 parent 7965d1b commit dd66965

File tree

3 files changed

+53
-4
lines changed

3 files changed

+53
-4
lines changed

chrome/browser/android/explore_sites/image_helper.cc

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "chrome/browser/android/explore_sites/image_helper.h"
66

77
#include "base/memory/weak_ptr.h"
8+
#include "base/metrics/histogram_macros.h"
89
#include "chrome/browser/android/explore_sites/explore_sites_types.h"
910
#include "content/public/common/service_manager_connection.h"
1011
#include "services/data_decoder/public/cpp/decode_image.h"
@@ -123,11 +124,17 @@ void ImageHelper::Job::DecodeImageBytes(
123124
std::move(callback));
124125
}
125126

127+
void RecordImageDecodedUMA(bool decoded) {
128+
UMA_HISTOGRAM_BOOLEAN("ExploreSites.ImageDecoded", decoded);
129+
}
130+
126131
void ImageHelper::Job::OnDecodeSiteImageDone(const SkBitmap& decoded_image) {
132+
bool decode_success = !decoded_image.isNull();
127133
DVLOG(1) << "Decoded site image, result "
128-
<< (decoded_image.isNull() ? "null" : "non-null");
134+
<< (decode_success ? "non-null" : "null");
135+
RecordImageDecodedUMA(decode_success);
129136

130-
if (decoded_image.isNull()) {
137+
if (!decode_success) {
131138
std::move(bitmap_callback_).Run(nullptr);
132139
} else {
133140
std::move(bitmap_callback_).Run(std::make_unique<SkBitmap>(decoded_image));
@@ -137,10 +144,12 @@ void ImageHelper::Job::OnDecodeSiteImageDone(const SkBitmap& decoded_image) {
137144

138145
void ImageHelper::Job::OnDecodeCategoryImageDone(
139146
const SkBitmap& decoded_image) {
147+
bool decode_success = !decoded_image.isNull();
140148
DVLOG(1) << "Decoded image for category, result "
141-
<< (decoded_image.isNull() ? "null" : "non-null");
149+
<< (decode_success ? "non-null" : "null");
150+
RecordImageDecodedUMA(decode_success);
142151

143-
if (decoded_image.isNull()) {
152+
if (!decode_success) {
144153
num_icons_--;
145154
} else {
146155
bitmaps_.push_back(decoded_image);

chrome/browser/android/explore_sites/image_helper_unittest.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "base/bind.h"
1111
#include "base/test/bind_test_util.h"
12+
#include "base/test/metrics/histogram_tester.h"
1213
#include "base/test/scoped_task_environment.h"
1314
#include "base/test/test_simple_task_runner.h"
1415
#include "services/data_decoder/public/cpp/test_data_decoder_service.h"
@@ -61,6 +62,8 @@ class ExploreSitesImageHelperTest : public testing::Test {
6162
});
6263
}
6364

65+
const base::HistogramTester& histograms() const { return histogram_tester_; }
66+
6467
std::vector<std::unique_ptr<SkBitmap>> last_bitmap_list;
6568

6669
protected:
@@ -71,6 +74,7 @@ class ExploreSitesImageHelperTest : public testing::Test {
7174
}
7275

7376
std::unique_ptr<service_manager::TestConnectorFactory> connector_factory_;
77+
base::HistogramTester histogram_tester_;
7478
};
7579

7680
// 1x1 webp image with color value of 0.
@@ -271,4 +275,35 @@ TEST_F(ExploreSitesImageHelperTest, TestImageHelper_CategoryImage_InvalidWebP) {
271275
}
272276
}
273277

278+
// Test that the ExploreSites.ImageDecoded UMA works.
279+
TEST_F(ExploreSitesImageHelperTest, TestImageHelper_ImageDecodedUMA) {
280+
ImageHelper image_helper;
281+
282+
// Record one success UMA from CompseSiteImage.
283+
image_helper.ComposeSiteImage(StoreBitmap(), GetEncodedImageList(1),
284+
GetConnector());
285+
scoped_task_environment_.RunUntilIdle();
286+
287+
histograms().ExpectTotalCount("ExploreSites.ImageDecoded", 1);
288+
histograms().ExpectBucketCount("ExploreSites.ImageDecoded", true, 1);
289+
290+
// Record one failure UMA from ComposeSiteImage.
291+
EncodedImageList image_list;
292+
image_list.push_back(std::make_unique<EncodedImageBytes>(kInvalidWebpBytes));
293+
image_helper.ComposeSiteImage(StoreBitmap(), std::move(image_list),
294+
GetConnector());
295+
scoped_task_environment_.RunUntilIdle();
296+
297+
histograms().ExpectTotalCount("ExploreSites.ImageDecoded", 2);
298+
histograms().ExpectBucketCount("ExploreSites.ImageDecoded", false, 1);
299+
300+
// Record 2 samples from ComposeCategoryImage.
301+
image_helper.ComposeCategoryImage(StoreBitmap(), kIconSize,
302+
GetEncodedImageList(2), GetConnector());
303+
scoped_task_environment_.RunUntilIdle();
304+
305+
histograms().ExpectTotalCount("ExploreSites.ImageDecoded", 4);
306+
histograms().ExpectBucketCount("ExploreSites.ImageDecoded", true, 3);
307+
}
308+
274309
} // namespace explore_sites

tools/metrics/histograms/histograms.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28145,6 +28145,11 @@ uploading your change for review.
2814528145
</summary>
2814628146
</histogram>
2814728147

28148+
<histogram name="ExploreSites.ImageDecoded" units="Boolean">
28149+
<owner>[email protected]</owner>
28150+
<summary>Tracks the result of image decoding for the favicons.</summary>
28151+
</histogram>
28152+
2814828153
<histogram name="ExploreSites.MonthlyHostCount" units="hosts">
2814928154
<owner>[email protected]</owner>
2815028155
<summary>

0 commit comments

Comments
 (0)