Move prerender histograms out of prerender manager.

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
===================================================================
--- chrome/browser/prerender/prerender_manager.cc	(revision 96057)
+++ chrome/browser/prerender/prerender_manager.cc	(working copy)
@@ -8,9 +8,8 @@
 
 #include "base/command_line.h"
 #include "base/logging.h"
-#include "base/metrics/field_trial.h"
-#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
+#include "base/string_util.h"
 #include "base/time.h"
 #include "base/values.h"
 #include "base/utf_string_conversions.h"
@@ -19,6 +18,7 @@
 #include "chrome/browser/prerender/prerender_condition.h"
 #include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/prerender/prerender_final_status.h"
+#include "chrome/browser/prerender/prerender_histograms.h"
 #include "chrome/browser/prerender/prerender_history.h"
 #include "chrome/browser/prerender/prerender_observer.h"
 #include "chrome/browser/prerender/prerender_tracker.h"
@@ -68,37 +68,6 @@
 
 }  // namespace
 
-// Helper macros for experiment-based and origin-based histogram reporting.
-#define PREFIXED_HISTOGRAM(histogram) \
-  PREFIXED_HISTOGRAM_INTERNAL(GetCurrentOrigin(), GetCurrentExperimentId(), \
-                              IsOriginExperimentWash(), histogram)
-
-#define PREFIXED_HISTOGRAM_PRERENDER_MANAGER(pm, histogram) \
-  PREFIXED_HISTOGRAM_INTERNAL(pm->GetCurrentOrigin(), \
-                              pm->GetCurrentExperimentId(), \
-                              pm->IsOriginExperimentWash(), histogram)
-
-#define PREFIXED_HISTOGRAM_ORIGIN_EXPERIMENT(origin, experiment, histogram) \
-  PREFIXED_HISTOGRAM_INTERNAL(origin, experiment, false, histogram)
-
-#define PREFIXED_HISTOGRAM_INTERNAL(origin, experiment, wash, histogram) { \
-  static uint8 recording_experiment = kNoExperiment; \
-  if (recording_experiment == kNoExperiment && experiment != kNoExperiment) \
-    recording_experiment = experiment; \
-  if (wash) { \
-    histogram; \
-  } else if (experiment != kNoExperiment && \
-             (origin != ORIGIN_LINK_REL_PRERENDER || \
-              experiment != recording_experiment)) { \
-  } else if (experiment != kNoExperiment) { \
-    histogram; \
-  } else if (origin == ORIGIN_OMNIBOX) { \
-    histogram; \
-  } else { \
-    histogram; \
-  } \
-}
-
 class PrerenderManager::OnCloseTabContentsDeleter : public TabContentsDelegate {
  public:
   OnCloseTabContentsDeleter(PrerenderManager* manager,
@@ -203,13 +172,11 @@
       profile_(profile),
       prerender_tracker_(prerender_tracker),
       prerender_contents_factory_(PrerenderContents::CreateFactory()),
-      last_experiment_id_(kNoExperiment),
-      last_origin_(ORIGIN_LINK_REL_PRERENDER),
-      origin_experiment_wash_(false),
       last_prerender_start_time_(GetCurrentTimeTicks() -
           base::TimeDelta::FromMilliseconds(kMinTimeBetweenPrerendersMs)),
       runnable_method_factory_(this),
-      prerender_history_(new PrerenderHistory(kHistoryLength)) {
+      prerender_history_(new PrerenderHistory(kHistoryLength)),
+      histograms_(new PrerenderHistograms(config_.max_age)) {

[dominich, 2011/08/10 00:32:46]

If max_age is ever changed dynamically we should reset this on the histograms.
Or the histograms should take a pointer to the config and look it up from there
every time instead of taking a copy.

[tburkard, 2011/08/10 01:05:13]

Only an issue for tests, in which case histograms not matter.  Not an issue in
the production setup.

On 2011/08/10 00:32:46, dominich wrote:

[dominich, 2011/08/10 15:53:37]

For now, but there's nothing stopping someone from deciding to change this on
the fly. Currently it would work correctly as the PrerenderManager checks the
max_age every time. This change breaks the consistency between the Config and
the Histograms. To be correct it should take a reference/pointer to the config
or max_age so it can reflect any changes.

[tburkard, 2011/08/10 17:11:33]

Ok I changed it by passing in the value into the only method needing it every
time (rather than in the constructor).

On 2011/08/10 15:53:37, dominich wrote:

   // There are some assumptions that the PrerenderManager is on the UI thread.
   // Any other checks simply make sure that the PrerenderManager is accessed on
   // the same thread that it was created on.
@@ -253,31 +220,13 @@
     const GURL& referrer) {
   DCHECK(CalledOnValidThread());
 
-  // Check if we are doing an experiment.
-  uint8 experiment = GetQueryStringBasedExperiment(url_arg);
-
-  // We need to update last_experiment_id_, last_origin_, and
-  // origin_experiment_wash_.
-  if (!WithinWindow()) {
-    // If we are outside a window, this is a fresh start and we are fine,
-    // and there is no mix.
-    origin_experiment_wash_ = false;
-  } else {
-    // If we are inside the last window, there is a mish mash of origins
-    // and experiments if either there was a mish mash before, or the current
-    // experiment/origin does not match the previous one.
-    if (experiment != last_experiment_id_ || origin != last_origin_)
-      origin_experiment_wash_ = true;
+  if (origin == ORIGIN_LINK_REL_PRERENDER &&
+      StartsWithASCII(referrer.host(), std::string("www.google."), true)) {

[dominich, 2011/08/10 00:32:46]

Is the referrer ever google.* without the www.?

[tburkard, 2011/08/10 01:05:13]

No.

On 2011/08/10 00:32:46, dominich wrote:

+    origin = ORIGIN_GWS_PRERENDER;
   }
 
-  last_origin_ = origin;
-  last_experiment_id_ = experiment;
+  histograms_->RecordPrerender(origin, url_arg);
 
-  // If we observe multiple tags within the 30 second window, we will still
-  // reset the window to begin at the most recent occurrence, so that we will
-  // always be in a window in the 30 seconds from each occurrence.
-  last_prerender_seen_time_ = GetCurrentTimeTicks();
-
   // If the referring page is prerendering, defer the prerender.
   if (FindPrerenderContentsForChildRouteIdPair(child_route_id_pair) !=
           prerender_list_.end()) {
@@ -297,6 +246,8 @@
   if (FindEntry(url))
     return false;
 
+  uint8 experiment = GetQueryStringBasedExperiment(url_arg);
+
   // Do not prerender if there are too many render processes, and we would
   // have to use an existing one.  We do not want prerendering to happen in
   // a shared process, so that we can always reliably lower the CPU
@@ -512,12 +463,10 @@
     return false;
 
   if (!prerender_contents->load_start_time().is_null())
-    RecordTimeUntilUsed(GetCurrentTimeTicks() -
-                        prerender_contents->load_start_time());
+    histograms_->RecordTimeUntilUsed(GetCurrentTimeTicks() -
+                                     prerender_contents->load_start_time());
 
-  PREFIXED_HISTOGRAM(UMA_HISTOGRAM_COUNTS(
-      GetDefaultHistogramName("PrerendersPerSessionCount"),
-      ++prerenders_per_session_count_));
+  histograms_->RecordPerSessionCount(++prerenders_per_session_count_);
   prerender_contents->set_final_status(FINAL_STATUS_USED);
 
   RenderViewHost* render_view_host =
@@ -656,18 +605,6 @@
   }
 }
 
-// Helper macro for histograms.
-#define RECORD_PLT(tag, perceived_page_load_time) { \
-  PREFIXED_HISTOGRAM_PRERENDER_MANAGER(prerender_manager, \
-    UMA_HISTOGRAM_CUSTOM_TIMES( \
-        base::FieldTrial::MakeName( \
-            prerender_manager->GetDefaultHistogramName(tag), "Prefetch"), \
-        perceived_page_load_time, \
-        base::TimeDelta::FromMilliseconds(10), \
-        base::TimeDelta::FromSeconds(60), \
-        100)); \
-}
-
 // static
 void PrerenderManager::RecordPerceivedPageLoadTime(
     base::TimeDelta perceived_page_load_time,
@@ -680,20 +617,13 @@
     return;
   if (!prerender_manager->is_enabled())
     return;
-  bool within_window = prerender_manager->WithinWindow();
-  RECORD_PLT("PerceivedPLT", perceived_page_load_time);
-  if (within_window)
-    RECORD_PLT("PerceivedPLTWindowed", perceived_page_load_time);
-  if (prerender_manager &&
+  bool was_prerender =
       ((mode_ == PRERENDER_MODE_EXPERIMENT_CONTROL_GROUP &&
         prerender_manager->WouldTabContentsBePrerendered(tab_contents)) ||
        (mode_ == PRERENDER_MODE_EXPERIMENT_PRERENDER_GROUP &&
-        prerender_manager->IsTabContentsPrerendered(tab_contents)))) {
-    RECORD_PLT("PerceivedPLTMatched", perceived_page_load_time);
-  } else {
-    if (within_window)
-      RECORD_PLT("PerceivedPLTWindowNotMatched", perceived_page_load_time);
-  }
+        prerender_manager->IsTabContentsPrerendered(tab_contents)));
+  prerender_manager->histograms_->RecordPerceivedPageLoadTime(
+      perceived_page_load_time, was_prerender);
 }
 
 bool PrerenderManager::is_enabled() const {
@@ -766,23 +696,11 @@
   }
 }
 
-bool PrerenderManager::WithinWindow() const {
-  DCHECK(CalledOnValidThread());
-  if (last_prerender_seen_time_.is_null())
-    return false;
-  base::TimeDelta elapsed_time =
-      base::TimeTicks::Now() - last_prerender_seen_time_;
-  return elapsed_time <= base::TimeDelta::FromSeconds(kWindowDurationSeconds);
-}
-
 bool PrerenderManager::DoesRateLimitAllowPrerender() const {
   DCHECK(CalledOnValidThread());
   base::TimeDelta elapsed_time =
       GetCurrentTimeTicks() - last_prerender_start_time_;
-  PREFIXED_HISTOGRAM(
-      UMA_HISTOGRAM_TIMES(
-          GetDefaultHistogramName("TimeBetweenPrerenderRequests"),
-          elapsed_time));
+  histograms_->RecordTimeBetweenPrerenderRequests(elapsed_time);
   if (!config_.rate_limit_enabled)
     return true;
   return elapsed_time >
@@ -1021,89 +939,10 @@
   DeletePendingDeleteEntries();
 }
 
-void PrerenderManager::RecordTimeUntilUsed(base::TimeDelta time_until_used) {
-  DCHECK(CalledOnValidThread());
-  PREFIXED_HISTOGRAM(UMA_HISTOGRAM_CUSTOM_TIMES(
-      GetDefaultHistogramName("TimeUntilUsed"),
-      time_until_used,
-      base::TimeDelta::FromMilliseconds(10),
-      config_.max_age,
-      50));
-}
-
 void PrerenderManager::RecordFinalStatus(Origin origin,
                                          uint8 experiment_id,
                                          FinalStatus final_status) const {
-  DCHECK(final_status != FINAL_STATUS_MAX);
-  // FINAL_STATUS_CONTROL_GROUP indicates that the PrerenderContents
-  // was created only to measure "would-have-been-prerendered" for
-  // control group measurements. Don't pollute data with it.
-  if (PrerenderManager::IsControlGroup() ||
-      final_status == FINAL_STATUS_CONTROL_GROUP)
-    return;
-  PREFIXED_HISTOGRAM_ORIGIN_EXPERIMENT(origin, experiment_id,
-                     UMA_HISTOGRAM_ENUMERATION(
-                         GetHistogramName(origin, experiment_id, "FinalStatus"),
-                         final_status,
-                         FINAL_STATUS_MAX));
+  histograms_->RecordFinalStatus(origin, experiment_id, final_status);
 }
 
-std::string PrerenderManager::ComposeHistogramName(
-    const std::string& prefix_type,
-    const std::string& name) const {
-  if (prefix_type.empty())
-    return std::string("Prerender.") + name;
-  return std::string("Prerender.") + prefix_type + std::string("_") + name;
-}
-
-std::string PrerenderManager::GetHistogramName(Origin origin,
-                                               uint8 experiment_id,
-                                               const std::string& name) const {
-  switch (origin) {
-    case ORIGIN_OMNIBOX:
-      if (experiment_id != kNoExperiment)
-        return ComposeHistogramName("wash", name);
-      return ComposeHistogramName("omnibox", name);
-    case ORIGIN_LINK_REL_PRERENDER:
-      if (experiment_id == kNoExperiment)
-        return ComposeHistogramName("", name);
-      return ComposeHistogramName("exp" + std::string(1, experiment_id + '0'),
-                                  name);
-    default:
-      NOTREACHED();
-      break;
-  };
-
-  // Dummy return value to make the compiler happy.
-  NOTREACHED();
-  return ComposeHistogramName("wash", name);
-}
-
-std::string PrerenderManager::GetDefaultHistogramName(
-    const std::string& name) const {
-  if (!WithinWindow())
-    return ComposeHistogramName("", name);
-  if (origin_experiment_wash_)
-    return ComposeHistogramName("wash", name);
-  return GetHistogramName(last_origin_, last_experiment_id_, name);
-}
-
-uint8 PrerenderManager::GetCurrentExperimentId() const {
-  if (!WithinWindow())
-    return kNoExperiment;
-  return last_experiment_id_;
-}
-
-Origin PrerenderManager::GetCurrentOrigin() const {
-  if (!WithinWindow())
-    return ORIGIN_LINK_REL_PRERENDER;
-  return last_origin_;
-}
-
-bool PrerenderManager::IsOriginExperimentWash() const {
-  if (!WithinWindow())
-    return false;
-  return origin_experiment_wash_;
-}
-
 }  // namespace prerender