Separate same domain and cross domain <link rel=...> prerenders for reporting.

chrome/browser/prerender/prerender_histograms.cc

 // Copyright (c) 2012 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 "chrome/browser/prerender/prerender_histograms.h"
 
 #include <string>
 
 #include "base/format_macros.h"
 #include "base/metrics/field_trial.h"
@@ skipping 34 matching lines @@
     return ComposeHistogramName("exp" + std::string(1, experiment_id + '0'),
                                 name);
   }
 
   if (experiment_id != kNoExperiment)
     return ComposeHistogramName("wash", name);
 
   switch (origin) {
     case ORIGIN_OMNIBOX:
       return ComposeHistogramName("omnibox", name);
-    case ORIGIN_LINK_REL_PRERENDER:
-      return ComposeHistogramName("web", name);
+    case ORIGIN_LINK_REL_PRERENDER_SAMEDOMAIN:
+      return ComposeHistogramName("websame", name);
+    case ORIGIN_LINK_REL_PRERENDER_CROSSDOMAIN:
+      return ComposeHistogramName("webcross", name);

[mmenke, 2012/10/17 15:12:16]

nit:  Move these after ORIGIN_NONE, to match declaration order, and the order in
PREFIXED_HISTOGRAM_INTERNAL.

[gavinp, 2012/10/17 20:57:49]

Done.

     case ORIGIN_NONE:
       return ComposeHistogramName("none", name);
     case ORIGIN_GWS_PRERENDER:  // Handled above.
     default:
       NOTREACHED();
       break;
   };
 
   // Dummy return value to make the compiler happy.
   NOTREACHED();
@@ skipping 22 matching lines @@
 
 #define PREFIXED_HISTOGRAM_INTERNAL(origin, experiment, wash, HISTOGRAM, \
                                     histogram_name) { \
   { \
     /* Do not rename.  HISTOGRAM expects a local variable "name". */ \
     std::string name = ComposeHistogramName("", histogram_name); \
     HISTOGRAM; \
   } \
   /* Do not rename.  HISTOGRAM expects a local variable "name". */ \
   std::string name = GetHistogramName(origin, experiment, wash, \
                                       histogram_name); \

[mmenke, 2012/10/17 15:12:16]

Would you mind just inlining GetHistogramName into this function?  Having two
functions that have to always have the exact same switch statement just seems a
bad idea.  Feel free to do it in another CL (Or not at all - I'm not going to
block you on it).

[gavinp, 2012/10/17 20:57:49]

I have a lot of refactoring I could imagine happening here, so I'm going to hold
off on that, at least until our discussion about how much of this complexity we
want runs out. So thanks for not blocking me, and please feel free to nag me if
we work in this area again...

   static uint8 recording_experiment = kNoExperiment; \
   if (recording_experiment == kNoExperiment && experiment != kNoExperiment) \
     recording_experiment = experiment; \
   if (wash) { \
     HISTOGRAM; \
   } else if (experiment != kNoExperiment && \
              (origin != ORIGIN_GWS_PRERENDER || \
               experiment != recording_experiment)) { \
-  } else if (origin == ORIGIN_LINK_REL_PRERENDER) { \
-    HISTOGRAM; \
   } else if (origin == ORIGIN_OMNIBOX) { \
     HISTOGRAM; \
   } else if (origin == ORIGIN_NONE) { \
     HISTOGRAM; \
+  } else if (origin == ORIGIN_LINK_REL_PRERENDER_SAMEDOMAIN) { \
+    HISTOGRAM; \
+  } else if (origin == ORIGIN_LINK_REL_PRERENDER_CROSSDOMAIN) { \
+    HISTOGRAM; \
   } else if (experiment != kNoExperiment) { \
     HISTOGRAM; \
   } else { \
     HISTOGRAM; \
   } \
 }
 
 PrerenderHistograms::PrerenderHistograms()
     : last_experiment_id_(kNoExperiment),
-      last_origin_(ORIGIN_LINK_REL_PRERENDER),
+      last_origin_(ORIGIN_MAX),
       origin_experiment_wash_(false),
       seen_any_pageload_(true),
       seen_pageload_started_after_prerender_(true) {
 }
 
 void PrerenderHistograms::RecordPrerender(Origin origin, const GURL& url) {
   // Check if we are doing an experiment.
   uint8 experiment = GetQueryStringBasedExperiment(url);
 
   // We need to update last_experiment_id_, last_origin_, and
@@ skipping 253 matching lines @@
   return last_experiment_id_;
 }
 
 bool PrerenderHistograms::IsOriginExperimentWash() const {
   if (!WithinWindow())
     return false;
   return origin_experiment_wash_;
 }
 
 }  // namespace prerender