[NoStatePrefetch] Track redirects in UMA

chrome/browser/prerender/prerender_resource_throttle.cc

 // Copyright 2013 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_resource_throttle.h"
 
 #include "build/build_config.h"
 #include "chrome/browser/prerender/prerender_final_status.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/web_contents.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/url_request.h"
 
 using content::ResourceType;
 
 namespace prerender {
 
 namespace {
 static const char kFollowOnlyWhenPrerenderShown[] =
     "follow-only-when-prerender-shown";
 
 PrerenderContents* g_prerender_contents_for_testing;
+
+// Returns true if the response has a "no-store" cache control header.
+bool IsNoStoreResponse(net::URLRequest* request) {

[pasko, 2016/09/02 16:34:48]

would it be possible to pass the argument by const reference, as recommended in
the style guide?

[droger, 2016/09/05 12:38:47]

Done.

+  const net::HttpResponseInfo& response_info = request->response_info();
+  return response_info.headers.get() &&
+         response_info.headers->HasHeaderValue("cache-control", "no-store");
+}
+
 }  // namespace
 
 void PrerenderResourceThrottle::OverridePrerenderContentsForTesting(
     PrerenderContents* contents) {
   g_prerender_contents_for_testing = contents;
 }
 
 PrerenderResourceThrottle::PrerenderResourceThrottle(net::URLRequest* request)
     : request_(request) {
 }
@@ skipping 14 matching lines @@
 void PrerenderResourceThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
     bool* defer) {
   const content::ResourceRequestInfo* info =
       content::ResourceRequestInfo::ForRequest(request_);
   *defer = true;
   std::string header;
   request_->GetResponseHeaderByName(kFollowOnlyWhenPrerenderShown, &header);
 
   content::BrowserThread::PostTask(
-      content::BrowserThread::UI,
-      FROM_HERE,
+      content::BrowserThread::UI, FROM_HERE,
       base::Bind(&PrerenderResourceThrottle::WillRedirectRequestOnUI,
                  AsWeakPtr(), header, info->GetResourceType(), info->IsAsync(),
-                 info->GetChildID(), info->GetRenderFrameID(),
-                 redirect_info.new_url));
+                 IsNoStoreResponse(request_), info->GetChildID(),
+                 info->GetRenderFrameID(), redirect_info.new_url));
 }
 
 void PrerenderResourceThrottle::WillProcessResponse(bool* defer) {
   const content::ResourceRequestInfo* info =
       content::ResourceRequestInfo::ForRequest(request_);
   if (!info)
     return;
 
-  bool is_no_store = false;
-  const net::HttpResponseInfo& response_info = request_->response_info();
-  if (response_info.headers.get()) {
-    is_no_store =
-        response_info.headers->HasHeaderValue("cache-control", "no-store");
-  }
-
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&PrerenderResourceThrottle::WillProcessResponseOnUI,
                  content::IsResourceTypeFrame(info->GetResourceType()),
-                 is_no_store, info->GetChildID(), info->GetRenderFrameID()));
+                 IsNoStoreResponse(request_), request_->url_chain().size() - 1,

[pasko, 2016/09/02 16:34:48]

Even though size of the chain is unlikely to be zero here, I want to be on the
safe side with this unsigned to signed conversion

how about:
    base::saturated_cast<int32_t>(request_->url_chain().size()),

and having int32_t everywhere after that just as base::HistogramBase::Sample?

[droger, 2016/09/05 12:38:47]

Done.

+                 info->GetChildID(), info->GetRenderFrameID()));
 }
 
 const char* PrerenderResourceThrottle::GetNameForLogging() const {
   return "PrerenderResourceThrottle";
 }
 
 void PrerenderResourceThrottle::Resume() {
   controller()->Resume();
 }
 
@@ skipping 38 matching lines @@
       base::Bind(cancel ? &PrerenderResourceThrottle::Cancel :
                  &PrerenderResourceThrottle::Resume, throttle));
 }
 
 // static
 void PrerenderResourceThrottle::WillRedirectRequestOnUI(
     const base::WeakPtr<PrerenderResourceThrottle>& throttle,
     const std::string& follow_only_when_prerender_shown_header,
     ResourceType resource_type,
     bool async,
+    bool is_no_store,
     int render_process_id,
     int render_frame_id,
     const GURL& new_url) {
   bool cancel = false;
   PrerenderContents* prerender_contents =
       PrerenderContentsFromRenderFrame(render_process_id, render_frame_id);
   if (prerender_contents) {
+    prerender_contents->prerender_manager()->RecordResourcePrefetch(
+        prerender_contents->origin(),
+        content::IsResourceTypeFrame(resource_type), true /*is_redirect*/,

[pasko, 2016/09/02 16:34:48]

most code I see surrounds the parameter name with spaces, e.g.:

/* is_redirect */

Also aesthetically this looks nicer to me.

[droger, 2016/09/05 12:38:47]

Done.

+        is_no_store);
     // Abort any prerenders with requests which redirect to invalid schemes.
     if (!PrerenderManager::DoesURLHaveValidScheme(new_url)) {
       prerender_contents->Destroy(FINAL_STATUS_UNSUPPORTED_SCHEME);
       ReportUnsupportedPrerenderScheme(new_url);
       cancel = true;
     } else if (follow_only_when_prerender_shown_header == "1" &&
                resource_type != content::RESOURCE_TYPE_MAIN_FRAME) {
       // Only defer redirects with the Follow-Only-When-Prerender-Shown
       // header. Do not defer redirects on main frame loads.
       if (!async) {
@@ skipping 12 matching lines @@
   content::BrowserThread::PostTask(
       content::BrowserThread::IO,
       FROM_HERE,
       base::Bind(cancel ? &PrerenderResourceThrottle::Cancel :
                  &PrerenderResourceThrottle::Resume, throttle));
 }
 
 // static
 void PrerenderResourceThrottle::WillProcessResponseOnUI(bool is_main_resource,
                                                         bool is_no_store,
+                                                        size_t redirect_count,
                                                         int render_process_id,
                                                         int render_frame_id) {
   PrerenderContents* prerender_contents =
       PrerenderContentsFromRenderFrame(render_process_id, render_frame_id);
   if (!prerender_contents)
     return;
 
   if (prerender_contents->prerender_mode() != PREFETCH_ONLY)
     return;
 
   prerender_contents->prerender_manager()->RecordResourcePrefetch(
-      prerender_contents->origin(), is_main_resource, is_no_store);
+      prerender_contents->origin(), is_main_resource, false /*is_redirect*/,
+      is_no_store);
+  prerender_contents->prerender_manager()->RecordPrefetchRedirectCount(
+      prerender_contents->origin(), is_main_resource, redirect_count);
 }
 
 // static
 PrerenderContents* PrerenderResourceThrottle::PrerenderContentsFromRenderFrame(
     int render_process_id, int render_frame_id) {
   if (g_prerender_contents_for_testing)
     return g_prerender_contents_for_testing;
   content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
       render_process_id, render_frame_id);
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(rfh);
   return PrerenderContents::FromWebContents(web_contents);
 }
 
 }  // namespace prerender