Make a separate histogram for MatchComplete Final Status'es and

chrome/browser/prerender/prerender_contents.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_contents.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/process_util.h"
@@ skipping 204 matching lines @@
     uint8 experiment_id)
     : prerender_manager_(prerender_manager),
       prerender_tracker_(prerender_tracker),
       prerender_url_(url),
       referrer_(referrer),
       profile_(profile),
       page_id_(0),
       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),
       origin_(origin),
       experiment_id_(experiment_id) {
   DCHECK(prerender_manager != NULL);
 }
 
 bool PrerenderContents::Init() {
   return AddAliasURL(prerender_url_);
@@ skipping 102 matching lines @@
 
 bool PrerenderContents::GetRouteId(int* route_id) const {
   CHECK(route_id);
   DCHECK_GE(route_id_, -1);
   *route_id = route_id_;
   return route_id_ != -1;
 }
 
 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);
-
-  // 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;
+  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.

 
   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_);
 
   // If we still have a TabContents, clean up anything we need to and then
   // destroy it.
   if (prerender_contents_.get())
     delete ReleasePrerenderContents();
 
   // The following URLs are no longer rendering.
@@ skipping 276 matching lines @@
 bool PrerenderContents::IsCrossSiteNavigationPending() const {
   if (!prerender_contents_.get() || !prerender_contents_->web_contents())
     return false;
   const WebContents* web_contents = prerender_contents_->web_contents();
   return (web_contents->GetSiteInstance() !=
           web_contents->GetPendingSiteInstance());
 }
 
 
 }  // namespace prerender