Refactor net predictor to use ResourceThrottles

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.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/renderer_host/chrome_resource_dispatcher_host_delegate.h"
 
 #include <stdint.h>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 13 matching lines @@
 #include "chrome/browser/mod_pagespeed/mod_pagespeed_metrics.h"
 #include "chrome/browser/net/resource_prefetch_predictor_observer.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/prerender/prerender_resource_throttle.h"
 #include "chrome/browser/prerender/prerender_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/renderer_host/chrome_navigation_data.h"
+#include "chrome/browser/renderer_host/predictor_resource_throttle.h"
 #include "chrome/browser/renderer_host/safe_browsing_resource_throttle.h"
 #include "chrome/browser/renderer_host/thread_hop_resource_throttle.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/signin/chrome_signin_helper.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/login/login_handler.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/features.h"
@@ skipping 544 matching lines @@
   }
 #endif
 
   const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);
   if (info->GetVisibilityState() == blink::WebPageVisibilityStatePrerender) {
     throttles->push_back(new prerender::PrerenderResourceThrottle(request));
   }
 
   if (ThreadHopResourceThrottle::IsEnabled())
     throttles->push_back(new ThreadHopResourceThrottle);
+
+  if (io_data->GetPredictor()) {

[mmenke, 2016/05/23 18:50:22]

I'd suggest avoiding the knowledge that we only create this throttle if
ProfileIOData::GetPredictor returns non-NULL here.  Instead I'd suggest:

base::unique_ptr<content::ResourceThrottle> predictor_throttle =
    PredictorResourceThrottle::MaybeCreate(request, io_data);
if (predictor_throttle)
  throttles->push_back(predictor_throttle.release());

While I don't see us ever adding more complexity to the logic here, it minimizes
what this code needs to know about the throttle, and follows the pattern of most
other logic in this method (Admittedly, other code here doesn't use smart
pointers, but it should).

[Charlie Harrison, 2016/05/23 21:13:32]

SGTM. Done.

+    throttles->push_back(
+        new PredictorResourceThrottle(request, io_data->GetPredictor()));
+  }
 }
 
 bool ChromeResourceDispatcherHostDelegate::ShouldForceDownloadResource(
     const GURL& url, const std::string& mime_type) {
 #if defined(ENABLE_EXTENSIONS)
   // Special-case user scripts to get downloaded instead of viewed.
   return extensions::UserScript::IsURLUserScript(url, mime_type);
 #else
   return false;
 #endif
@@ skipping 206 matching lines @@
   data_reduction_proxy::DataReductionProxyData* data_reduction_proxy_data =
       data_reduction_proxy::DataReductionProxyData::GetData(*request);
   // DeepCopy the DataReductionProxyData from the URLRequest to prevent the
   // URLRequest and DataReductionProxyData from both having ownership of the
   // same object. This copy will be shortlived as it will be deep copied again
   // when content makes a clone of NavigationData for the UI thread.
   if (data_reduction_proxy_data)
     data->SetDataReductionProxyData(data_reduction_proxy_data->DeepCopy());
   return data;
 }