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 32 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);
     case ORIGIN_GWS_PRERENDER:  // Handled above.
     default:
       NOTREACHED();
       break;
   };
 
   // Dummy return value to make the compiler happy.
   NOTREACHED();
   return ComposeHistogramName("wash", name);
 }
@@ skipping 29 matching lines @@
   std::string name = GetHistogramName(origin, experiment, wash, \
                                       histogram_name); \
   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) { \
+  } else if (origin == ORIGIN_OMNIBOX) { \
     HISTOGRAM; \
-  } else if (origin == ORIGIN_OMNIBOX) { \
+  } 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_LINK_REL_PRERENDER_CROSSDOMAIN),
       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 256 matching lines @@
 }
 
 uint8 PrerenderHistograms::GetCurrentExperimentId() const {
   if (!WithinWindow())
     return kNoExperiment;
   return last_experiment_id_;
 }
 
 Origin PrerenderHistograms::GetCurrentOrigin() const {
   if (!WithinWindow())
-    return ORIGIN_LINK_REL_PRERENDER;
+    return ORIGIN_LINK_REL_PRERENDER_CROSSDOMAIN;

[dominich, 2012/09/13 20:03:36]

why is this the default here?

[gavinp, 2012/09/14 02:00:42]

Why was ORIGIN_LINK_REL_PRERENDER the default before?

[dominich, 2012/09/14 14:40:45]

I have no idea, but it seems very wrong to me. We probably don't have a
last_origin_ if we're within window, but we should then have a 'current origin'.
I think this is a bug, and I think we should fix it, given that we're now
looking to analyze the same/cross domain delta.

[gavinp, 2012/09/14 21:10:47]

I'm adding Timo to this review to explain, since I couldn't understand this code
when I read it. Timo, what should we do here?

On 2012/09/14 14:40:45, dominich wrote:

[mmenke, 2012/09/17 20:39:27]

I'm not Timo (Though I've always wanted to play him on TV), but I think now that
we allow multiple prerenders, this may be a rather dysfunctional function.  Some
places, at least, we can get the relevant origin directly from the prerender.

[gavinp, 2012/09/20 04:18:10]

Oh boy, I've now reviewed this code, and these histograms are really broken
right now, since this entire file operates using what amounts to global
variables.

I'm in the process of fixing them, but there's ten distinct PLTs in
"FirstFooAfterBarWithExtraQuux"...  I'm trying to decide how much to preserve
and how much to shred.

Timo, how many of these are you using? Which ones are most interesting?

   return last_origin_;
 }
 
 bool PrerenderHistograms::IsOriginExperimentWash() const {
   if (!WithinWindow())
     return false;
   return origin_experiment_wash_;
 }
 
 void PrerenderHistograms::RecordLocalPredictorEvent(
@@ skipping 11 matching lines @@
       "LocalPredictorTimeUntilUsed",
       UMA_HISTOGRAM_CUSTOM_TIMES(
           name,
           time_until_used,
           base::TimeDelta::FromMilliseconds(10),
           max_age,
           50));
 }
 
 }  // namespace prerender