Add code to keep track of what fraction of pageviews are top sites.

Add code to keep track of what fraction of pageviews are top sites.
This code will be removed again once histograms have been collected (prior
to the M15 branchpoint).
R=cbentzel@chromium.org, dominich@chromium.org
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98131

[dominich, 2011-08-24 21:37:49]

Don't forget to add the new histograms.

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:175: explicit
MostVisitedSites(Profile* profile) :
nit: these method definitions should be moved out of the class definition.

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:201: if (!urls_.count(url))
You're using a set so you don't need this check.

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:214: return urls_.count(url);
nit: return (urls_.count(url) > 0);

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_observer.cc (right):

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_observer.cc:196:
RecordPageviewEvent(PAGEVIEW_EVENT_NEW_URL);
This will record a new url event even if prerender_manager is NULL but the top
site event won't be recorded in that case. It seems that we'd overcount as we'd
include new url events from sessions that could never have a top site event.

[tburkard, 2011-08-24 21:45:47]

I will update the histograms in a separate CL.

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:175: explicit
MostVisitedSites(Profile* profile) :
Since this is temporary ill keep this for now.
On 2011/08/24 21:37:49, dominich wrote:
> nit: these method definitions should be moved out of the class definition.

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:201: if (!urls_.count(url))
On 2011/08/24 21:37:49, dominich wrote:
> You're using a set so you don't need this check.

Done.

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:214: return urls_.count(url);
On 2011/08/24 21:37:49, dominich wrote:
> nit: return (urls_.count(url) > 0);

Done.

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_observer.cc (right):

http://codereview.chromium.org/7728004/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_observer.cc:196:
RecordPageviewEvent(PAGEVIEW_EVENT_NEW_URL);
This is intentional.  The scenario is an incognito window.  Those are
pageviews..  The idea is, how many pageviews could have been prerendered.  In
Incognito, that's not possible, but they are still pageviews.

On 2011/08/24 21:37:49, dominich wrote:
> This will record a new url event even if prerender_manager is NULL but the top
> site event won't be recorded in that case. It seems that we'd overcount as
we'd
> include new url events from sessions that could never have a top site event.

[dominich, 2011-08-24 22:22:12]

LGTM++

http://codereview.chromium.org/7728004/diff/3002/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/7728004/diff/3002/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_manager.cc:210: bool IsTopSite(const GURL&
url) {
nit: method could be const

http://codereview.chromium.org/7728004/diff/3002/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_manager.cc:221: bool
PrerenderManager::IsTopSite(const GURL& url) {
nit: method could be const.

http://codereview.chromium.org/7728004/diff/3002/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_observer.cc (right):

http://codereview.chromium.org/7728004/diff/3002/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_observer.cc:326: bool
PrerenderObserver::IsTopSite(const GURL& url) {
nit: method could be const.