Add a PLT histogram for prerender local predictor prefetches.

chrome/browser/prerender/prerender_local_predictor.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_local_predictor.h"
 
 #include <ctype.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 437 matching lines @@
       return return_value;
     if (type == SEEN_TABCONTENTS_OBSERVER && !it->second->seen_tabcontents_) {
       it->second->seen_tabcontents_ = true;
       return_value = true;
     }
     if (type == SEEN_HISTORY && !it->second->seen_history_) {
       it->second->seen_history_ = true;
       return_value = true;
     }
     // If the item has been seen in both the history and in tab contents,
-    // erase it from the map to make room for new prefetches.
-    if (it->second->seen_tabcontents_ && it->second->seen_history_)
+    // and the page load time has been recorded, erase it from the map to
+    // make room for new prefetches.
+    if (it->second->seen_tabcontents_ && it->second->seen_history_ &&
+        it->second->plt_recorded_) {
       entries_.erase(url.spec().c_str());
+    }
     return return_value;
   }
 
+  // To be called when a PLT was seen for a given URL.
+  // Returns true iff:
+  // - There is an entry of a prefetch for the URL provided.
+  // - The prefetch was started before the page load was started (based on the
+  //   plt).
+  // - The prefetch entry has not been returned "true" for as a result of a call
+  //   to this method previously.
+  bool SeenPLTForURL(const GURL& url, base::TimeDelta plt) {

[jkarlin, 2014/08/21 14:34:14]

Rename to MarkPLTSeen?  Then reformat the comments to look more like MarkURLSeen
as well, as those are clearer.

// Marks the PLT for the provided URL as seen in the context specified.  Returns
true iff ... i.e. the marking was successful.

[tburkard, 2014/08/21 15:28:11]

Done.

+    ExpireOldItems();
+    base::hash_map<string, ListEntry*>::iterator it =
+        entries_.find(url.spec().c_str());
+    if (it == entries_.end() || it->second->plt_recorded_ ||
+        it->second->add_time_ > GetCurrentTime() - plt) {
+      return false;
+    }
+    it->second->plt_recorded_ = true;
+    // If the item has been seen in both the history and in tab contents,
+    // and the page load time has been recorded, erase it from the map to
+    // make room for new prefetches.
+    if (it->second->seen_tabcontents_ && it->second->seen_history_ &&
+        it->second->plt_recorded_) {
+      entries_.erase(url.spec().c_str());
+    }
+    return true;
+  }
+
  private:
   struct ListEntry {
     explicit ListEntry(const string& url)
         : url_(url),
           add_time_(GetCurrentTime()),
           seen_tabcontents_(false),
-          seen_history_(false) {
+          seen_history_(false),
+          plt_recorded_(false) {
     }
     std::string url_;
     base::Time add_time_;
     bool seen_tabcontents_;
     bool seen_history_;
+    bool plt_recorded_;

[jkarlin, 2014/08/21 14:34:14]

Why not seen_plt_ like the other members?  PrefetchList doesn't actually do the
recording.

[tburkard, 2014/08/21 15:28:11]

Done.

   };
 
   void ExpireOldItems() {
     base::Time expiry_cutoff = GetCurrentTime() -
         base::TimeDelta::FromSeconds(GetPrerenderPrefetchListTimeoutSeconds());
     while (!entry_list_.empty() &&
            (entry_list_.front()->add_time_ < expiry_cutoff ||
             entries_.size() > kMaxPrefetchItems)) {
       ListEntry* entry = entry_list_.front();
       entry_list_.pop_front();
@@ skipping 626 matching lines @@
         &history_db_tracker_);
     history->AddVisitDatabaseObserver(this);
     is_visit_database_observer_ = true;
   } else {
     RecordEvent(EVENT_INIT_FAILED_NO_HISTORY);
   }
 }
 
 void PrerenderLocalPredictor::OnPLTEventForURL(const GURL& url,
                                                base::TimeDelta page_load_time) {
+  if (prefetch_list_->SeenPLTForURL(url, page_load_time)) {
+    UMA_HISTOGRAM_CUSTOM_TIMES("Prerender.LocalPredictorPrefetchMatchPLT",
+                               page_load_time,
+                               base::TimeDelta::FromMilliseconds(10),
+                               base::TimeDelta::FromSeconds(60),
+                               100);
+  }
+
   scoped_ptr<PrerenderProperties> prerender;
   if (DoesPrerenderMatchPLTRecord(last_swapped_in_prerender_.get(),
                                   url, page_load_time)) {
     prerender.reset(last_swapped_in_prerender_.release());
   }
   if (DoesPrerenderMatchPLTRecord(current_prerender_.get(),
                                   url, page_load_time)) {
     prerender.reset(current_prerender_.release());
   }
   if (!prerender.get())
@@ skipping 199 matching lines @@
       num_issued++;
       continue;
     }
   }
 }
 
 void PrerenderLocalPredictor::IssuePrerender(
     CandidatePrerenderInfo* info,
     LocalPredictorURLInfo* url_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (prefetch_list_->AddURL(url_info->url))
+  if (prefetch_list_->AddURL(url_info->url)) {
     RecordEvent(EVENT_PREFETCH_LIST_ADDED);
+    // If we are prefetching rather than prerendering, now is the time to launch
+    // the prefetch.
+    if (IsLocalPredictorPrerenderPrefetchEnabled()) {
+      // Obtain the render frame host that caused this prefetch.
+      RenderFrameHost* rfh = RenderFrameHost::FromID(info->render_process_id_,
+                                                     info->render_frame_id_);
+      // If it is still alive, launch the prefresh.
+      if (rfh)
+        rfh->Send(new PrefetchMsg_Prefetch(rfh->GetRoutingID(), url_info->url));
+    }
+  }
   PrerenderProperties* prerender_properties =
       GetIssuedPrerenderSlotForPriority(url_info->url, url_info->priority);
   if (!prerender_properties) {
     RecordEvent(EVENT_CONTINUE_PRERENDER_CHECK_PRIORITY_TOO_LOW);
     return;
   }
   RecordEvent(EVENT_CONTINUE_PRERENDER_CHECK_ISSUING_PRERENDER);
   DCHECK(prerender_properties != NULL);
   DCHECK(info != NULL);
   DCHECK(url_info != NULL);
@@ skipping 35 matching lines @@
     prerender_properties->start_time = current_time;
     prerender_properties->actual_start_time = current_time;
     prerender_properties->would_have_matched = false;
     prerender_properties->prerender_handle.swap(new_prerender_handle);
     // new_prerender_handle now represents the old previou prerender that we
     // are replacing. So we need to cancel it.
     if (new_prerender_handle) {
       new_prerender_handle->OnCancel();
       RecordEvent(EVENT_ISSUE_PRERENDER_CANCELLED_OLD_PRERENDER);
     }
-    // If we are prefetching rather than prerendering, now is the time to launch
-    // the prefetch.
-    if (IsLocalPredictorPrerenderPrefetchEnabled()) {
-      // Obtain the render frame host that caused this prefetch.
-      RenderFrameHost* rfh = RenderFrameHost::FromID(info->render_process_id_,
-                                                     info->render_frame_id_);
-      // If it is still alive, launch the prefresh.
-      if (rfh)
-        rfh->Send(new PrefetchMsg_Prefetch(rfh->GetRoutingID(), url));
-    }
   }
 
   RecordEvent(EVENT_ADD_VISIT_PRERENDERING);
   if (current_prerender_.get() && current_prerender_->url_id == url_id) {
     RecordEvent(EVENT_ADD_VISIT_PRERENDERING_EXTENDED);
     if (priority > current_prerender_->priority)
       current_prerender_->priority = priority;
     // If the prerender already existed, we want to extend it.  However,
     // we do not want to set its start_time to the current time to
     // disadvantage PLT computations when the prerender is swapped in.
@@ skipping 108 matching lines @@
       break;
     case content::SessionStorageNamespace::MERGE_RESULT_MERGEABLE:
       RecordEvent(EVENT_NAMESPACE_MISMATCH_MERGE_RESULT_MERGEABLE);
       break;
     default:
       NOTREACHED();
   }
 }
 
 }  // namespace prerender