Move NewTabPage.LogoShownTime metrics recording into LogoBridge

chrome/browser/android/logo_bridge.h

Index: chrome/browser/android/logo_bridge.h
diff --git a/chrome/browser/android/logo_bridge.h b/chrome/browser/android/logo_bridge.h
index 89f5f48b12a3ecf1dbc0e09e08098b45d4aee407..0fac79ccbada46b13fe4bc87e0b766f3bbf731f1 100644
--- a/chrome/browser/android/logo_bridge.h
+++ b/chrome/browser/android/logo_bridge.h
@@ -35,6 +35,24 @@ struct RequestMetadata;
 // default search provider's logo.
 class LogoBridge : public doodle::DoodleService::Observer {
  public:
+  // Stores its creation time and writes it to a UMA histogram. Each instance
+  // writes the metric once or never.
+  class LoadTimeMetricRecorder {

[tschumann, 2017/04/27 06:09:17]

this seems overly complicated to me. Why do we need to maintain so much state?
What are we abstracting here?

I guess the main question is why do we have a long-lived metric-recorder (as a
member) and not one per request. In the end, it's all about capturing a start
time and deciding whether to log it or not, based on conditions we already
know... (at the places where we now call the recorder).

The more state we can make request-scoped (i.e. not outlive the request), the
simpler the solution would get (e.g., one-time callbacks instead of registered
observers; per-request objects (passed through parameters) instead of members.
Happy to discuss offline.

[fhorschig, 2017/04/27 11:16:13]

I moved the state out of the class. 
Although this CL should be abandoned soon as we discussed to do this in the UI.

+   public:
+    // Writes the diff between |Now| and the recorders creation time to a UMA
+    // metric. If the recorder was aborted/finished before, nothing happens.
+    virtual void Finish() = 0;
+
+    // The recorder instance immediately stops to record.
+    virtual void Abort() = 0;
+
+    // The recorder waits for a pending result.
+    virtual void WaitForResult() = 0;
+
+    // Unless the recorder is waiting for a result, |Abort| will be called.
+    virtual void AbortIfNotWaiting() = 0;
+  };
+
   explicit LogoBridge(jobject j_profile);
   void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
 
@@ -75,6 +93,10 @@ class LogoBridge : public doodle::DoodleService::Observer {
                           const gfx::Image& image,
                           const image_fetcher::RequestMetadata& metadata);
 
+  // Returns the currently running LoadTimeRecorder. If there is none, a null
+  // object is returned.
+  LoadTimeMetricRecorder* load_time_recorder();
+
   // Only valid if UseNewDoodleApi is disabled.
   LogoService* logo_service_;
 
@@ -87,6 +109,8 @@ class LogoBridge : public doodle::DoodleService::Observer {
 
   std::unique_ptr<AnimatedLogoFetcher> animated_logo_fetcher_;
 
+  std::unique_ptr<LoadTimeMetricRecorder> load_time_recorder_;
+
   base::WeakPtrFactory<LogoBridge> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LogoBridge);