Commit 1bf0b01f authored by lizeb's avatar lizeb Committed by Commit bot

predictors: Remove ResourcePrefetchPredictorConfig from ResourcePrefetcher.

The object is quite large, this class doesn't need to know about
it. This is intended to ease refactoring.

BUG=715525

Review-Url: https://codereview.chromium.org/2845103002
Cr-Commit-Position: refs/heads/master@{#467717}
parent d0f88cff
...@@ -48,14 +48,15 @@ ResourcePrefetcher::PrefetcherStats::PrefetcherStats( ...@@ -48,14 +48,15 @@ ResourcePrefetcher::PrefetcherStats::PrefetcherStats(
start_time(other.start_time), start_time(other.start_time),
requests_stats(other.requests_stats) {} requests_stats(other.requests_stats) {}
ResourcePrefetcher::ResourcePrefetcher( ResourcePrefetcher::ResourcePrefetcher(Delegate* delegate,
Delegate* delegate, size_t max_concurrent_requests,
const ResourcePrefetchPredictorConfig& config, size_t max_concurrent_requests_per_host,
const GURL& main_frame_url, const GURL& main_frame_url,
const std::vector<GURL>& urls) const std::vector<GURL>& urls)
: state_(INITIALIZED), : state_(INITIALIZED),
delegate_(delegate), delegate_(delegate),
config_(config), max_concurrent_requests_(max_concurrent_requests),
max_concurrent_requests_per_host_(max_concurrent_requests_per_host),
main_frame_url_(main_frame_url), main_frame_url_(main_frame_url),
prefetched_count_(0), prefetched_count_(0),
prefetched_bytes_(0), prefetched_bytes_(0),
...@@ -99,8 +100,7 @@ void ResourcePrefetcher::TryToLaunchPrefetchRequests() { ...@@ -99,8 +100,7 @@ void ResourcePrefetcher::TryToLaunchPrefetchRequests() {
// max_prefetches_inflight_per_host_per_navigation limit, looking for a URL // max_prefetches_inflight_per_host_per_navigation limit, looking for a URL
// for which the max_prefetches_inflight_per_host_per_navigation limit has // for which the max_prefetches_inflight_per_host_per_navigation limit has
// not been reached. Try to launch as many requests as possible. // not been reached. Try to launch as many requests as possible.
while ((inflight_requests_.size() < while ((inflight_requests_.size() < max_concurrent_requests_) &&
config_.max_prefetches_inflight_per_navigation) &&
request_available) { request_available) {
auto request_it = request_queue_.begin(); auto request_it = request_queue_.begin();
for (; request_it != request_queue_.end(); ++request_it) { for (; request_it != request_queue_.end(); ++request_it) {
...@@ -109,8 +109,7 @@ void ResourcePrefetcher::TryToLaunchPrefetchRequests() { ...@@ -109,8 +109,7 @@ void ResourcePrefetcher::TryToLaunchPrefetchRequests() {
std::map<std::string, size_t>::iterator host_it = std::map<std::string, size_t>::iterator host_it =
host_inflight_counts_.find(host); host_inflight_counts_.find(host);
if (host_it == host_inflight_counts_.end() || if (host_it == host_inflight_counts_.end() ||
host_it->second < host_it->second < max_concurrent_requests_per_host_)
config_.max_prefetches_inflight_per_host_per_navigation)
break; break;
} }
request_available = request_it != request_queue_.end(); request_available = request_it != request_queue_.end();
......
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/threading/thread_checker.h" #include "base/threading/thread_checker.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/predictors/resource_prefetch_common.h"
#include "net/url_request/redirect_info.h" #include "net/url_request/redirect_info.h"
#include "net/url_request/url_request.h" #include "net/url_request/url_request.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -83,7 +82,8 @@ class ResourcePrefetcher : public net::URLRequest::Delegate { ...@@ -83,7 +82,8 @@ class ResourcePrefetcher : public net::URLRequest::Delegate {
// |delegate| has to outlive the ResourcePrefetcher. // |delegate| has to outlive the ResourcePrefetcher.
ResourcePrefetcher(Delegate* delegate, ResourcePrefetcher(Delegate* delegate,
const ResourcePrefetchPredictorConfig& config, size_t max_concurrent_requests,
size_t max_concurrent_requests_per_host,
const GURL& main_frame_url, const GURL& main_frame_url,
const std::vector<GURL>& urls); const std::vector<GURL>& urls);
~ResourcePrefetcher() override; ~ResourcePrefetcher() override;
...@@ -142,7 +142,8 @@ class ResourcePrefetcher : public net::URLRequest::Delegate { ...@@ -142,7 +142,8 @@ class ResourcePrefetcher : public net::URLRequest::Delegate {
base::ThreadChecker thread_checker_; base::ThreadChecker thread_checker_;
PrefetcherState state_; PrefetcherState state_;
Delegate* const delegate_; Delegate* const delegate_;
ResourcePrefetchPredictorConfig const config_; size_t max_concurrent_requests_;
size_t max_concurrent_requests_per_host_;
GURL main_frame_url_; GURL main_frame_url_;
// For histogram reports. // For histogram reports.
......
...@@ -60,8 +60,10 @@ void ResourcePrefetcherManager::MaybeAddPrefetch( ...@@ -60,8 +60,10 @@ void ResourcePrefetcherManager::MaybeAddPrefetch(
if (base::ContainsKey(prefetcher_map_, key)) if (base::ContainsKey(prefetcher_map_, key))
return; return;
auto prefetcher = auto prefetcher = base::MakeUnique<ResourcePrefetcher>(
base::MakeUnique<ResourcePrefetcher>(this, config_, main_frame_url, urls); this, config_.max_prefetches_inflight_per_navigation,
config_.max_prefetches_inflight_per_host_per_navigation, main_frame_url,
urls);
prefetcher->Start(); prefetcher->Start();
prefetcher_map_[key] = std::move(prefetcher); prefetcher_map_[key] = std::move(prefetcher);
} }
......
...@@ -30,15 +30,23 @@ using testing::Property; ...@@ -30,15 +30,23 @@ using testing::Property;
namespace predictors { namespace predictors {
constexpr size_t kMaxConcurrentRequests = 5;
constexpr size_t kMaxConcurrentRequestsPerHost = 2;
// Wrapper over the ResourcePrefetcher that stubs out the StartURLRequest call // Wrapper over the ResourcePrefetcher that stubs out the StartURLRequest call
// since we do not want to do network fetches in this unittest. // since we do not want to do network fetches in this unittest.
class TestResourcePrefetcher : public ResourcePrefetcher { class TestResourcePrefetcher : public ResourcePrefetcher {
public: public:
TestResourcePrefetcher(ResourcePrefetcher::Delegate* delegate, TestResourcePrefetcher(ResourcePrefetcher::Delegate* delegate,
const ResourcePrefetchPredictorConfig& config, size_t max_concurrent_requests,
size_t max_concurrent_requests_per_host,
const GURL& main_frame_url, const GURL& main_frame_url,
const std::vector<GURL>& urls) const std::vector<GURL>& urls)
: ResourcePrefetcher(delegate, config, main_frame_url, urls) {} : ResourcePrefetcher(delegate,
max_concurrent_requests,
max_concurrent_requests_per_host,
main_frame_url,
urls) {}
~TestResourcePrefetcher() override {} ~TestResourcePrefetcher() override {}
...@@ -144,7 +152,6 @@ class ResourcePrefetcherTest : public testing::Test { ...@@ -144,7 +152,6 @@ class ResourcePrefetcherTest : public testing::Test {
base::MessageLoop loop_; base::MessageLoop loop_;
content::TestBrowserThread io_thread_; content::TestBrowserThread io_thread_;
ResourcePrefetchPredictorConfig config_;
TestResourcePrefetcherDelegate prefetcher_delegate_; TestResourcePrefetcherDelegate prefetcher_delegate_;
std::unique_ptr<TestResourcePrefetcher> prefetcher_; std::unique_ptr<TestResourcePrefetcher> prefetcher_;
...@@ -155,9 +162,7 @@ class ResourcePrefetcherTest : public testing::Test { ...@@ -155,9 +162,7 @@ class ResourcePrefetcherTest : public testing::Test {
ResourcePrefetcherTest::ResourcePrefetcherTest() ResourcePrefetcherTest::ResourcePrefetcherTest()
: loop_(base::MessageLoop::TYPE_IO), : loop_(base::MessageLoop::TYPE_IO),
io_thread_(content::BrowserThread::IO, &loop_), io_thread_(content::BrowserThread::IO, &loop_),
prefetcher_delegate_(&loop_) { prefetcher_delegate_(&loop_) {}
config_.max_prefetches_inflight_per_host_per_navigation = 2;
}
ResourcePrefetcherTest::~ResourcePrefetcherTest() { ResourcePrefetcherTest::~ResourcePrefetcherTest() {
} }
...@@ -177,8 +182,9 @@ TEST_F(ResourcePrefetcherTest, TestPrefetcherFinishes) { ...@@ -177,8 +182,9 @@ TEST_F(ResourcePrefetcherTest, TestPrefetcherFinishes) {
GURL("http://yahoo.com/resource4.png"), GURL("http://yahoo.com/resource4.png"),
GURL("http://yahoo.com/resource5.png")}; GURL("http://yahoo.com/resource5.png")};
prefetcher_.reset(new TestResourcePrefetcher(&prefetcher_delegate_, config_, prefetcher_ = base::MakeUnique<TestResourcePrefetcher>(
main_frame_url, urls)); &prefetcher_delegate_, kMaxConcurrentRequests,
kMaxConcurrentRequestsPerHost, main_frame_url, urls);
// Starting the prefetcher maxes out the number of possible requests. // Starting the prefetcher maxes out the number of possible requests.
AddStartUrlRequestExpectation("http://www.google.com/resource1.html"); AddStartUrlRequestExpectation("http://www.google.com/resource1.html");
...@@ -247,8 +253,9 @@ TEST_F(ResourcePrefetcherTest, TestPrefetcherStopped) { ...@@ -247,8 +253,9 @@ TEST_F(ResourcePrefetcherTest, TestPrefetcherStopped) {
GURL("http://yahoo.com/resource3.png"), GURL("http://yahoo.com/resource3.png"),
GURL("http://m.google.com/resource1.jpg")}; GURL("http://m.google.com/resource1.jpg")};
prefetcher_.reset(new TestResourcePrefetcher(&prefetcher_delegate_, config_, prefetcher_ = base::MakeUnique<TestResourcePrefetcher>(
main_frame_url, urls)); &prefetcher_delegate_, kMaxConcurrentRequests,
kMaxConcurrentRequestsPerHost, main_frame_url, urls);
// Starting the prefetcher maxes out the number of possible requests. // Starting the prefetcher maxes out the number of possible requests.
AddStartUrlRequestExpectation("http://www.google.com/resource1.html"); AddStartUrlRequestExpectation("http://www.google.com/resource1.html");
...@@ -293,7 +300,8 @@ TEST_F(ResourcePrefetcherTest, TestHistogramsCollected) { ...@@ -293,7 +300,8 @@ TEST_F(ResourcePrefetcherTest, TestHistogramsCollected) {
GURL("http://www.google.com/resource6.png")}; GURL("http://www.google.com/resource6.png")};
prefetcher_ = base::MakeUnique<TestResourcePrefetcher>( prefetcher_ = base::MakeUnique<TestResourcePrefetcher>(
&prefetcher_delegate_, config_, main_frame_url, urls); &prefetcher_delegate_, kMaxConcurrentRequests,
kMaxConcurrentRequestsPerHost, main_frame_url, urls);
// Starting the prefetcher maxes out the number of possible requests. // Starting the prefetcher maxes out the number of possible requests.
AddStartUrlRequestExpectation("http://www.google.com/resource1.png"); AddStartUrlRequestExpectation("http://www.google.com/resource1.png");
...@@ -341,7 +349,8 @@ TEST_F(ResourcePrefetcherTest, TestReferrer) { ...@@ -341,7 +349,8 @@ TEST_F(ResourcePrefetcherTest, TestReferrer) {
std::vector<GURL> urls = {GURL(https_resource), GURL(http_resource)}; std::vector<GURL> urls = {GURL(https_resource), GURL(http_resource)};
prefetcher_ = base::MakeUnique<TestResourcePrefetcher>( prefetcher_ = base::MakeUnique<TestResourcePrefetcher>(
&prefetcher_delegate_, config_, GURL(url), urls); &prefetcher_delegate_, kMaxConcurrentRequests,
kMaxConcurrentRequestsPerHost, GURL(url), urls);
AddStartUrlRequestExpectation(https_resource); AddStartUrlRequestExpectation(https_resource);
AddStartUrlRequestExpectation(http_resource); AddStartUrlRequestExpectation(http_resource);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment