Skip to content

Commit

Permalink
[Explore Sites] Adding UMA to capture the result of the ImageDecoder …
Browse files Browse the repository at this point in the history
…service.

Change-Id: I1a5cca85ac14fcec14a6536b57d8bfa5db211fd6
Reviewed-on: https://chromium-review.googlesource.com/c/1277525
Commit-Queue: Jonathan Freed <freedjm@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599237}
  • Loading branch information
Jonathan Freed authored and Commit Bot committed Oct 12, 2018
1 parent 659bcd6 commit 0ad8cbe
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 4 deletions.
17 changes: 13 additions & 4 deletions chrome/browser/android/explore_sites/image_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<SkBitmap>(decoded_image));
Expand All @@ -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);
Expand Down
35 changes: 35 additions & 0 deletions chrome/browser/android/explore_sites/image_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -61,6 +62,8 @@ class ExploreSitesImageHelperTest : public testing::Test {
});
}

const base::HistogramTester& histograms() const { return histogram_tester_; }

std::vector<std::unique_ptr<SkBitmap>> last_bitmap_list;

protected:
Expand All @@ -71,6 +74,7 @@ class ExploreSitesImageHelperTest : public testing::Test {
}

std::unique_ptr<service_manager::TestConnectorFactory> connector_factory_;
base::HistogramTester histogram_tester_;
};

// 1x1 webp image with color value of 0.
Expand Down Expand Up @@ -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<EncodedImageBytes>(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
5 changes: 5 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28145,6 +28145,11 @@ uploading your change for review.
</summary>
</histogram>

<histogram name="ExploreSites.ImageDecoded" units="Boolean">
<owner>freedjm@chromium.org</owner>
<summary>Tracks the result of image decoding for the favicons.</summary>
</histogram>

<histogram name="ExploreSites.MonthlyHostCount" units="hosts">
<owner>dimich@chromium.org</owner>
<summary>
Expand Down

0 comments on commit 0ad8cbe

Please sign in to comment.