Make a separate histogram for MatchComplete Final Status'es and

chrome/browser/prerender/prerender_histograms.cc

Index: chrome/browser/prerender/prerender_histograms.cc
===================================================================
--- chrome/browser/prerender/prerender_histograms.cc	(revision 118385)
+++ chrome/browser/prerender/prerender_histograms.cc	(working copy)
@@ -169,6 +169,26 @@
         100)); \
 }
 
+// Summary of all histograms Perceived PLT histograms:
+// (all suffix PerceivedPLT)

[dominich, 2012/01/20 22:23:37]

itym prefix

+// PerceivedPLT -- Perceived Pageloadtimes (PPLT) for all pages in the group.
+// ...Windowed -- PPLT for pages in the 30s after a prerender.
+// ...Matched -- A prerendered page that was swapped in.
+// ...MatchedComplete -- A prerendered page that was swapped in + a few
+// that were not swapped in so that the set of pages lines up more closely with

[dominich, 2012/01/20 22:23:37]

what few that were not swapped in are included? What does it mean to have
...MatchedComplete...Control vs ...MatchedComplete...Experiment? Is there ever a
time to compare ...Matched...Control with ...Matched...Experiment? If not, maybe
we should only output both Control and Experiment for ...MatchedComplete?

[tburkard, 2012/01/20 23:23:00]

Coalescing histograms is unrelated to this change -- so let's discuss is
separately and make it a separate change.

On 2012/01/20 22:23:37, dominich wrote:

+// the control group.
+// ...FirstAfterMiss -- First page to finish loading after a prerender, which

[dominich, 2012/01/20 22:23:37]

I can see that these might be useful, but when we do our analysis which are
actually used and can we drop some of them? 

[tburkard, 2012/01/20 23:23:00]

Coalescing histograms is unrelated to this change -- so let's discuss is
separately and make it a separate change.

On 2012/01/20 22:23:37, dominich wrote:

+// is different from the page that was prerendered.
+// ...FirstAfterMissNonOverlapping -- Same as FirstAfterMiss, but only
+// triggering for the first page to finish after the prerender that also started
+// after the prerender started.
+// ...FirstAfterMissBoth -- pages meeting
+// FirstAfterMiss AND FirstAfterMissNonOverlapping
+// ...FirstAfterMissAnyOnly -- pages meeting
+// FirstAfterMiss but NOT FirstAfterMissNonOverlapping
+// ..FirstAfterMissNonOverlappingOnly -- pages meeting
+// FirstAfterMissNonOverlapping but NOT FirstAfterMiss

[dominich, 2012/01/20 22:23:37]

Can these be compared between Control, Experiment, and NoUse? Do these have both
...Matched... and ...MatchedComplete...? If not, why not?

[tburkard, 2012/01/20 23:23:00]

Yes, they appear in all variants, which is implied by not specifying it.  I
think the definition here + the definition of the field trials make it clear
what is being recorded.  I dont want to provide additional info on possible use
cases in comments here.
On 2012/01/20 22:23:37, dominich wrote:

+
 void PrerenderHistograms::RecordPerceivedPageLoadTime(
     base::TimeDelta perceived_page_load_time, bool was_prerender,
     bool was_complete_prerender, const GURL& url) {
@@ -266,6 +286,17 @@
       UMA_HISTOGRAM_ENUMERATION(name, final_status, FINAL_STATUS_MAX));
 }
 
+void PrerenderHistograms::RecordMatchCompleteFinalStatus(

[dominich, 2012/01/20 22:23:37]

nit: If the histogram is FinalStatusMatchComplete then the method should be
RecordFinalStatusMatchComplete.

[tburkard, 2012/01/20 23:23:00]

Done.

+    Origin origin,
+    uint8 experiment_id,
+    FinalStatus final_status) const {
+  DCHECK(final_status != FINAL_STATUS_MAX);
+  PREFIXED_HISTOGRAM_ORIGIN_EXPERIMENT(
+      base::FieldTrial::MakeName("FinalStatusMatchComplete", "Prerender"),
+      origin, experiment_id,
+      UMA_HISTOGRAM_ENUMERATION(name, final_status, FINAL_STATUS_MAX));
+}
+
 uint8 PrerenderHistograms::GetCurrentExperimentId() const {
   if (!WithinWindow())
     return kNoExperiment;