Add Metrics struct to IsolatedPrerenderTabHelper

This struct keeps all the metrics about prefetching actions. This CL is
in preparation for recording these metrics in UKM by plumbing them out
of the TabHelper.

Two small behavioral changes:
* all predicted URLs will now be checked for eligibility, even if the
  max prefetching limit has already been reached. In reality, this
  would probably never happen anyways but it came up in testing.
* Redirects count against attempts. This way, eligible and attempt #s
  always line up with respect to the maximum number of prefetches

Bug: 1064726
Change-Id: I5d0f0b9706c12952f65335e67915b04eea60ed7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2125615
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754880}

chrome/browser/prerender/isolated/isolated_prerender_tab_helper.cc

@@ -116,7 +116,7 @@ void IsolatedPrerenderTabHelper::DidStartNavigation(
   }
 
   // User is navigating, don't bother prefetching further.
-  page_->url_loader.reset();
+  page_->url_loader_.reset();
 }
 
 void IsolatedPrerenderTabHelper::DidFinishNavigation(
@@ -133,6 +133,9 @@ void IsolatedPrerenderTabHelper::DidFinishNavigation(
   }
 
   DCHECK(!PrefetchingActive());
+
+  // |page_| is reset on commit so that any available cached prefetches that
+  // result from a redirect get used.
   page_ = std::make_unique<CurrentPageLoad>();
 }
 
@@ -154,31 +157,31 @@ void IsolatedPrerenderTabHelper::OnVisibilityChanged(
 std::unique_ptr<PrefetchedMainframeResponseContainer>
 IsolatedPrerenderTabHelper::TakePrefetchResponse(const GURL& url) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  auto it = page_->prefetched_responses.find(url);
-  if (it == page_->prefetched_responses.end())
+  auto it = page_->prefetched_responses_.find(url);
+  if (it == page_->prefetched_responses_.end())
     return nullptr;
 
   std::unique_ptr<PrefetchedMainframeResponseContainer> response =
       std::move(it->second);
-  page_->prefetched_responses.erase(it);
+  page_->prefetched_responses_.erase(it);
   return response;
 }
 
 bool IsolatedPrerenderTabHelper::PrefetchingActive() const {
-  return page_ && page_->url_loader;
+  return page_ && page_->url_loader_;
 }
 
 void IsolatedPrerenderTabHelper::Prefetch() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(IsolatedPrerenderIsEnabled());
 
-  page_->url_loader.reset();
-  if (page_->urls_to_prefetch.empty()) {
+  page_->url_loader_.reset();
+  if (page_->urls_to_prefetch_.empty()) {
     return;
   }
 
   if (IsolatedPrerenderMaximumNumberOfPrefetches().has_value() &&
-      page_->num_prefetches_attempted >=
+      page_->metrics_.prefetch_attempted_count >=
           IsolatedPrerenderMaximumNumberOfPrefetches().value()) {
     return;
   }
@@ -189,10 +192,10 @@ void IsolatedPrerenderTabHelper::Prefetch() {
     return;
   }
 
-  page_->num_prefetches_attempted++;
+  page_->metrics_.prefetch_attempted_count++;
 
-  GURL url = page_->urls_to_prefetch[0];
-  page_->urls_to_prefetch.erase(page_->urls_to_prefetch.begin());
+  GURL url = page_->urls_to_prefetch_[0];
+  page_->urls_to_prefetch_.erase(page_->urls_to_prefetch_.begin());
 
   net::NetworkIsolationKey key =
       net::NetworkIsolationKey::CreateOpaqueAndNonTransient();
@@ -235,15 +238,15 @@ void IsolatedPrerenderTabHelper::Prefetch() {
 
   // TODO(crbug/1023485): Disallow auth challenges.
 
-  page_->url_loader =
+  page_->url_loader_ =
       network::SimpleURLLoader::Create(std::move(request), traffic_annotation);
 
-  // base::Unretained is safe because |page_->url_loader| is owned by |this|.
-  page_->url_loader->SetOnRedirectCallback(base::BindRepeating(
+  // base::Unretained is safe because |page_->url_loader_| is owned by |this|.
+  page_->url_loader_->SetOnRedirectCallback(base::BindRepeating(
       &IsolatedPrerenderTabHelper::OnPrefetchRedirect, base::Unretained(this)));
-  page_->url_loader->SetAllowHttpErrorResults(true);
-  page_->url_loader->SetTimeoutDuration(IsolatedPrefetchTimeoutDuration());
-  page_->url_loader->DownloadToString(
+  page_->url_loader_->SetAllowHttpErrorResults(true);
+  page_->url_loader_->SetTimeoutDuration(IsolatedPrefetchTimeoutDuration());
+  page_->url_loader_->DownloadToString(
       GetURLLoaderFactory(),
       base::BindOnce(&IsolatedPrerenderTabHelper::OnPrefetchComplete,
                      base::Unretained(this), url, key),
@@ -255,13 +258,11 @@ void IsolatedPrerenderTabHelper::OnPrefetchRedirect(
     const network::mojom::URLResponseHead& response_head,
     std::vector<std::string>* removed_headers) {
   DCHECK(PrefetchingActive());
+
   // Run the new URL through all the eligibility checks. In the mean time,
   // continue on with other Prefetches.
-  if (CheckAndMaybePrefetchURL(redirect_info.new_url)) {
-    // The redirect shouldn't count against our prefetch limit if the redirect
-    // was followed.
-    page_->num_prefetches_attempted--;
-  }
+  CheckAndMaybePrefetchURL(redirect_info.new_url);
+
   // Cancels the current request.
   Prefetch();
 }
@@ -274,12 +275,12 @@ void IsolatedPrerenderTabHelper::OnPrefetchComplete(
   DCHECK(PrefetchingActive());
 
   base::UmaHistogramSparse("IsolatedPrerender.Prefetch.Mainframe.NetError",
-                           std::abs(page_->url_loader->NetError()));
+                           std::abs(page_->url_loader_->NetError()));
 
-  if (page_->url_loader->NetError() == net::OK && body &&
-      page_->url_loader->ResponseInfo()) {
+  if (page_->url_loader_->NetError() == net::OK && body &&
+      page_->url_loader_->ResponseInfo()) {
     network::mojom::URLResponseHeadPtr head =
-        page_->url_loader->ResponseInfo()->Clone();
+        page_->url_loader_->ResponseInfo()->Clone();
     HandlePrefetchResponse(url, key, std::move(head), std::move(body));
   }
   Prefetch();
@@ -336,7 +337,8 @@ void IsolatedPrerenderTabHelper::HandlePrefetchResponse(
   std::unique_ptr<PrefetchedMainframeResponseContainer> response =
       std::make_unique<PrefetchedMainframeResponseContainer>(
           key, std::move(head), std::move(body));
-  page_->prefetched_responses.emplace(url, std::move(response));
+  page_->prefetched_responses_.emplace(url, std::move(response));
+  page_->metrics_.prefetch_successful_count++;
 }
 
 void IsolatedPrerenderTabHelper::OnPredictionUpdated(
@@ -359,15 +361,6 @@ void IsolatedPrerenderTabHelper::OnPredictionUpdated(
     return;
   }
 
-  // This is also checked before prefetching from the network, but checking
-  // again here allows us to skip querying for cookies if we won't be
-  // prefetching the url anyways.
-  if (IsolatedPrerenderMaximumNumberOfPrefetches().has_value() &&
-      page_->num_prefetches_attempted >=
-          IsolatedPrerenderMaximumNumberOfPrefetches().value()) {
-    return;
-  }
-
   if (!prediction.has_value()) {
     return;
   }
@@ -382,7 +375,20 @@ void IsolatedPrerenderTabHelper::OnPredictionUpdated(
     return;
   }
 
-  for (const GURL& url : prediction.value().sorted_predicted_urls()) {
+  // It is possible, since it is not stipulated by the API contract, that the
+  // navigation predictor will issue multiple predictions during a single page
+  // load. Additional predictions should be treated as appending to the ordering
+  // of previous predictions.
+  size_t original_prediction_ordering_starting_size =
+      page_->original_prediction_ordering_.size();
+
+  for (size_t i = 0; i < prediction.value().sorted_predicted_urls().size();
+       ++i) {
+    GURL url = prediction.value().sorted_predicted_urls()[i];
+
+    size_t url_index = original_prediction_ordering_starting_size + i;
+    page_->original_prediction_ordering_.emplace(url, url_index);
+
     CheckAndMaybePrefetchURL(url);
   }
 }
@@ -442,11 +448,29 @@ void IsolatedPrerenderTabHelper::OnGotCookieList(
     const net::CookieStatusList& cookie_with_status_list,
     const net::CookieStatusList& excluded_cookies) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   if (!cookie_with_status_list.empty())
     return;
 
   // TODO(robertogden): Consider adding redirect URLs to the front of the list.
-  page_->urls_to_prefetch.push_back(url);
+  page_->urls_to_prefetch_.push_back(url);
+  page_->metrics_.prefetch_eligible_count++;
+
+  // The queried url may not have been part of this page's prediction if it was
+  // a redirect (common) or if the cookie query finished after
+  // |OnFinishNavigation| (less common). Either way, don't record anything in
+  // the bitmask.
+  if (page_->original_prediction_ordering_.find(url) !=
+      page_->original_prediction_ordering_.end()) {
+    size_t original_prediction_index =
+        page_->original_prediction_ordering_.find(url)->second;
+    // Check that we won't go above the allowable size.
+    if (original_prediction_index <
+        sizeof(page_->metrics_.ordered_eligible_pages_bitmask) * 8) {
+      page_->metrics_.ordered_eligible_pages_bitmask |=
+          1 << original_prediction_index;
+    }
+  }
 
   if (!PrefetchingActive()) {
     Prefetch();
@@ -455,16 +479,16 @@ void IsolatedPrerenderTabHelper::OnGotCookieList(
 
 network::mojom::URLLoaderFactory*
 IsolatedPrerenderTabHelper::GetURLLoaderFactory() {
-  if (!page_->isolated_url_loader_factory) {
+  if (!page_->isolated_url_loader_factory_) {
     CreateIsolatedURLLoaderFactory();
   }
-  DCHECK(page_->isolated_url_loader_factory);
-  return page_->isolated_url_loader_factory.get();
+  DCHECK(page_->isolated_url_loader_factory_);
+  return page_->isolated_url_loader_factory_.get();
 }
 
 void IsolatedPrerenderTabHelper::CreateIsolatedURLLoaderFactory() {
-  page_->isolated_network_context.reset();
-  page_->isolated_url_loader_factory.reset();
+  page_->isolated_network_context_.reset();
+  page_->isolated_url_loader_factory_.reset();
 
   IsolatedPrerenderService* isolated_prerender_service =
       IsolatedPrerenderServiceFactory::GetForProfile(profile_);
@@ -484,7 +508,7 @@ void IsolatedPrerenderTabHelper::CreateIsolatedURLLoaderFactory() {
       std::move(config_client));
 
   content::GetNetworkService()->CreateNetworkContext(
-      page_->isolated_network_context.BindNewPipeAndPassReceiver(),
+      page_->isolated_network_context_.BindNewPipeAndPassReceiver(),
       std::move(context_params));
 
   auto factory_params = network::mojom::URLLoaderFactoryParams::New();
@@ -492,8 +516,8 @@ void IsolatedPrerenderTabHelper::CreateIsolatedURLLoaderFactory() {
   factory_params->is_trusted = true;
   factory_params->is_corb_enabled = false;
 
-  page_->isolated_network_context->CreateURLLoaderFactory(
-      page_->isolated_url_loader_factory.BindNewPipeAndPassReceiver(),
+  page_->isolated_network_context_->CreateURLLoaderFactory(
+      page_->isolated_url_loader_factory_.BindNewPipeAndPassReceiver(),
       std::move(factory_params));
 }
 

chrome/browser/prerender/isolated/isolated_prerender_tab_helper.h

@@ -43,9 +43,33 @@ class IsolatedPrerenderTabHelper
  public:
   ~IsolatedPrerenderTabHelper() override;
 
-  size_t prefetched_responses_size_for_testing() const {
-    return page_ ? page_->prefetched_responses.size() : 0;
-  }
+  // Simple container for several metrics which pertain to prefetching actions
+  // on a Google SRP.
+  struct PrefetchMetrics {
+    // This bitmask keeps track each eligible page's placement in the original
+    // navigation prediction. The Nth-LSB is set if the Nth predicted page is
+    // eligible. Pages are in descending order of likelihood of user clicking.
+    // For example, if the following prediction is made:
+    //
+    //   [eligible, not eligible, eligible, eligible]
+    //
+    // then the resulting bitmask will be
+    //
+    //   0b1101.
+    int64_t ordered_eligible_pages_bitmask = 0;
+
+    // The number of SRP links that were eligible to be prefetched.
+    size_t prefetch_eligible_count = 0;
+
+    // The number of eligible prefetches that were attempted.
+    size_t prefetch_attempted_count = 0;
+
+    // The number of attempted prefetches that were successful (net error was OK
+    // and HTTP response code was 2XX).
+    size_t prefetch_successful_count = 0;
+  };
+
+  const PrefetchMetrics& metrics() const { return page_->metrics_; }
 
   void CallHandlePrefetchResponseForTesting(
       const GURL& url,
@@ -79,25 +103,29 @@ class IsolatedPrerenderTabHelper
     CurrentPageLoad();
     ~CurrentPageLoad();
 
+    // The metrics pertaining to prefetching actions on a Google SRP page.
+    PrefetchMetrics metrics_;
+
+    // A map of all predicted URLs to their original placement in the ordered
+    // prediction.
+    std::map<GURL, size_t> original_prediction_ordering_;
+
     // The url loader that does all the prefetches. Set only when active.
-    std::unique_ptr<network::SimpleURLLoader> url_loader;
+    std::unique_ptr<network::SimpleURLLoader> url_loader_;
 
     // An ordered list of the URLs to prefetch.
-    std::vector<GURL> urls_to_prefetch;
-
-    // The number of prefetches that have been attempted on this page.
-    size_t num_prefetches_attempted = 0;
+    std::vector<GURL> urls_to_prefetch_;
 
     // All prefetched responses by URL. This is cleared every time a mainframe
     // navigation commits.
     std::map<GURL, std::unique_ptr<PrefetchedMainframeResponseContainer>>
-        prefetched_responses;
+        prefetched_responses_;
 
     // The network context and url loader factory that will be used for
     // prefetches. A separate network context is used so that the prefetch proxy
-    // can be used via a custom porxy configuration.
-    mojo::Remote<network::mojom::URLLoaderFactory> isolated_url_loader_factory;
-    mojo::Remote<network::mojom::NetworkContext> isolated_network_context;
+    // can be used via a custom proxy configuration.
+    mojo::Remote<network::mojom::URLLoaderFactory> isolated_url_loader_factory_;
+    mojo::Remote<network::mojom::NetworkContext> isolated_network_context_;
   };
 
   // A helper method to make it easier to tell when prefetching is already