Move NewTabPage.LogoShownTime metrics recording into LogoBridge

chrome/browser/android/logo_bridge.cc

Index: chrome/browser/android/logo_bridge.cc
diff --git a/chrome/browser/android/logo_bridge.cc b/chrome/browser/android/logo_bridge.cc
index 8b24611abf5673c2f5d5822656c2494971bc0b04..6e4bcddc552dc0881c1b01a0e841caa9e69cb13d 100644
--- a/chrome/browser/android/logo_bridge.cc
+++ b/chrome/browser/android/logo_bridge.cc
@@ -77,12 +77,43 @@ ScopedJavaLocalRef<jobject> ConvertLogoToJavaObject(
                       GURL(logo->metadata.animated_url));
 }
 
+class LoadTimeMetricRecorderImpl : public LogoBridge::LoadTimeMetricRecorder {
+ public:
+  LoadTimeMetricRecorderImpl()
+      : logo_load_start_(base::TimeTicks::Now()), is_recording_(true) {}
+
+  void Cancel() override { is_recording_ = false; }
+
+  void Finish() override {
+    if (!is_recording_) {
+      return;
+    }
+    is_recording_ = false;
+    UMA_HISTOGRAM_MEDIUM_TIMES("NewTabPage.LogoShownTime",
+                               base::TimeTicks::Now() - logo_load_start_);
+  }
+
+ private:
+  base::TimeTicks logo_load_start_;
+  bool is_recording_;
+  bool waiting_for_result_;
+};
+
+class LoadTimeMetricRecorderNullImpl
+    : public LogoBridge::LoadTimeMetricRecorder {
+ public:
+  LoadTimeMetricRecorderNullImpl() {}
+  void Cancel() override {}
+  void Finish() override {}
+};
+
 class LogoObserverAndroid : public search_provider_logos::LogoObserver {
  public:
   LogoObserverAndroid(base::WeakPtr<LogoBridge> logo_bridge,
                       JNIEnv* env,
                       jobject j_logo_observer)
-      : logo_bridge_(logo_bridge) {
+      : logo_bridge_(logo_bridge),
+        load_time_recorder_(new LoadTimeMetricRecorderImpl()) {
     j_logo_observer_.Reset(env, j_logo_observer);
   }
 
@@ -96,6 +127,7 @@ class LogoObserverAndroid : public search_provider_logos::LogoObserver {
 
     JNIEnv* env = base::android::AttachCurrentThread();
     ScopedJavaLocalRef<jobject> j_logo = ConvertLogoToJavaObject(env, logo);
+    load_time_recorder_->Finish();
     Java_LogoObserver_onLogoAvailable(env, j_logo_observer_, j_logo,
                                       from_cache);
   }
@@ -107,6 +139,7 @@ class LogoObserverAndroid : public search_provider_logos::LogoObserver {
   // been destroyed.
   base::WeakPtr<LogoBridge> logo_bridge_;
 
+  std::unique_ptr<LogoBridge::LoadTimeMetricRecorder> load_time_recorder_;
   base::android::ScopedJavaGlobalRef<jobject> j_logo_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(LogoObserverAndroid);
@@ -220,16 +253,28 @@ void LogoBridge::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
   delete this;
 }
 
+LogoBridge::LoadTimeMetricRecorder* LogoBridge::load_time_recorder() {
+  if (!load_time_recorder_) {
+    load_time_recorder_ = base::MakeUnique<LoadTimeMetricRecorderNullImpl>();
+  }
+  return load_time_recorder_.get();
+}
+
 void LogoBridge::GetCurrentLogo(JNIEnv* env,
                                 const JavaParamRef<jobject>& obj,
                                 const JavaParamRef<jobject>& j_logo_observer) {
   if (doodle_service_) {
     j_logo_observer_.Reset(j_logo_observer);
+    load_time_recorder_ = base::MakeUnique<LoadTimeMetricRecorderImpl>();
 
     // Immediately hand out any current cached config.
-    DoodleConfigReceived(doodle_service_->config(), /*from_cache=*/true);
+    bool cache_hit =
+        DoodleConfigReceived(doodle_service_->config(), /*from_cache=*/true);
     // Also request a refresh, in case something changed.
-    doodle_service_->Refresh();
+    bool refresh_triggered =
+        doodle_service_->Refresh() if (!cache_hit && !refresh_triggered) {
+      load_time_recorder()->Cancel();
+    }
   } else {
     // |observer| is deleted in LogoObserverAndroid::OnObserverRemoved().
     LogoObserverAndroid* observer = new LogoObserverAndroid(
@@ -254,7 +299,7 @@ void LogoBridge::OnDoodleConfigUpdated(
   DoodleConfigReceived(maybe_doodle_config, /*from_cache=*/false);
 }
 
-void LogoBridge::DoodleConfigReceived(
+bool LogoBridge::DoodleConfigReceived(
     const base::Optional<doodle::DoodleConfig>& maybe_doodle_config,
     bool from_cache) {
   DCHECK(!j_logo_observer_.is_null());
@@ -263,8 +308,9 @@ void LogoBridge::DoodleConfigReceived(
     JNIEnv* env = base::android::AttachCurrentThread();
     Java_LogoObserver_onLogoAvailable(
         env, j_logo_observer_, ScopedJavaLocalRef<jobject>(), from_cache);
-    return;
+    return false;
   }
+  load_time_recorder()->WaitForResult();
   const doodle::DoodleConfig& doodle_config = maybe_doodle_config.value();
   // If there is a CTA image, that means the main image is animated. Show the
   // non-animated CTA image first, and load the animated one only when the
@@ -282,6 +328,7 @@ void LogoBridge::DoodleConfigReceived(
       image_url.spec(), image_url,
       base::Bind(&LogoBridge::DoodleImageFetched, base::Unretained(this),
                  from_cache, on_click_url, alt_text, animated_image_url));
+  return true;
 }
 
 void LogoBridge::DoodleImageFetched(
@@ -296,6 +343,7 @@ void LogoBridge::DoodleImageFetched(
 
   if (image.IsEmpty()) {
     DLOG(WARNING) << "Failed to download doodle image";
+    load_time_recorder()->Cancel();
     Java_LogoObserver_onLogoAvailable(env, j_logo_observer_,
                                       ScopedJavaLocalRef<jobject>(),
                                       config_from_cache);
@@ -305,6 +353,7 @@ void LogoBridge::DoodleImageFetched(
   UMA_HISTOGRAM_BOOLEAN("NewTabPage.LogoImageDownloaded",
                         metadata.from_http_cache);
 
+  load_time_recorder()->Finish();
   ScopedJavaLocalRef<jobject> j_logo = MakeJavaLogo(
       env, image.ToSkBitmap(), on_click_url, alt_text, animated_image_url);
   Java_LogoObserver_onLogoAvailable(env, j_logo_observer_, j_logo,