Make a separate histogram for MatchComplete Final Status'es and

chrome/browser/prerender/prerender_contents.cc

Index: chrome/browser/prerender/prerender_contents.cc
===================================================================
--- chrome/browser/prerender/prerender_contents.cc	(revision 118385)
+++ chrome/browser/prerender/prerender_contents.cc	(working copy)
@@ -222,6 +222,7 @@
       has_stopped_loading_(false),
       final_status_(FINAL_STATUS_MAX),
       prerendering_has_started_(false),
+      mc_status_(MC_DEFAULT),
       prerendering_has_been_cancelled_(false),
       child_id_(-1),
       route_id_(-1),
@@ -344,34 +345,22 @@
 
 void PrerenderContents::set_final_status(FinalStatus final_status) {
   DCHECK(final_status >= FINAL_STATUS_USED && final_status < FINAL_STATUS_MAX);
-  DCHECK(final_status_ == FINAL_STATUS_MAX ||
-         final_status_ == FINAL_STATUS_CONTROL_GROUP ||
-         final_status_ == FINAL_STATUS_MATCH_COMPLETE_DUMMY);
+  DCHECK(final_status_ == FINAL_STATUS_MAX);

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

This is SO much cleaner and better. :)

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

Done.

 
-  // Don't override final_status_ if it's FINAL_STATUS_CONTROL_GROUP or
-  // FINAL_STATUS_MATCH_COMPLETE_DUMMY, otherwise data will be collected
-  // in the Prerender.FinalStatus histogram.
-  if (final_status_ == FINAL_STATUS_CONTROL_GROUP ||
-      final_status_ == FINAL_STATUS_MATCH_COMPLETE_DUMMY)
-    return;
-
   final_status_ = final_status;
 }
 
 PrerenderContents::~PrerenderContents() {
   DCHECK(final_status_ != FINAL_STATUS_MAX);
   DCHECK(prerendering_has_been_cancelled_ ||
-         final_status_ == FINAL_STATUS_USED ||
-         final_status_ == FINAL_STATUS_CONTROL_GROUP ||
-         final_status_ == FINAL_STATUS_MATCH_COMPLETE_DUMMY);
+         final_status_ == FINAL_STATUS_USED);
   DCHECK(origin_ != ORIGIN_MAX);
 
-  // If we haven't even started prerendering, we were just in the control
-  // group (or a match complete dummy), which means we do not want to record
-  // the status.
-  if (prerendering_has_started())
-    prerender_manager_->RecordFinalStatus(origin_, experiment_id_,
-                                          final_status_);
+  prerender_manager_->RecordFinalStatusWithMatchCompleteStatus(

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

Is RecordFinalStatus ever called directly? If not, maybe make this
RecordFinalStatus (taking the mc_status) to simplify the API.

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

Yes, RecordFinalStatus is called directly many times from within
PrerenderManager, and mentioning PrerenderContents::MC_DEFAULT every time is
tedious.

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

+      origin_,
+      experiment_id_,
+      mc_status_,
+      final_status_);
 
   if (child_id_ != -1 && route_id_ != -1)
     prerender_tracker_->OnPrerenderingFinished(child_id_, route_id_);