Record NTP.LogoShownTime for timely refreshs only

components/doodle/doodle_service.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/doodle/doodle_service.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
@@ skipping 69 matching lines @@
   observers_.RemoveObserver(observer);
 }
 
 void DoodleService::Refresh() {
   base::TimeTicks now_ticks = tick_clock_->NowTicks();
   // Check if we have passed the minimum refresh interval.
   base::TimeDelta time_since_fetch = now_ticks - last_successful_fetch_;
   if (time_since_fetch < min_refresh_interval_) {
     RecordDownloadMetrics(OUTCOME_REFRESH_INTERVAL_NOT_PASSED,
                           base::TimeDelta());
+    for (auto& observer : observers_) {
+      observer.OnDoodleConfigRevalidated(/*from_cache=*/true);
+    }
     return;
   }
   fetcher_->FetchDoodle(base::BindOnce(&DoodleService::DoodleFetched,
                                        base::Unretained(this), now_ticks));
 }
 
 // static
 bool DoodleService::DownloadOutcomeIsSuccess(DownloadOutcome outcome) {
   switch (outcome) {
     case OUTCOME_NEW_DOODLE:
@@ skipping 85 matching lines @@
 
   // Handle the case where the new config is already expired.
   bool expired = time_to_live <= base::TimeDelta();
   const base::Optional<DoodleConfig>& new_config =
       expired ? base::nullopt : doodle_config;
 
   // Determine the download outcome *before* updating the cached config.
   DownloadOutcome outcome =
       DetermineDownloadOutcome(cached_config_, new_config, state, expired);
 
-  // If the config changed, update our cache and notify observers.
   // Note that this checks both for existence changes as well as changes of the
   // configs themselves.
   if (cached_config_ != new_config) {
     UpdateCachedConfig(time_to_live, new_config);
 
     for (auto& observer : observers_) {
       observer.OnDoodleConfigUpdated(cached_config_);
     }
+  } else if (!expired) {

[Marc Treib, 2017/04/27 15:22:55]

Why the "if (!expired)"? This seems like an edge case where we'd end up not
calling the UI again?

[fhorschig, 2017/04/28 08:20:07]

Gone.
We previously checked in the test DisregardsAlreadyExpiredConfigs that no
observer should be called. But having no config because the most recent
config is expired can be treated as revalidation.
This is in line with what this event means --> Test expectations changed.

+    for (auto& observer : observers_) {
+      observer.OnDoodleConfigRevalidated(/*from_cache=*/false);
+    }
   }
 
   // Even if the configs are identical, the time-to-live might have changed.
   // (Re-)schedule the cache expiry.
   if (cached_config_.has_value()) {
     expiry_timer_->Start(
         FROM_HERE, time_to_live,
         base::Bind(&DoodleService::DoodleExpired, base::Unretained(this)));
   } else {
     expiry_timer_->Stop();
@@ skipping 19 matching lines @@
     pref_service_->ClearPref(prefs::kCachedConfigExpiry);
   }
 }
 
 void DoodleService::DoodleExpired() {
   DCHECK(cached_config_.has_value());
   HandleNewConfig(DoodleState::NO_DOODLE, base::TimeDelta(), base::nullopt);
 }
 
 }  // namespace doodle