Make a separate histogram for MatchComplete Final Status'es and

chrome/browser/prerender/prerender_contents.h

 // 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.
 
 #ifndef CHROME_BROWSER_PRERENDER_PRERENDER_CONTENTS_H_
 #define CHROME_BROWSER_PRERENDER_PRERENDER_CONTENTS_H_
 #pragma once
 
 #include <list>
 #include <string>
@@ skipping 63 matching lines @@
     PendingPrerenderData(Origin origin,
                          const GURL& url,
                          const content::Referrer& referrer);
 
     Origin origin;
     GURL url;
     content::Referrer referrer;
   };
   typedef std::list<PendingPrerenderData> PendingPrerenderList;
 
+  enum MatchCompleteStatus {

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

Needs a comment explaining what this is.

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

Done.

+    MC_DEFAULT,
+    MC_REPLACED,
+    MC_REPLACEMENT
+  };
+
   virtual ~PrerenderContents();
 
   bool Init();
 
   static Factory* CreateFactory();
 
   // |source_render_view_host| is the RenderViewHost that initiated
   // prerendering.
   virtual void StartPrerendering(
       const RenderViewHost* source_render_view_host,
       SessionStorageNamespace* session_storage_namespace);
 
   // Verifies that the prerendering is not using too many resources, and kills
   // it if not.
   void DestroyWhenUsingTooManyResources();
 
   RenderViewHost* render_view_host_mutable();
   const RenderViewHost* render_view_host() const;
   string16 title() const { return title_; }
   int32 page_id() const { return page_id_; }
   GURL icon_url() const { return icon_url_; }
   const GURL& prerender_url() const { return prerender_url_; }
   const content::Referrer& referrer() const { return referrer_; }
   bool has_stopped_loading() const { return has_stopped_loading_; }
   bool prerendering_has_started() const { return prerendering_has_started_; }
+  MatchCompleteStatus mc_status() const { return mc_status_; }

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

This is more of a style question: If the get and set are this simple and both
public, is there any reason not to just make the member public directly?

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

In general that's not the way things are usually done.  Unless you have an all
public struct, member variables should always be private, and provide
getters/setters.

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

+  void set_mc_status(MatchCompleteStatus status) { mc_status_ = status; }
 
   // Sets the parameter to the value of the associated RenderViewHost's child id
   // and returns a boolean indicating the validity of that id.
   virtual bool GetChildId(int* child_id) const;
 
   // Sets the parameter to the value of the associated RenderViewHost's route id
   // and returns a boolean indicating the validity of that id.
   virtual bool GetRouteId(int* route_id) const;
 
   // Set the final status for how the PrerenderContents was used. This
@@ skipping 143 matching lines @@
   std::vector<GURL> alias_urls_;
 
   bool has_stopped_loading_;
 
   // This must be the same value as the PrerenderTracker has recorded for
   // |this|, when |this| has a RenderView.
   FinalStatus final_status_;
 
   bool prerendering_has_started_;
 
+  MatchCompleteStatus mc_status_;

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

Needs a comment explaining what it is.

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

mc_status_ should be match_complete_status_

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

Done.

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

Since this is used so frequently, mc_status is much more
convenient, esp. for getters/setters.

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

+
   // Tracks whether or not prerendering has been cancelled by calling Destroy.
   // Used solely to prevent double deletion.
   bool prerendering_has_been_cancelled_;
 
   // Time at which we started to load the URL.  This is used to compute
   // the time elapsed from initiating a prerender until the time the
   // (potentially only partially) prerendered page is shown to the user.
   base::TimeTicks load_start_time_;
 
   // Process Metrics of the render process associated with the
@@ skipping 19 matching lines @@
 
   // List of all pages the prerendered page has tried to prerender.
   PendingPrerenderList pending_prerender_list_;
 
   DISALLOW_COPY_AND_ASSIGN(PrerenderContents);
 };
 
 }  // namespace prerender
 
 #endif  // CHROME_BROWSER_PRERENDER_PRERENDER_CONTENTS_H_