Record NTP.LogoShownTime for timely refreshs only

components/doodle/doodle_service_unittest.cc

Index: components/doodle/doodle_service_unittest.cc
diff --git a/components/doodle/doodle_service_unittest.cc b/components/doodle/doodle_service_unittest.cc
index 8f6fc7e1011d70d07632e7a9e00e68b94adf3ffc..c413ec0f2a929e0d3ea374720b05c57915edcb72 100644
--- a/components/doodle/doodle_service_unittest.cc
+++ b/components/doodle/doodle_service_unittest.cc
@@ -490,6 +490,23 @@ TEST_F(DoodleServiceTest, RecordsMetricsForEarlyRefreshRequest) {
   histograms.ExpectTotalCount("Doodle.ConfigDownloadTime", 0);
 }
 
+TEST_F(DoodleServiceTest, RefreshReturnsFalseOnlyForSkippedRefreshAttempts) {
+  // Create a service with some refresh interval.
+  RecreateService(/*min_refresh_interval=*/base::TimeDelta::FromMinutes(10));
+
+  // Trigger a refresh within the min refresh interval that is not skipped.

[tschumann, 2017/04/20 18:53:11]

if find the last part of the sentence extremely confusing. I guess you're trying
to say that this actually results in a fetch? (also see next comment below).

[fhorschig, 2017/04/24 13:52:35]

Yes, changed/merged.

+  EXPECT_TRUE(service()->Refresh());
+  fetcher()->ServeAllCallbacks(DoodleState::NO_DOODLE, base::TimeDelta(),

[tschumann, 2017/04/20 18:53:11]

can you explain the parameters with /**/ comments? 
Alternatively extending the comment above to something like:
// Trigger a refresh that gets the information about no doodle being available.

[fhorschig, 2017/04/24 13:52:35]

Done.

+                               base::nullopt);
+
+  // Request a refresh before the min refresh interval has passed.
+  task_runner()->FastForwardBy(base::TimeDelta::FromMinutes(5));

[tschumann, 2017/04/20 18:53:11]

why forward the time at all?

[fhorschig, 2017/04/24 13:52:35]

Gone.

+  EXPECT_FALSE(service()->Refresh());
+
+  // This should not have resulted in a request.

[tschumann, 2017/04/20 18:53:11]

nit: combine this comment with the one above and write the 2 expectations in one
block?

[fhorschig, 2017/04/24 13:52:34]

Done.

+  EXPECT_THAT(fetcher()->num_pending_callbacks(), Eq(0u));
+}
+
 TEST_F(DoodleServiceTest, DoesNotRecordMetricsAtStartup) {
   // Creating the service should not emit any histogram samples.
   base::HistogramTester histograms;