[Icon cacher] Add metrics for downloading favicons for NTP Tiles

components/ntp_tiles/icon_cacher_impl_unittest.cc

Index: components/ntp_tiles/icon_cacher_impl_unittest.cc
diff --git a/components/ntp_tiles/icon_cacher_impl_unittest.cc b/components/ntp_tiles/icon_cacher_impl_unittest.cc
index 0e3620876310963e4e5438b4dca91aa37b8c5a44..cd0198f6bb598df42168eaa012217806fd1e562f 100644
--- a/components/ntp_tiles/icon_cacher_impl_unittest.cc
+++ b/components/ntp_tiles/icon_cacher_impl_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/test/histogram_tester.h"
 #include "base/test/mock_callback.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/test/test_simple_task_runner.h"
@@ -30,7 +31,9 @@
 #include "ui/base/ui_base_paths.h"
 #include "ui/gfx/image/image_unittest_util.h"
 
+using base::Bucket;
 using ::testing::_;
+using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Invoke;
 using ::testing::InSequence;
@@ -401,6 +404,54 @@ TEST_F(IconCacherTestPopularSites, LargeNotCachedAndFetchPerformedOnlyOnce) {
   EXPECT_TRUE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
 }
 
+TEST_F(IconCacherTestPopularSites, LargeNotCachedAndFetchSuccessReported) {
+  base::HistogramTester histogram_tester;
+  base::MockCallback<base::Closure> done;
+  base::RunLoop loop;
+  {
+    InSequence s;
+    EXPECT_CALL(*image_fetcher_,
+                SetDataUseServiceName(
+                    data_use_measurement::DataUseUserData::NTP_TILES));
+    EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
+    EXPECT_CALL(*image_fetcher_,
+                StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
+        .WillOnce(PassFetch(128, 128));
+    EXPECT_CALL(done, Run()).WillOnce(Quit(&loop));
+  }
+
+  IconCacherImpl cacher(&favicon_service_, nullptr, std::move(image_fetcher_));
+  cacher.StartFetchPopularSites(site_, done.Get(), done.Get());
+  loop.Run();
+  EXPECT_THAT(
+      histogram_tester.GetAllSamples("NewTabPage.TileFaviconFetched.popular"),
+      ElementsAre(Bucket(/*bucket=*/0, /*count=*/1)));  // bucket 0 = SUCCESS.
+}
+
+TEST_F(IconCacherTestPopularSites, LargeNotCachedAndFetchFailureReported) {
+  base::HistogramTester histogram_tester;
+  base::MockCallback<base::Closure> done;
+  EXPECT_CALL(done, Run()).Times(0);
+  {
+    InSequence s;
+    EXPECT_CALL(*image_fetcher_,
+                SetDataUseServiceName(
+                    data_use_measurement::DataUseUserData::NTP_TILES));
+    EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
+    EXPECT_CALL(*image_fetcher_,
+                StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
+        .WillOnce(FailFetch());
+  }
+
+  IconCacherImpl cacher(&favicon_service_, nullptr, std::move(image_fetcher_));
+  cacher.StartFetchPopularSites(site_, done.Get(), done.Get());
+  WaitForHistoryThreadTasksToFinish();
+  WaitForMainThreadTasksToFinish();
+  EXPECT_THAT(
+      histogram_tester.GetAllSamples("NewTabPage.TileFaviconFetched.popular"),
+      ElementsAre(Bucket(/*bucket=*/1, /*count=*/1)));  // bucket 1 = FAILURE.
+}
+

[sfiera, 2017/05/18 13:40:32]

Could you also please test that cached and baked-in icons do not result in any
stats?

If it simplifies things for you, I would be OK putting the histogram checking in
the main tests. Metrics shouldn't be "separate and extra", they should be part
of the functionality :) (but separate is also fine by me)

[jkrcal, 2017/05/18 14:09:54]

Done.

I've merged histograms expects back into the original tests. I've put them only
into a subset of tests, where it makes sense to me.

 class IconCacherTestMostLikely : public IconCacherTestBase {
  protected:
   IconCacherTestMostLikely()
@@ -573,5 +624,74 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchPerformedOnlyOnce) {
   EXPECT_TRUE(IconIsCachedFor(page_url, favicon_base::TOUCH_ICON));
 }
 
+TEST_F(IconCacherTestMostLikely, NotCachedAndFetchSuccessReported) {
+  base::HistogramTester histogram_tester;
+  GURL page_url("http://www.site.com");
+
+  base::MockCallback<base::Closure> done;
+  base::RunLoop loop;
+  {
+    InSequence s;
+    EXPECT_CALL(*fetcher_for_large_icon_service_,
+                SetDataUseServiceName(
+                    data_use_measurement::DataUseUserData::LARGE_ICON_SERVICE));
+    EXPECT_CALL(*fetcher_for_large_icon_service_,
+                StartOrQueueNetworkRequest(_, _, _))
+        .WillOnce(PassFetch(128, 128));
+    EXPECT_CALL(done, Run()).WillOnce(Quit(&loop));
+  }
+
+  favicon::LargeIconService large_icon_service(
+      &favicon_service_, large_icon_service_background_task_runner_,
+      std::move(fetcher_for_large_icon_service_));
+  IconCacherImpl cacher(&favicon_service_, &large_icon_service,
+                        std::move(fetcher_for_icon_cacher_));
+
+  cacher.StartFetchMostLikely(page_url, done.Get());
+  // Both these task runners need to be flushed in order to get |done| called by
+  // running the main loop.
+  WaitForHistoryThreadTasksToFinish();
+  large_icon_service_background_task_runner_->RunUntilIdle();
+
+  loop.Run();
+  EXPECT_THAT(
+      histogram_tester.GetAllSamples("NewTabPage.TileFaviconFetched.server"),
+      ElementsAre(Bucket(/*bucket=*/0, /*count=*/1)));  // bucket 0 = SUCCESS.
+}
+
+TEST_F(IconCacherTestMostLikely, NotCachedAndFetchFailureReported) {
+  base::HistogramTester histogram_tester;
+  GURL page_url("http://www.site.com");
+
+  base::MockCallback<base::Closure> done;
+  {
+    InSequence s;
+    EXPECT_CALL(*fetcher_for_large_icon_service_,
+                SetDataUseServiceName(
+                    data_use_measurement::DataUseUserData::LARGE_ICON_SERVICE));
+    EXPECT_CALL(*fetcher_for_large_icon_service_,
+                StartOrQueueNetworkRequest(_, _, _))
+        .WillOnce(FailFetch());
+    EXPECT_CALL(done, Run()).Times(0);
+  }
+
+  favicon::LargeIconService large_icon_service(
+      &favicon_service_, large_icon_service_background_task_runner_,
+      std::move(fetcher_for_large_icon_service_));
+  IconCacherImpl cacher(&favicon_service_, &large_icon_service,
+                        std::move(fetcher_for_icon_cacher_));
+
+  cacher.StartFetchMostLikely(page_url, done.Get());
+  // Both these task runners need to be flushed before flushing the main thread
+  // queue in order to finish the work.
+  WaitForHistoryThreadTasksToFinish();
+  large_icon_service_background_task_runner_->RunUntilIdle();
+  WaitForMainThreadTasksToFinish();
+
+  EXPECT_THAT(
+      histogram_tester.GetAllSamples("NewTabPage.TileFaviconFetched.server"),
+      ElementsAre(Bucket(/*bucket=*/1, /*count=*/1)));  // bucket 1 = FAILURE.
+}
+
 }  // namespace
 }  // namespace ntp_tiles