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..decf737927a3712637c27ed901cfe4acb1407d36 100644
--- a/components/doodle/doodle_service_unittest.cc
+++ b/components/doodle/doodle_service_unittest.cc
@@ -55,6 +55,7 @@ class MockDoodleObserver : public DoodleService::Observer {
  public:
   MOCK_METHOD1(OnDoodleConfigUpdated,
                void(const base::Optional<DoodleConfig>&));
+  MOCK_METHOD1(OnDoodleConfigRevalidated, void(bool));
 };
 
 DoodleConfig CreateConfig(DoodleType type) {
@@ -309,7 +310,7 @@ TEST_F(DoodleServiceTest, CallsObserverOnConfigUpdated) {
   service()->RemoveObserver(&observer);
 }
 
-TEST_F(DoodleServiceTest, DoesNotCallObserverIfConfigEquivalent) {
+TEST_F(DoodleServiceTest, CallsObserverIfConfigRevalidatedByNetworkRequest) {
   // Load some doodle config.
   service()->Refresh();
   DoodleConfig config = CreateConfig(DoodleType::SIMPLE);
@@ -317,14 +318,18 @@ TEST_F(DoodleServiceTest, DoesNotCallObserverIfConfigEquivalent) {
                                base::TimeDelta::FromHours(1), config);
   ASSERT_THAT(service()->config(), Eq(config));
 
-  // Register an observer and request a refresh.
+  // Let some time pass (more than the refresh interval).
+  task_runner()->FastForwardBy(base::TimeDelta::FromMinutes(16));
+
+  // Register an observer and request a refresh after refresh intervall passed.
   StrictMock<MockDoodleObserver> observer;
+  EXPECT_CALL(observer, OnDoodleConfigRevalidated(Eq(/*from_cache=*/false)));
   service()->AddObserver(&observer);
   service()->Refresh();
   ASSERT_THAT(fetcher()->num_pending_callbacks(), Eq(1u));
 
   // Serve the request with an equivalent doodle config. The observer should
-  // *not* get notified.
+  // get notified about a (non-cached) revalidation.
   DoodleConfig equivalent_config = CreateConfig(DoodleType::SIMPLE);
   DCHECK(config == equivalent_config);
   fetcher()->ServeAllCallbacks(
@@ -334,6 +339,28 @@ TEST_F(DoodleServiceTest, DoesNotCallObserverIfConfigEquivalent) {
   service()->RemoveObserver(&observer);
 }
 
+TEST_F(DoodleServiceTest, CallsObserverIfConfigRevalidatedByCache) {
+  // Create a service with the default refresh interval.
+  RecreateService(/*min_refresh_interval=*/base::nullopt);
+
+  // Load some doodle config.
+  service()->Refresh();
+  DoodleConfig config = CreateConfig(DoodleType::SIMPLE);
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE,
+                               base::TimeDelta::FromHours(1), config);
+  ASSERT_THAT(service()->config(), Eq(config));
+
+  // Register an observer and request a refresh within refresh intervall.
+  StrictMock<MockDoodleObserver> observer;
+  EXPECT_CALL(observer, OnDoodleConfigRevalidated(Eq(/*from_cache=*/true)));
+  service()->AddObserver(&observer);
+  service()->Refresh();
+  ASSERT_THAT(fetcher()->num_pending_callbacks(), Eq(0u));
+
+  // Remove the observer before the service gets destroyed.
+  service()->RemoveObserver(&observer);
+}
+
 TEST_F(DoodleServiceTest, CallsObserverWhenConfigExpires) {
   // Load some doodle config.
   service()->Refresh();
@@ -364,10 +391,12 @@ TEST_F(DoodleServiceTest, DisregardsAlreadyExpiredConfigs) {
   StrictMock<MockDoodleObserver> observer;
   service()->AddObserver(&observer);
 
+  // If there was no config and an expired config is loaded, not having a config
+  // must be revalidated.
+  EXPECT_CALL(observer, OnDoodleConfigRevalidated(Eq(/*from_cache=*/false)));

[Marc Treib, 2017/04/28 08:22:59]

I like to have EXPECT_CALLs as close as possible to the thing that actually
triggers them. Can we move this to just before ServeAllCallbacks?

[fhorschig, 2017/04/28 08:29:01]

Sure.

   ASSERT_THAT(service()->config(), Eq(base::nullopt));
 
-  // Load an already-expired config. This should have no effect; in particular
-  // no call to the observer.
+  // Load an already-expired config.
   service()->Refresh();
   DoodleConfig config = CreateConfig(DoodleType::SIMPLE);
   fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE,