Make a separate histogram for MatchComplete Final Status'es and

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
===================================================================
--- chrome/browser/prerender/prerender_manager.cc	(revision 118385)
+++ chrome/browser/prerender/prerender_manager.cc	(working copy)
@@ -437,9 +437,7 @@
 
   prerender_list_.push_back(data);
 
-  if (IsControlGroup()) {
-    data.contents_->set_final_status(FINAL_STATUS_CONTROL_GROUP);
-  } else {
+  if (!IsControlGroup()) {
     last_prerender_start_time_ = GetCurrentTimeTicks();
     data.contents_->StartPrerendering(source_render_view_host,
                                       session_storage_namespace);
@@ -569,6 +567,7 @@
   // would be showing a prerendered contents, but otherwise, don't do anything.
   if (!prerender_contents->prerendering_has_started()) {

[dominich, 2012/01/20 21:17:08]

I'm a bit puzzled why this is a different check to IsControlGroup()

[tburkard, 2012/01/20 21:57:54]

This is necessary to also cover e.g. the case of a MatchComplete dummy. 
prerendering_has_started() is the easiest check to cover all these cases.  It is
also more robust in case one day, the field trial could change on the fly.

On 2012/01/20 21:17:08, dominich wrote:

     MarkWebContentsAsWouldBePrerendered(web_contents);
+    prerender_contents.release()->Destroy(FINAL_STATUS_USED);
     return false;
   }
 
@@ -576,6 +575,7 @@
   // See http://crbug.com/98541
   if (content::DevToolsAgentHostRegistry::IsDebuggerAttached(web_contents)) {
     prerender_contents.release()->Destroy(FINAL_STATUS_DEVTOOLS_ATTACHED);
+    // TODO: record MatchComplete status correctly.

[dominich, 2012/01/20 21:17:08]

If the current addition of the mc_status doesn't allow this to be tracked
correctly, maybe we need to consider an alternative solution for MatchComplete.

[tburkard, 2012/01/20 21:57:54]

It does suffice.. All that's needed is exercising it.  I updated that in this
latest revision, using a new helper function that conveniently takes care of
this at all three places that I marked as TODO.

On 2012/01/20 21:17:08, dominich wrote:

     return false;
   }
 
@@ -584,6 +584,7 @@
   if (prerender_contents->IsCrossSiteNavigationPending()) {
     prerender_contents.release()->Destroy(
         FINAL_STATUS_CROSS_SITE_NAVIGATION_PENDING);
+    // TODO: record MatchComplete status correctly.
     return false;
   }
 
@@ -598,6 +599,7 @@
       new_render_view_host->session_storage_namespace()) {
     prerender_contents.release()->Destroy(
         FINAL_STATUS_SESSION_STORAGE_NAMESPACE_MISMATCH);
+    // TODO: record MatchComplete status correctly.
     return false;
   }
 
@@ -705,13 +707,15 @@
       // for the Match Complete group.
       // This is the case if the cancellation is for any reason that would not
       // occur in the control group case.
-      if (NeedMatchCompleteDummyForFinalStatus(final_status)) {
+      if (entry->mc_status() == PrerenderContents::MC_DEFAULT &&
+          NeedMatchCompleteDummyForFinalStatus(final_status)) {
         // TODO(tburkard): I'd like to DCHECK that we are actually prerendering.
         // However, what if new conditions are added and
         // NeedMatchCompleteDummyForFinalStatus, is not being updated.  Not sure
         // what's the best thing to do here.  For now, I will just check whether
         // we are actually prerendering.
         if (ActuallyPrerendering()) {
+          entry->set_mc_status(PrerenderContents::MC_REPLACED);
           PrerenderContents* dummy_replacement_prerender_contents =
               CreatePrerenderContents(
                   entry->prerender_url(),
@@ -722,8 +726,9 @@
               dummy_replacement_prerender_contents->Init()) {
             dummy_replacement_prerender_contents->
                 AddAliasURLsFromOtherPrerenderContents(entry);
+            dummy_replacement_prerender_contents->set_mc_status(
+                PrerenderContents::MC_REPLACEMENT);
             it->contents_ = dummy_replacement_prerender_contents;
-            it->contents_->set_final_status(FINAL_STATUS_MATCH_COMPLETE_DUMMY);
             swapped_in_dummy_replacement = true;
           }
         }
@@ -1099,12 +1104,28 @@
   DeletePendingDeleteEntries();
 }
 
+void PrerenderManager::RecordFinalStatusWithMatchCompleteStatus(
+    Origin origin,
+    uint8 experiment_id,
+    PrerenderContents::MatchCompleteStatus mc_status,
+    FinalStatus final_status) const {

[dominich, 2012/01/20 21:17:08]

if mc_status is MC_DEFAULT this will be recorded in two histograms. Is that
expected?

[tburkard, 2012/01/20 21:57:54]

Yes.  There will be 2 histograms for the Prerender gorups only: The actual
FinalStatus'es as perceived by the user, and the MatchComplete FinalStatus'es,
that should line up with the control group.  Notice that the matchComplete final
status histograms will not be generated for the control groups or the no use
groups, so this is only adding a handful of new histograms.

On 2012/01/20 21:17:08, dominich wrote:

+  if (mc_status != PrerenderContents::MC_REPLACEMENT)
+    histograms_->RecordFinalStatus(origin, experiment_id, final_status);
+  if (mc_status != PrerenderContents::MC_REPLACED) {
+    histograms_->RecordMatchCompleteFinalStatus(origin, experiment_id,
+                                                final_status);
+  }
+}
+
 void PrerenderManager::RecordFinalStatus(Origin origin,
                                          uint8 experiment_id,
                                          FinalStatus final_status) const {
-  histograms_->RecordFinalStatus(origin, experiment_id, final_status);
+  RecordFinalStatusWithMatchCompleteStatus(origin, experiment_id,
+                                           PrerenderContents::MC_DEFAULT,
+                                           final_status);
 }
 
+
 PrerenderManager* FindPrerenderManagerUsingRenderProcessId(
     int render_process_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));