predictors: Add RedirectStatus histogram + fix redirects related bug.

chrome/browser/predictors/resource_prefetch_predictor.cc

 // Copyright 2014 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/predictors/resource_prefetch_predictor.h"
 
 #include <map>
 #include <set>
 #include <utility>
 
@@ skipping 153 matching lines @@
       internal::kResourcePrefetchPredictorPrefetchHitsCountNotCached,
       not_cached_hits_count);
   UMA_HISTOGRAM_COUNTS_10000(
       internal::kResourcePrefetchPredictorPrefetchHitsSize, hits_bytes / 1024);
   UMA_HISTOGRAM_COUNTS_10000(
       internal::kResourcePrefetchPredictorPrefetchMissesSize,
       misses_bytes / 1024);
 }
 
 void ReportPredictionAccuracy(
-    const std::vector<GURL>& predicted_urls,
+    const ResourcePrefetchPredictor::Prediction& prediction,
     const ResourcePrefetchPredictor::PageRequestSummary& summary) {
-  DCHECK(!predicted_urls.empty());
-
+  const std::vector<GURL>& predicted_urls = prediction.subresource_urls;
   if (predicted_urls.empty() || summary.subresource_requests.empty())
     return;
 
   std::set<GURL> predicted_urls_set(predicted_urls.begin(),
                                     predicted_urls.end());
   std::set<GURL> actual_urls_set;
   for (const auto& request_summary : summary.subresource_requests)
     actual_urls_set.insert(request_summary.resource_url);
 
   size_t correctly_predicted_count = 0;
   for (const GURL& predicted_url : predicted_urls_set) {
     if (actual_urls_set.find(predicted_url) != actual_urls_set.end())
       correctly_predicted_count++;
   }
 
   size_t precision_percentage =
       (100 * correctly_predicted_count) / predicted_urls_set.size();
   size_t recall_percentage =
       (100 * correctly_predicted_count) / actual_urls_set.size();
 
+  using RedirectStatus = ResourcePrefetchPredictor::RedirectStatus;
+  RedirectStatus redirect_status;
+  if (summary.main_frame_url == summary.initial_url) {
+    // The actual navigation wasn't redirected.
+    redirect_status = prediction.is_redirected
+                          ? RedirectStatus::NO_REDIRECT_BUT_PREDICTED
+                          : RedirectStatus::NO_REDIRECT;
+  } else {
+    if (prediction.is_redirected) {
+      std::string main_frame_key = prediction.is_host
+                                       ? summary.main_frame_url.host()
+                                       : summary.main_frame_url.spec();
+      redirect_status = main_frame_key == prediction.main_frame_key
+                            ? RedirectStatus::REDIRECT_CORRECTLY_PREDICTED
+                            : RedirectStatus::REDIRECT_WRONG_PREDICTED;
+    } else {
+      redirect_status = RedirectStatus::REDIRECT_NOT_PREDICTED;
+    }
+  }
+
   UMA_HISTOGRAM_PERCENTAGE(
       internal::kResourcePrefetchPredictorPrecisionHistogram,
       precision_percentage);
   UMA_HISTOGRAM_PERCENTAGE(internal::kResourcePrefetchPredictorRecallHistogram,
                            recall_percentage);
   UMA_HISTOGRAM_COUNTS_100(internal::kResourcePrefetchPredictorCountHistogram,
                            predicted_urls.size());
+  UMA_HISTOGRAM_ENUMERATION(
+      internal::kResourcePrefetchPredictorRedirectStatusHistogram,
+      static_cast<int>(redirect_status), static_cast<int>(RedirectStatus::MAX));
 }
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // ResourcePrefetchPredictor static functions.
 
 // static
 bool ResourcePrefetchPredictor::ShouldRecordRequest(
     net::URLRequest* request,
@@ skipping 227 matching lines @@
 
 ResourcePrefetchPredictor::PageRequestSummary::PageRequestSummary(
     const GURL& i_main_frame_url)
     : main_frame_url(i_main_frame_url), initial_url(i_main_frame_url) {}
 
 ResourcePrefetchPredictor::PageRequestSummary::PageRequestSummary(
     const PageRequestSummary& other) = default;
 
 ResourcePrefetchPredictor::PageRequestSummary::~PageRequestSummary() {}
 
+ResourcePrefetchPredictor::Prediction::Prediction() = default;
+
+ResourcePrefetchPredictor::Prediction::Prediction(
+    const ResourcePrefetchPredictor::Prediction& other) = default;
+
+ResourcePrefetchPredictor::Prediction::~Prediction() = default;
+
 ////////////////////////////////////////////////////////////////////////////////
 // ResourcePrefetchPredictor.
 
 ResourcePrefetchPredictor::ResourcePrefetchPredictor(
     const ResourcePrefetchPredictorConfig& config,
     Profile* profile)
     : profile_(profile),
       observer_(nullptr),
       config_(config),
       initialization_state_(NOT_INITIALIZED),
@@ skipping 100 matching lines @@
   if (inflight_prefetches_.find(url) == inflight_prefetches_.end() &&
       IsUrlPrefetchable(url)) {
     inflight_prefetches_.insert(std::make_pair(url, base::TimeTicks::Now()));
   }
 
   if (!prefetch_manager_.get())  // Prefetching not enabled.
     return;
   if (!config_.IsPrefetchingEnabledForOrigin(profile_, origin))
     return;
 
-  std::vector<GURL> subresource_urls;
-  if (!GetPrefetchData(url, &subresource_urls))
+  ResourcePrefetchPredictor::Prediction prediction;
+  if (!GetPrefetchData(url, &prediction))
     return;
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&ResourcePrefetcherManager::MaybeAddPrefetch,
-                 prefetch_manager_, url, subresource_urls));
+                 prefetch_manager_, url, prediction.subresource_urls));
 }
 
 void ResourcePrefetchPredictor::StopPrefetching(const GURL& url) {
   TRACE_EVENT1("browser", "ResourcePrefetchPredictor::StopPrefetching", "url",
                url.spec());
   auto it = inflight_prefetches_.find(url);
   if (it != inflight_prefetches_.end()) {
     UMA_HISTOGRAM_TIMES(
         internal::kResourcePrefetchPredictorPrefetchingDurationHistogram,
         base::TimeTicks::Now() - it->second);
@@ skipping 112 matching lines @@
 
   NavigationMap::iterator nav_it =
       inflight_navigations_.find(nav_id_without_timing_info);
   if (nav_it == inflight_navigations_.end())
     return;
 
   // Remove the navigation from the inflight navigations.
   std::unique_ptr<PageRequestSummary> summary = std::move(nav_it->second);
   inflight_navigations_.erase(nav_it);
 
-  const GURL& main_frame_url = nav_id_without_timing_info.main_frame_url;

[alexilin, 2017/02/24 17:24:10]

This is a bug that I'm talking about.
We starts prefetch using the first URL in the redirect chain. We need to use
initial_url to get the same prefetch data.
The same thing with prefetcher_stats_. OnPrefetchingFinished will be called with
initial_url.

-  std::vector<GURL> predicted_urls;
-  bool has_data = GetPrefetchData(main_frame_url, &predicted_urls);
+  const GURL& initial_url = summary->initial_url;
+  ResourcePrefetchPredictor::Prediction prediction;
+  bool has_data = GetPrefetchData(initial_url, &prediction);
   if (has_data)
-    ReportPredictionAccuracy(predicted_urls, *summary);
+    ReportPredictionAccuracy(prediction, *summary);
 
-  auto it = prefetcher_stats_.find(main_frame_url);
+  auto it = prefetcher_stats_.find(initial_url);
   if (it != prefetcher_stats_.end()) {
     const std::vector<URLRequestSummary>& summaries =
         summary->subresource_requests;
     ReportPrefetchAccuracy(*it->second, summaries);
     prefetcher_stats_.erase(it);
   }
 
   // Kick off history lookup to determine if we should record the URL.
   history::HistoryService* history_service =
       HistoryServiceFactory::GetForProfile(profile_,
                                            ServiceAccessType::EXPLICIT_ACCESS);
   DCHECK(history_service);
   history_service->ScheduleDBTask(
       std::unique_ptr<history::HistoryDBTask>(new GetUrlVisitCountTask(
           std::move(summary),
           base::Bind(&ResourcePrefetchPredictor::OnVisitCountLookup,
                      AsWeakPtr()))),
       &history_lookup_consumer_);
 
   // Report readiness metric with 20% probability.
   if (base::RandInt(1, 5) == 5) {
     history_service->TopHosts(
         kNumSampleHosts,
         base::Bind(&ResourcePrefetchPredictor::ReportDatabaseReadiness,
                    AsWeakPtr()));
   }
 }
 
-bool ResourcePrefetchPredictor::GetPrefetchData(const GURL& main_frame_url,
-                                                std::vector<GURL>* urls) const {
+bool ResourcePrefetchPredictor::GetPrefetchData(
+    const GURL& main_frame_url,
+    ResourcePrefetchPredictor::Prediction* prediction) const {
+  std::vector<GURL>* urls =
+      prediction ? &prediction->subresource_urls : nullptr;
   DCHECK(!urls || urls->empty());
 
   // Fetch URLs based on a redirect endpoint for URL/host first.
   std::string redirect_endpoint;
   if (GetRedirectEndpoint(main_frame_url.spec(), *url_redirect_table_cache_,
                           &redirect_endpoint) &&
       PopulatePrefetcherRequest(redirect_endpoint, *url_table_cache_, urls)) {
+    if (prediction) {
+      prediction->is_host = false;
+      prediction->is_redirected = true;
+      prediction->main_frame_key = redirect_endpoint;
+    }
     return true;
   }
 
   if (GetRedirectEndpoint(main_frame_url.host(), *host_redirect_table_cache_,
                           &redirect_endpoint) &&
       PopulatePrefetcherRequest(redirect_endpoint, *host_table_cache_, urls)) {
+    if (prediction) {
+      prediction->is_host = true;
+      prediction->is_redirected = true;
+      prediction->main_frame_key = redirect_endpoint;
+    }
     return true;
   }
 
   // Fallback to fetching URLs based on the incoming URL/host.
   if (PopulatePrefetcherRequest(main_frame_url.spec(), *url_table_cache_,
                                 urls)) {
+    if (prediction) {
+      prediction->is_host = false;
+      prediction->is_redirected = false;
+      prediction->main_frame_key = main_frame_url.spec();
+    }
     return true;
   }
 
-  return PopulatePrefetcherRequest(main_frame_url.host(), *host_table_cache_,
-                                   urls);
+  if (PopulatePrefetcherRequest(main_frame_url.host(), *host_table_cache_,
+                                urls)) {
+    if (prediction) {
+      prediction->is_host = true;
+      prediction->is_redirected = false;
+      prediction->main_frame_key = main_frame_url.host();
+    }
+    return true;
+  }
+
+  return false;
 }
 
 bool ResourcePrefetchPredictor::PopulatePrefetcherRequest(
     const std::string& main_frame_key,
     const PrefetchDataMap& data_map,
     std::vector<GURL>* urls) const {
   PrefetchDataMap::const_iterator it = data_map.find(main_frame_key);
   if (it == data_map.end())
     return false;
 
@@ skipping 554 matching lines @@
 TestObserver::~TestObserver() {
   predictor_->SetObserverForTesting(nullptr);
 }
 
 TestObserver::TestObserver(ResourcePrefetchPredictor* predictor)
     : predictor_(predictor) {
   predictor_->SetObserverForTesting(this);
 }
 
 }  // namespace predictors