Fix prerender histograms for multiple prerender case.

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index 2282f21a288c64f61f39568e07796d8d169d78f2..fb24531d9df91aafc14c1437451296e2230f587b 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -165,14 +165,19 @@ PrerenderManager::PrerenderManagerMode PrerenderManager::mode_ =
     PRERENDER_MODE_ENABLED;
 
 struct PrerenderManager::NavigationRecord {
-  GURL url_;
-  base::TimeTicks time_;
+  GURL url;
+  base::TimeTicks time;
+
   NavigationRecord(const GURL& url, base::TimeTicks time)
-      : url_(url),
-        time_(time) {
+      : url(url),
+        time(time) {
   }
 };
 
+PrerenderManager::PrerenderedWebContentsData::
+PrerenderedWebContentsData(Origin origin) : origin(origin) {
+}
+
 PrerenderManager::PrerenderManager(Profile* profile,
                                    PrerenderTracker* prerender_tracker)
     : enabled_(true),
@@ -182,8 +187,7 @@ PrerenderManager::PrerenderManager(Profile* profile,
       last_prerender_start_time_(GetCurrentTimeTicks() -
           base::TimeDelta::FromMilliseconds(kMinTimeBetweenPrerendersMs)),
       weak_factory_(this),
-      prerender_history_(new PrerenderHistory(kHistoryLength)),
-      histograms_(new PrerenderHistograms()) {
+      prerender_history_(new PrerenderHistory(kHistoryLength)) {
   // There are some assumptions that the PrerenderManager is on the UI thread.
   // Any other checks simply make sure that the PrerenderManager is accessed on
   // the same thread that it was created on.
@@ -200,6 +204,8 @@ PrerenderManager::PrerenderManager(Profile* profile,
     default:
       break;
   }
+
+  histograms_.reset(new PrerenderHistograms(config_.time_to_live));
 }
 
 PrerenderManager::~PrerenderManager() {
@@ -232,7 +238,7 @@ PrerenderHandle* PrerenderManager::AddPrerenderFromLinkRelPrerender(
           new PrerenderHandle(pending_prerender_list_.back().get());
       contents->AddPendingPrerender(
           prerender_handle->weak_ptr_factory_.GetWeakPtr(),
-          url, referrer, size);
+          ORIGIN_LINK_REL_PRERENDER, url, referrer, size);
       return prerender_handle;
     }
   }
@@ -372,11 +378,13 @@ bool PrerenderManager::MaybeUsePrerenderedPage(WebContents* web_contents,
 
   if (!prerender_contents->load_start_time().is_null()) {
     histograms_->RecordTimeUntilUsed(
+        prerender_contents->origin(),
         GetCurrentTimeTicks() - prerender_contents->load_start_time(),
         config_.time_to_live);
   }
 
-  histograms_->RecordPerSessionCount(++prerenders_per_session_count_);
+  histograms_->RecordPerSessionCount(prerender_contents->origin(),
+                                     ++prerenders_per_session_count_);
   histograms_->RecordUsedPrerender(prerender_contents->origin());
   prerender_contents->set_final_status(FINAL_STATUS_USED);
 
@@ -397,7 +405,8 @@ bool PrerenderManager::MaybeUsePrerenderedPage(WebContents* web_contents,
   DCHECK(new_tab_contents);
   DCHECK(old_tab_contents);
 
-  MarkWebContentsAsPrerendered(new_tab_contents->web_contents());
+  MarkWebContentsAsPrerendered(new_tab_contents->web_contents(),
+                               prerender_contents->origin());
 
   // Merge the browsing history.
   new_tab_contents->web_contents()->GetController().CopyStateFromAndPrune(
@@ -511,18 +520,23 @@ void PrerenderManager::RecordPerceivedPageLoadTime(
     return;
   if (!prerender_manager->IsEnabled())
     return;
+  Origin prerender_origin = ORIGIN_NONE;
   bool was_prerender =
-      prerender_manager->IsWebContentsPrerendered(web_contents);
+      prerender_manager->IsWebContentsPrerendered(web_contents,
+                                                  &prerender_origin);
   bool was_complete_prerender = was_prerender ||
       prerender_manager->WouldWebContentsBePrerendered(web_contents);

[tburkard, 2012/10/05 21:54:04]

WouldWebContentsBePrerendered must pass back the origin (see my comment about
that in the .h file). If IsWebContentsPrerendered returned false, then
WouldWebContentsBePrerendered must be called, and the appropriate origin must be
retrieved (rather than the ORIGIN_NONE that the false caused to be stored in
prerender_origin).

[gavinp, 2012/10/10 18:14:56]

Done.

   if (prerender_manager->IsWebContentsPrerendering(web_contents)) {
     prerender_manager->histograms_->RecordPageLoadTimeNotSwappedIn(
-        perceived_page_load_time, url);
+        prerender_origin, perceived_page_load_time, url);

[tburkard, 2012/10/05 21:54:04]

prerender_origin is not correct here: if it is currently prerendering,
IsWebContentsPrerendered should return false, but IsWebContentsPrerendering
would return true.  Could you verify that's the case.  I think the way to fix
this is to augment IsWebContentsPrerendering to pass back the origin, just like
IsWebContenstPrerendered did.

[gavinp, 2012/10/10 18:14:56]

Done.

Good catch. This would have broken stats on all currently running prerenders if
not fixed.

   } else {
     prerender_manager->histograms_->RecordPerceivedPageLoadTime(
-        perceived_page_load_time, was_prerender, was_complete_prerender, url);
-    prerender_manager->histograms_->RecordPercentLoadDoneAtSwapin(
-        fraction_plt_elapsed_at_swap_in);
+        prerender_origin, perceived_page_load_time, was_prerender,
+        was_complete_prerender, url);
+    if (was_prerender) {
+      prerender_manager->histograms_->RecordPercentLoadDoneAtSwapin(
+          prerender_origin, fraction_plt_elapsed_at_swap_in);
+    }
     if (prerender_manager->local_predictor_.get()) {
       prerender_manager->local_predictor_->
           OnPLTEventForURL(url, perceived_page_load_time);
@@ -530,6 +544,14 @@ void PrerenderManager::RecordPerceivedPageLoadTime(
   }
 }
 
+void PrerenderManager::RecordFractionPixelsFinalAtSwapin(
+    content::WebContents* web_contents,
+    double fraction) {
+  Origin origin = ORIGIN_NONE;
+  IsWebContentsPrerendered(web_contents, &origin);
+  histograms_->RecordFractionPixelsFinalAtSwapin(origin, fraction);
+}
+
 void PrerenderManager::set_enabled(bool enabled) {
   DCHECK(CalledOnValidThread());
   enabled_ = enabled;
@@ -636,9 +658,13 @@ PrerenderContents* PrerenderManager::GetPrerenderContents(
   return NULL;
 }
 
-void PrerenderManager::MarkWebContentsAsPrerendered(WebContents* web_contents) {
+void PrerenderManager::MarkWebContentsAsPrerendered(WebContents* web_contents,
+                                                    Origin origin) {
   DCHECK(CalledOnValidThread());
-  prerendered_tab_contents_set_.insert(web_contents);
+  prerendered_web_contents_data_.insert(
+      base::hash_map<content::WebContents*,
+                     PrerenderedWebContentsData>::value_type(
+                         web_contents, PrerenderedWebContentsData(origin)));
 }
 
 void PrerenderManager::MarkWebContentsAsWouldBePrerendered(
@@ -650,7 +676,7 @@ void PrerenderManager::MarkWebContentsAsWouldBePrerendered(
 void PrerenderManager::MarkWebContentsAsNotPrerendered(
     WebContents* web_contents) {
   DCHECK(CalledOnValidThread());
-  prerendered_tab_contents_set_.erase(web_contents);
+  prerendered_web_contents_data_.erase(web_contents);
   WouldBePrerenderedMap::iterator it =
       would_be_prerendered_map_.find(web_contents);
   if (it != would_be_prerendered_map_.end()) {
@@ -664,9 +690,19 @@ void PrerenderManager::MarkWebContentsAsNotPrerendered(
 }
 
 bool PrerenderManager::IsWebContentsPrerendered(
-    content::WebContents* web_contents) const {
+    content::WebContents* web_contents,
+    Origin* origin) const {
   DCHECK(CalledOnValidThread());
-  return prerendered_tab_contents_set_.count(web_contents) > 0;
+  base::hash_map<content::WebContents*, PrerenderedWebContentsData>::
+      const_iterator it = prerendered_web_contents_data_.find(web_contents);
+  if (it == prerendered_web_contents_data_.end()) {
+    if (origin)
+      *origin = ORIGIN_NONE;
+    return false;
+  }
+  if (origin)
+    *origin = it->second.origin;
+  return true;
 }
 
 bool PrerenderManager::WouldWebContentsBePrerendered(
@@ -675,7 +711,8 @@ bool PrerenderManager::WouldWebContentsBePrerendered(
   return would_be_prerendered_map_.count(web_contents) > 0;
 }
 
-bool PrerenderManager::HasRecentlyBeenNavigatedTo(const GURL& url) {
+bool PrerenderManager::HasRecentlyBeenNavigatedTo(Origin origin,
+                                                  const GURL& url) {
   DCHECK(CalledOnValidThread());
 
   CleanUpOldNavigations();
@@ -684,9 +721,9 @@ bool PrerenderManager::HasRecentlyBeenNavigatedTo(const GURL& url) {
            navigations_.rbegin();
        it != end;
        ++it) {
-    if (it->url_ == url) {
-      base::TimeDelta delta = GetCurrentTimeTicks() - it->time_;
-      histograms_->RecordTimeSinceLastRecentVisit(delta);
+    if (it->url == url) {
+      base::TimeDelta delta = GetCurrentTimeTicks() - it->time;
+      histograms_->RecordTimeSinceLastRecentVisit(origin, delta);
       return true;
     }
   }
@@ -968,7 +1005,7 @@ PrerenderHandle* PrerenderManager::AddPrerender(
 #endif
 
   // Check if enough time has passed since the last prerender.
-  if (!DoesRateLimitAllowPrerender()) {
+  if (!DoesRateLimitAllowPrerender(origin)) {
     // Cancel the prerender. We could add it to the pending prerender list but
     // this doesn't make sense as the next prerender request will be triggered
     // by a navigation and is unlikely to be the same site.
@@ -1171,11 +1208,11 @@ PrerenderManager::FindIteratorForPrerenderContents(
   return active_prerender_list_.end();
 }
 
-bool PrerenderManager::DoesRateLimitAllowPrerender() const {
+bool PrerenderManager::DoesRateLimitAllowPrerender(Origin origin) const {
   DCHECK(CalledOnValidThread());
   base::TimeDelta elapsed_time =
       GetCurrentTimeTicks() - last_prerender_start_time_;
-  histograms_->RecordTimeBetweenPrerenderRequests(elapsed_time);
+  histograms_->RecordTimeBetweenPrerenderRequests(origin, elapsed_time);
   if (!config_.rate_limit_enabled)
     return true;
   return elapsed_time >
@@ -1198,7 +1235,7 @@ void PrerenderManager::CleanUpOldNavigations() {
   base::TimeTicks cutoff = GetCurrentTimeTicks() -
       base::TimeDelta::FromMilliseconds(kNavigationRecordWindowMs);
   while (!navigations_.empty()) {
-    if (navigations_.front().time_ > cutoff)
+    if (navigations_.front().time > cutoff)
       break;
     navigations_.pop_front();
   }