[NoStatePrefetch] Add performance histograms.

chrome/browser/prerender/prerender_manager.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_manager.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <functional>
@@ skipping 127 matching lines @@
 };
 
 // static
 int PrerenderManager::prerenders_per_session_count_ = 0;
 
 // static
 PrerenderManager::PrerenderManagerMode PrerenderManager::mode_ =
     PRERENDER_MODE_ENABLED;
 
 struct PrerenderManager::NavigationRecord {
-  NavigationRecord(const GURL& url, base::TimeTicks time)
-      : url(url),
-        time(time) {
-  }
+  NavigationRecord(const GURL& url, base::TimeTicks time, Origin origin)
+      : url(url), time(time), origin(origin) {}
 
   GURL url;
   base::TimeTicks time;
+  Origin origin;
 };
 
 PrerenderManager::PrerenderManager(Profile* profile)
     : profile_(profile),
       prerender_contents_factory_(PrerenderContents::CreateFactory()),
       last_prerender_start_time_(GetCurrentTimeTicks() -
           base::TimeDelta::FromMilliseconds(kMinTimeBetweenPrerendersMs)),
       prerender_history_(new PrerenderHistory(kHistoryLength)),
       histograms_(new PrerenderHistograms()),
       profile_network_bytes_(0),
@@ skipping 394 matching lines @@
                                               is_redirect, is_no_store);
 }
 
 void PrerenderManager::RecordPrefetchRedirectCount(Origin origin,
                                                    bool is_main_resource,
                                                    int redirect_count) {
   histograms_->RecordPrefetchRedirectCount(origin, is_main_resource,
                                            redirect_count);
 }
 
+void PrerenderManager::RecordFirstContentfulPaint(const GURL& url,
+                                                  bool is_no_store,
+                                                  base::TimeDelta time) {

[mmenke, 2016/09/16 16:53:48]

Is this called on prefetches themselves?  That is, if we prefetch a URL twice in
a row (The second one after the old one is destroyed), will the second one
result in calling this method for the abbreviated prefetched page?

[droger, 2016/09/19 09:55:23]

No, this is not called on the prefetch themselves.

[mmenke, 2016/09/19 15:43:46]

Why isn't it?  Prefetches don't have first contentful paints?

[droger, 2016/09/19 15:52:55]

For two reasons:
- prefetches are never rendered, and thus never painted
- the caller (PageLoadMetricsObserver) is disabled for prerender/prefetch loads.

+  CleanUpOldNavigations(&prefetches_, base::TimeDelta::FromMinutes(30));
+
+  // Compute the prefetch age.
+  base::TimeDelta prefetch_age;
+  Origin origin = ORIGIN_NONE;
+  for (auto it = prefetches_.crbegin(); it != prefetches_.crend(); ++it) {
+    if (it->url == url) {
+      prefetch_age = GetCurrentTimeTicks() - it->time;
+      origin = it->origin;
+      break;
+    }
+  }
+
+  histograms_->RecordFirstContentfulPaint(origin, is_no_store, time,

[mmenke, 2016/09/16 16:53:48]

What do you plan to do with these numbers?  If we're just going to compare them
to PLT of random non-prefetched URLs, that probably won't work well - there's a
strong bias in the type of pages that are prerendered (landing pages, mostly).

[droger, 2016/09/19 09:55:23]

Very good point.

I know we have plans to do UMA experiments where we don't actually prefetch
pages but record the ones that would have been prefetched (similar to the old
experiments for prerendering).

Doing this should enable comparisons between prefetched page and
"would-have-been-prefetched" page.

However, your concern is still valid. In particular it's not immediately clear
that the "Reference" histograms (recorded for pages that are not prefetched)
will ever be useful.

I can probably remove this particular histogram if this is a problem.

[mmenke, 2016/09/19 15:43:46]

I'm happy to defer to you on whether we should add this histogram, I just want
you to be aware that you can't really compare it to other ContentfulPaint
histograms.  This bias may be worth mentioning on the histogram description, at
least.

We've previously addressed this by making a group that "pretends" to prerender
pages, but doesn't really, and then comparing it with this.  In the prerender
case, we had trouble with cancellation reasons not being accounted for, but if
we did something similar for prefetch, I don't think we'd have similar issues,
since we're including prefetches that are cancelled early in prefetches. 
Anyhow, just throwing the idea out there.  I don't think we'd want to keep that
group around indefinitely, if we add it.

[droger, 2016/09/19 15:52:55]

Yes, as far as I know we have the same plan for prefetch.

+                                          prefetch_age);
+
+  // Loading a prefetched URL resets the revalidation bypass. Remove the url
+  // from the prefetch list for more accurate metrics.
+  prefetches_.erase(
+      std::remove_if(prefetches_.begin(), prefetches_.end(),
+                     [url](const NavigationRecord& r) { return r.url == url; }),
+      prefetches_.end());
+}
+
 // static
 PrerenderManager::PrerenderManagerMode PrerenderManager::GetMode() {
   return mode_;
 }
 
 // static
 void PrerenderManager::SetMode(PrerenderManagerMode mode) {
   mode_ = mode;
 }
 
@@ skipping 106 matching lines @@
     }
   }
 
   return result;
 }
 
 bool PrerenderManager::HasRecentlyBeenNavigatedTo(Origin origin,
                                                   const GURL& url) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  CleanUpOldNavigations();
+  CleanUpOldNavigations(&navigations_, base::TimeDelta::FromMilliseconds(
+                                           kNavigationRecordWindowMs));
   for (auto it = navigations_.rbegin(); it != navigations_.rend(); ++it) {
     if (it->url == url) {
       base::TimeDelta delta = GetCurrentTimeTicks() - it->time;
       histograms_->RecordTimeSinceLastRecentVisit(origin, delta);
       return true;
     }
   }
 
   return false;
 }
@@ skipping 63 matching lines @@
 }
 
 void PrerenderManager::RecordFinalStatus(Origin origin,
                                          FinalStatus final_status) const {
   histograms_->RecordFinalStatus(origin, final_status);
 }
 
 void PrerenderManager::RecordNavigation(const GURL& url) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  navigations_.push_back(NavigationRecord(url, GetCurrentTimeTicks()));
-  CleanUpOldNavigations();
+  navigations_.emplace_back(url, GetCurrentTimeTicks(), ORIGIN_NONE);
+  CleanUpOldNavigations(&navigations_, base::TimeDelta::FromMilliseconds(
+                                           kNavigationRecordWindowMs));
 }
 
 struct PrerenderManager::PrerenderData::OrderByExpiryTime {
   bool operator()(const std::unique_ptr<PrerenderData>& a,
                   const std::unique_ptr<PrerenderData>& b) const {
     return a->expiry_time() < b->expiry_time();
   }
 };
 
 PrerenderManager::PrerenderData::PrerenderData(
@@ skipping 159 matching lines @@
                                       GetExpiryTimeForNewPrerender(origin)));
   if (!prerender_contents_ptr->Init()) {
     DCHECK(active_prerenders_.end() ==
            FindIteratorForPrerenderContents(prerender_contents_ptr));
     return nullptr;
   }
 
   histograms_->RecordPrerenderStarted(origin);
   DCHECK(!prerender_contents_ptr->prerendering_has_started());
 
+  if (prerender_contents_ptr->prerender_mode() == PREFETCH_ONLY)
+    prefetches_.emplace_back(url, GetCurrentTimeTicks(), origin);
+
   std::unique_ptr<PrerenderHandle> prerender_handle =
       base::WrapUnique(new PrerenderHandle(active_prerenders_.back().get()));
   SortActivePrerenders();
 
   last_prerender_start_time_ = GetCurrentTimeTicks();
 
   gfx::Rect contents_bounds =
       bounds.IsEmpty() ? config_.default_tab_bounds : bounds;
 
   prerender_contents_ptr->StartPrerendering(contents_bounds,
@@ skipping 151 matching lines @@
 
 void PrerenderManager::DeleteOldWebContents() {
   for (WebContents* web_contents : old_web_contents_list_) {
     // TODO(dominich): should we use Instant Unload Handler here?
     // Or should |old_web_contents_list_| contain unique_ptrs?
     delete web_contents;
   }
   old_web_contents_list_.clear();
 }
 
-void PrerenderManager::CleanUpOldNavigations() {
+void PrerenderManager::CleanUpOldNavigations(
+    std::vector<NavigationRecord>* navigations,
+    base::TimeDelta max_age) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Cutoff. Navigations before this cutoff can be discarded.
-  base::TimeTicks cutoff = GetCurrentTimeTicks() -
-      base::TimeDelta::FromMilliseconds(kNavigationRecordWindowMs);
-  auto it = navigations_.begin();
-  for (; it != navigations_.end(); ++it) {
+  base::TimeTicks cutoff = GetCurrentTimeTicks() - max_age;
+  auto it = navigations->begin();
+  for (; it != navigations->end(); ++it) {
     if (it->time > cutoff)
       break;
   }
-  navigations_.erase(navigations_.begin(), it);
+  navigations->erase(navigations->begin(), it);
 }
 
 void PrerenderManager::ScheduleDeleteOldWebContents(
     std::unique_ptr<WebContents> tab,
     OnCloseWebContentsDeleter* deleter) {
   old_web_contents_list_.push_back(tab.release());
   PostCleanupTask();
 
   if (!deleter)
     return;
@@ skipping 177 matching lines @@
   DCHECK_EQ(1u, erased);
 }
 
 void PrerenderManager::SetPrerenderContentsFactoryForTest(
     PrerenderContents::Factory* prerender_contents_factory) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   prerender_contents_factory_.reset(prerender_contents_factory);
 }
 
 }  // namespace prerender