[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

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 557 matching lines @@
                                            redirect_count);
 }
 
 void PrerenderManager::RecordNoStateFirstContentfulPaint(const GURL& url,
                                                          bool is_no_store,
                                                          bool was_hidden,
                                                          base::TimeDelta time) {
   base::TimeDelta prefetch_age;
   Origin origin;
   GetPrefetchInformation(url, &prefetch_age, &origin);
+  OnPrefetchUsed(url);
 
   histograms_->RecordPrefetchFirstContentfulPaintTime(
       origin, is_no_store, was_hidden, time, prefetch_age);
 
   for (auto& observer : observers_) {
     observer->OnFirstContentfulPaint();
   }
 }
 
 void PrerenderManager::RecordPrerenderFirstContentfulPaint(
     const GURL& url,
     content::WebContents* web_contents,
     bool is_no_store,
     bool was_hidden,
     base::TimeTicks first_contentful_paint) {
   DCHECK(!first_contentful_paint.is_null());
 
   PrerenderTabHelper* tab_helper =
       PrerenderTabHelper::FromWebContents(web_contents);
   DCHECK(tab_helper);
 
   base::TimeDelta prefetch_age;
   // The origin at prefetch is superceeded by the tab_helper origin for the
   // histogram recording, below.
   Origin unused_origin;
   GetPrefetchInformation(url, &prefetch_age, &unused_origin);
+  OnPrefetchUsed(url);
 
   base::TimeTicks swap_ticks = tab_helper->swap_ticks();
   bool fcp_recorded = false;
   if (!swap_ticks.is_null() && !first_contentful_paint.is_null()) {
     histograms_->RecordPrefetchFirstContentfulPaintTime(
         tab_helper->origin(), is_no_store, was_hidden,
         first_contentful_paint - swap_ticks, prefetch_age);
     fcp_recorded = true;
   }
   histograms_->RecordPerceivedFirstContentfulPaintStatus(
@@ skipping 318 matching lines @@
   if (prerendering_status != NetworkPredictionStatus::ENABLED) {
     FinalStatus final_status =
         prerendering_status == NetworkPredictionStatus::DISABLED_DUE_TO_NETWORK
             ? FINAL_STATUS_CELLULAR_NETWORK
             : FINAL_STATUS_PRERENDERING_DISABLED;
     RecordFinalStatusWithoutCreatingPrerenderContents(url, origin,
                                                       final_status);
     return nullptr;
   }
 
-  if (PrerenderData* preexisting_prerender_data =
-          FindPrerenderData(url, session_storage_namespace)) {
+  if (IsNoStatePrefetch(origin)) {
+    base::TimeDelta prefetch_age;
+    Origin origin;

[pasko, 2017/01/04 15:00:45]

overriding |origin| in this local scope looks dangerous, can it be
|prefetch_origin|?

+    GetPrefetchInformation(url, &prefetch_age, &origin);
+    if (!prefetch_age.is_zero() && prefetch_age < config_.time_to_live) {
+      RecordFinalStatusWithoutCreatingPrerenderContents(url, origin,

[pasko, 2017/01/04 15:00:45]

this should register the origin from the AddPrerender arg, not the origin from
prefetch information, right?

[droger, 2017/01/04 15:14:48]

This is a bug, good catch.
The local variable should not be used, only the parameter from the function is
important.

+                                                        FINAL_STATUS_DUPLICATE);
+      return nullptr;
+    }
+  } else if (PrerenderData* preexisting_prerender_data =
+                 FindPrerenderData(url, session_storage_namespace)) {
     RecordFinalStatusWithoutCreatingPrerenderContents(
         url, origin, FINAL_STATUS_DUPLICATE);
     return base::WrapUnique(new PrerenderHandle(preexisting_prerender_data));
   }
 
   // Do not prerender if there are too many render processes, and we would
   // have to use an existing one.  We do not want prerendering to happen in
   // a shared process, so that we can always reliably lower the CPU
   // priority for prerendering.
   // In single-process mode, ShouldTryToUseExistingProcessHost() always returns
@@ skipping 243 matching lines @@
 
   *prefetch_age = base::TimeDelta();
   *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;
     }
   }
+}
 
+void PrerenderManager::OnPrefetchUsed(const GURL& url) {
   // Loading a prefetched URL resets the revalidation bypass. Remove all
   // matching urls 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());
 }
 
 void PrerenderManager::CleanUpOldNavigations(
     std::vector<NavigationRecord>* navigations,
@@ skipping 204 matching lines @@
   return weak_factory_.GetWeakPtr();
 }
 
 void PrerenderManager::SetPrerenderContentsFactoryForTest(
     PrerenderContents::Factory* prerender_contents_factory) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   prerender_contents_factory_.reset(prerender_contents_factory);
 }
 
 }  // namespace prerender