diff options
author | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-22 20:17:46 +0000 |
---|---|---|
committer | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-22 20:17:46 +0000 |
commit | 5033ab8cdfdfd67f92195f6b9d7a71e35ffa0090 (patch) | |
tree | df91bd6e3b25a948557026a559833531922df6e3 /net/url_request | |
parent | f43fba7ce6713432a8301aee47e9a7030ddca082 (diff) | |
download | chromium_src-5033ab8cdfdfd67f92195f6b9d7a71e35ffa0090.zip chromium_src-5033ab8cdfdfd67f92195f6b9d7a71e35ffa0090.tar.gz chromium_src-5033ab8cdfdfd67f92195f6b9d7a71e35ffa0090.tar.bz2 |
[Net] Propagate priority changes from URLRequest to HttpTransaction
This is in preparation for propagating priority changes from
ResourceScheduler all the way to HostResolver and ClientSocketPool.
Add some NetLog events and parameters for priority.
BUG=166689
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=189829
Review URL: https://codereview.chromium.org/12701011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@189894 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/url_request')
-rw-r--r-- | net/url_request/url_request.cc | 15 | ||||
-rw-r--r-- | net/url_request/url_request.h | 8 | ||||
-rw-r--r-- | net/url_request/url_request_ftp_job.cc | 79 | ||||
-rw-r--r-- | net/url_request/url_request_ftp_job.h | 17 | ||||
-rw-r--r-- | net/url_request/url_request_ftp_job_unittest.cc | 106 | ||||
-rw-r--r-- | net/url_request/url_request_http_job.cc | 159 | ||||
-rw-r--r-- | net/url_request/url_request_http_job.h | 124 | ||||
-rw-r--r-- | net/url_request/url_request_http_job_unittest.cc | 120 | ||||
-rw-r--r-- | net/url_request/url_request_job.cc | 3 | ||||
-rw-r--r-- | net/url_request/url_request_job.h | 8 | ||||
-rw-r--r-- | net/url_request/url_request_test_job.cc | 8 | ||||
-rw-r--r-- | net/url_request/url_request_test_job.h | 5 | ||||
-rw-r--r-- | net/url_request/url_request_unittest.cc | 82 |
13 files changed, 553 insertions, 181 deletions
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc index 5e426d1..245f389 100644 --- a/net/url_request/url_request.cc +++ b/net/url_request/url_request.cc @@ -526,6 +526,7 @@ void URLRequest::StartJob(URLRequestJob* job) { job_ = job; job_->SetExtraRequestHeaders(extra_request_headers_); + job_->SetPriority(priority_); if (upload_data_stream_.get()) job_->SetUpload(upload_data_stream_.get()); @@ -828,6 +829,20 @@ int64 URLRequest::GetExpectedContentSize() const { return expected_content_size; } +void URLRequest::SetPriority(RequestPriority priority) { + DCHECK_GE(priority, MINIMUM_PRIORITY); + DCHECK_LT(priority, NUM_PRIORITIES); + if (priority_ == priority) + return; + + priority_ = priority; + if (job_) { + net_log_.AddEvent(NetLog::TYPE_URL_REQUEST_SET_PRIORITY, + NetLog::IntegerCallback("priority", priority_)); + job_->SetPriority(priority_); + } +} + bool URLRequest::GetHSTSRedirect(GURL* redirect_url) const { const GURL& url = this->url(); if (!url.SchemeIs("http")) diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h index 5580b63..1e5a95b 100644 --- a/net/url_request/url_request.h +++ b/net/url_request/url_request.h @@ -615,11 +615,9 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe), // Returns the priority level for this request. RequestPriority priority() const { return priority_; } - void set_priority(RequestPriority priority) { - DCHECK_GE(priority, MINIMUM_PRIORITY); - DCHECK_LT(priority, NUM_PRIORITIES); - priority_ = priority; - } + + // Sets the priority level for this request and any related jobs. + void SetPriority(RequestPriority priority); // Returns true iff this request would be internally redirected to HTTPS // due to HSTS. If so, |redirect_url| is rewritten to the new HTTPS URL. diff --git a/net/url_request/url_request_ftp_job.cc b/net/url_request/url_request_ftp_job.cc index ebb540d..cc3e3f8 100644 --- a/net/url_request/url_request_ftp_job.cc +++ b/net/url_request/url_request_ftp_job.cc @@ -28,6 +28,7 @@ URLRequestFtpJob::URLRequestFtpJob( FtpTransactionFactory* ftp_transaction_factory, FtpAuthCache* ftp_auth_cache) : URLRequestJob(request, network_delegate), + priority_(DEFAULT_PRIORITY), pac_request_(NULL), response_info_(NULL), read_in_progress_(false), @@ -38,6 +39,11 @@ URLRequestFtpJob::URLRequestFtpJob( DCHECK(ftp_auth_cache); } +URLRequestFtpJob::~URLRequestFtpJob() { + if (pac_request_) + request_->context()->proxy_service()->CancelPacRequest(pac_request_); +} + // static URLRequestJob* URLRequestFtpJob::Factory(URLRequest* request, NetworkDelegate* network_delegate, @@ -95,9 +101,42 @@ HostPortPair URLRequestFtpJob::GetSocketAddress() const { } } -URLRequestFtpJob::~URLRequestFtpJob() { - if (pac_request_) - request_->context()->proxy_service()->CancelPacRequest(pac_request_); +void URLRequestFtpJob::SetPriority(RequestPriority priority) { + priority_ = priority; + if (http_transaction_) + http_transaction_->SetPriority(priority); +} + +void URLRequestFtpJob::Start() { + DCHECK(!pac_request_); + DCHECK(!ftp_transaction_); + DCHECK(!http_transaction_); + + int rv = OK; + if (request_->load_flags() & LOAD_BYPASS_PROXY) { + proxy_info_.UseDirect(); + } else { + rv = request_->context()->proxy_service()->ResolveProxy( + request_->url(), + &proxy_info_, + base::Bind(&URLRequestFtpJob::OnResolveProxyComplete, + base::Unretained(this)), + &pac_request_, + request_->net_log()); + + if (rv == ERR_IO_PENDING) + return; + } + OnResolveProxyComplete(rv); +} + +void URLRequestFtpJob::Kill() { + if (ftp_transaction_) + ftp_transaction_.reset(); + if (http_transaction_) + http_transaction_.reset(); + URLRequestJob::Kill(); + weak_factory_.InvalidateWeakPtrs(); } void URLRequestFtpJob::OnResolveProxyComplete(int result) { @@ -168,7 +207,7 @@ void URLRequestFtpJob::StartHttpTransaction() { http_request_info_.request_id = request_->identifier(); int rv = request_->context()->http_transaction_factory()->CreateTransaction( - request_->priority(), &http_transaction_, NULL); + priority_, &http_transaction_, NULL); if (rv == OK) { rv = http_transaction_->Start( &http_request_info_, @@ -260,38 +299,6 @@ void URLRequestFtpJob::RestartTransactionWithAuth() { OnStartCompletedAsync(rv); } -void URLRequestFtpJob::Start() { - DCHECK(!pac_request_); - DCHECK(!ftp_transaction_); - DCHECK(!http_transaction_); - - int rv = OK; - if (request_->load_flags() & LOAD_BYPASS_PROXY) { - proxy_info_.UseDirect(); - } else { - rv = request_->context()->proxy_service()->ResolveProxy( - request_->url(), - &proxy_info_, - base::Bind(&URLRequestFtpJob::OnResolveProxyComplete, - base::Unretained(this)), - &pac_request_, - request_->net_log()); - - if (rv == ERR_IO_PENDING) - return; - } - OnResolveProxyComplete(rv); -} - -void URLRequestFtpJob::Kill() { - if (ftp_transaction_) - ftp_transaction_.reset(); - if (http_transaction_) - http_transaction_.reset(); - URLRequestJob::Kill(); - weak_factory_.InvalidateWeakPtrs(); -} - LoadState URLRequestFtpJob::GetLoadState() const { if (proxy_info_.is_direct()) { return ftp_transaction_ ? diff --git a/net/url_request/url_request_ftp_job.h b/net/url_request/url_request_ftp_job.h index 3b053c1..a739891 100644 --- a/net/url_request/url_request_ftp_job.h +++ b/net/url_request/url_request_ftp_job.h @@ -9,6 +9,7 @@ #include "base/memory/weak_ptr.h" #include "net/base/auth.h" +#include "net/base/net_export.h" #include "net/ftp/ftp_request_info.h" #include "net/ftp/ftp_transaction.h" #include "net/http/http_request_info.h" @@ -25,7 +26,7 @@ class FtpAuthCache; // A URLRequestJob subclass that is built on top of FtpTransaction. It // provides an implementation for FTP. -class URLRequestFtpJob : public URLRequestJob { +class NET_EXPORT_PRIVATE URLRequestFtpJob : public URLRequestJob { public: URLRequestFtpJob(URLRequest* request, NetworkDelegate* network_delegate, @@ -37,15 +38,21 @@ class URLRequestFtpJob : public URLRequestJob { NetworkDelegate* network_delegate, const std::string& scheme); + protected: + virtual ~URLRequestFtpJob(); + // Overridden from URLRequestJob: virtual bool IsSafeRedirect(const GURL& location) OVERRIDE; virtual bool GetMimeType(std::string* mime_type) const OVERRIDE; virtual void GetResponseInfo(HttpResponseInfo* info) OVERRIDE; virtual HostPortPair GetSocketAddress() const OVERRIDE; + virtual void SetPriority(RequestPriority priority) OVERRIDE; + virtual void Start() OVERRIDE; + virtual void Kill() OVERRIDE; - private: - virtual ~URLRequestFtpJob(); + RequestPriority priority() const { return priority_; } + private: void OnResolveProxyComplete(int result); void StartFtpTransaction(); @@ -60,8 +67,6 @@ class URLRequestFtpJob : public URLRequestJob { void LogFtpServerType(char server_type); // Overridden from URLRequestJob: - virtual void Start() OVERRIDE; - virtual void Kill() OVERRIDE; virtual LoadState GetLoadState() const OVERRIDE; virtual bool NeedsAuth() OVERRIDE; virtual void GetAuthChallengeInfo( @@ -75,6 +80,8 @@ class URLRequestFtpJob : public URLRequestJob { int buf_size, int *bytes_read) OVERRIDE; + RequestPriority priority_; + ProxyInfo proxy_info_; ProxyService::PacRequest* pac_request_; diff --git a/net/url_request/url_request_ftp_job_unittest.cc b/net/url_request/url_request_ftp_job_unittest.cc index 61dde753..7eec8ee 100644 --- a/net/url_request/url_request_ftp_job_unittest.cc +++ b/net/url_request/url_request_ftp_job_unittest.cc @@ -2,8 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "net/url_request/url_request_ftp_job.h" + +#include "base/memory/ref_counted.h" #include "base/memory/scoped_vector.h" #include "base/run_loop.h" +#include "googleurl/src/gurl.h" +#include "net/http/http_transaction_unittest.h" #include "net/proxy/proxy_config_service.h" #include "net/socket/socket_test_util.h" #include "net/url_request/url_request.h" @@ -49,6 +54,107 @@ class SimpleProxyConfigService : public ProxyConfigService { Observer* observer_; }; +// Inherit from URLRequestFtpJob to expose the priority and some +// other hidden functions. +class TestURLRequestFtpJob : public URLRequestFtpJob { + public: + explicit TestURLRequestFtpJob(URLRequest* request) + : URLRequestFtpJob(request, NULL, + request->context()->ftp_transaction_factory(), + request->context()->ftp_auth_cache()) {} + + using URLRequestFtpJob::SetPriority; + using URLRequestFtpJob::Start; + using URLRequestFtpJob::Kill; + using URLRequestFtpJob::priority; + + protected: + virtual ~TestURLRequestFtpJob() {} +}; + +// Fixture for priority-related tests. Priority matters when there is +// an HTTP proxy. +class URLRequestFtpJobPriorityTest : public testing::Test { + protected: + URLRequestFtpJobPriorityTest() + : proxy_service_(new SimpleProxyConfigService, NULL, NULL), + req_(GURL("ftp://ftp.example.com"), &delegate_, &context_, NULL) { + context_.set_proxy_service(&proxy_service_); + context_.set_http_transaction_factory(&network_layer_); + } + + ProxyService proxy_service_; + MockNetworkLayer network_layer_; + TestURLRequestContext context_; + TestDelegate delegate_; + TestURLRequest req_; +}; + +// Make sure that SetPriority actually sets the URLRequestFtpJob's +// priority, both before and after start. +TEST_F(URLRequestFtpJobPriorityTest, SetPriorityBasic) { + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + EXPECT_EQ(DEFAULT_PRIORITY, job->priority()); + + job->SetPriority(LOWEST); + EXPECT_EQ(LOWEST, job->priority()); + + job->SetPriority(LOW); + EXPECT_EQ(LOW, job->priority()); + + job->Start(); + EXPECT_EQ(LOW, job->priority()); + + job->SetPriority(MEDIUM); + EXPECT_EQ(MEDIUM, job->priority()); +} + +// Make sure that URLRequestFtpJob passes on its priority to its +// transaction on start. +TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriorityOnStart) { + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + job->SetPriority(LOW); + + EXPECT_FALSE(network_layer_.last_transaction()); + + job->Start(); + + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); +} + +// Make sure that URLRequestFtpJob passes on its priority updates to +// its transaction. +TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriority) { + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + job->SetPriority(LOW); + job->Start(); + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); + + job->SetPriority(HIGHEST); + EXPECT_EQ(HIGHEST, network_layer_.last_transaction()->priority()); +} + +// Make sure that URLRequestFtpJob passes on its priority updates to +// newly-created transactions after the first one. +TEST_F(URLRequestFtpJobPriorityTest, SetSubsequentTransactionPriority) { + scoped_refptr<TestURLRequestFtpJob> job(new TestURLRequestFtpJob(&req_)); + job->Start(); + + job->SetPriority(LOW); + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); + + job->Kill(); + network_layer_.ClearLastTransaction(); + + // Creates a second transaction. + job->Start(); + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); +} + class FtpTestURLRequestContext : public TestURLRequestContext { public: FtpTestURLRequestContext(ClientSocketFactory* socket_factory, diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index fcf296b..e2cd99d 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -228,6 +228,7 @@ URLRequestHttpJob::URLRequestHttpJob( NetworkDelegate* network_delegate, const HttpUserAgentSettings* http_user_agent_settings) : URLRequestJob(request, network_delegate), + priority_(DEFAULT_PRIORITY), response_info_(NULL), response_cookies_save_index_(0), proxy_auth_state_(AUTH_STATE_DONT_NEED_AUTH), @@ -268,6 +269,87 @@ URLRequestHttpJob::URLRequestHttpJob( ResetTimer(); } +URLRequestHttpJob::~URLRequestHttpJob() { + CHECK(!awaiting_callback_); + + DCHECK(!sdch_test_control_ || !sdch_test_activated_); + if (!is_cached_content_) { + if (sdch_test_control_) + RecordPacketStats(FilterContext::SDCH_EXPERIMENT_HOLDBACK); + if (sdch_test_activated_) + RecordPacketStats(FilterContext::SDCH_EXPERIMENT_DECODE); + } + // Make sure SDCH filters are told to emit histogram data while + // filter_context_ is still alive. + DestroyFilters(); + + if (sdch_dictionary_url_.is_valid()) { + // Prior to reaching the destructor, request_ has been set to a NULL + // pointer, so request_->url() is no longer valid in the destructor, and we + // use an alternate copy |request_info_.url|. + SdchManager* manager = SdchManager::Global(); + // To be extra safe, since this is a "different time" from when we decided + // to get the dictionary, we'll validate that an SdchManager is available. + // At shutdown time, care is taken to be sure that we don't delete this + // globally useful instance "too soon," so this check is just defensive + // coding to assure that IF the system is shutting down, we don't have any + // problem if the manager was deleted ahead of time. + if (manager) // Defensive programming. + manager->FetchDictionary(request_info_.url, sdch_dictionary_url_); + } + DoneWithRequest(ABORTED); +} + +void URLRequestHttpJob::SetPriority(RequestPriority priority) { + priority_ = priority; + if (transaction_) + transaction_->SetPriority(priority_); +} + +void URLRequestHttpJob::Start() { + DCHECK(!transaction_.get()); + + // Ensure that we do not send username and password fields in the referrer. + GURL referrer(request_->GetSanitizedReferrer()); + + request_info_.url = request_->url(); + request_info_.method = request_->method(); + request_info_.load_flags = request_->load_flags(); + request_info_.request_id = request_->identifier(); + + // Strip Referer from request_info_.extra_headers to prevent, e.g., plugins + // from overriding headers that are controlled using other means. Otherwise a + // plugin could set a referrer although sending the referrer is inhibited. + request_info_.extra_headers.RemoveHeader(HttpRequestHeaders::kReferer); + + // Our consumer should have made sure that this is a safe referrer. See for + // instance WebCore::FrameLoader::HideReferrer. + if (referrer.is_valid()) { + request_info_.extra_headers.SetHeader(HttpRequestHeaders::kReferer, + referrer.spec()); + } + + request_info_.extra_headers.SetHeaderIfMissing( + HttpRequestHeaders::kUserAgent, + http_user_agent_settings_ ? + http_user_agent_settings_->GetUserAgent(request_->url()) : + EmptyString()); + + AddExtraHeaders(); + AddCookieHeaderAndStart(); +} + +void URLRequestHttpJob::Kill() { + http_transaction_delegate_->OnDetachRequest(); + + if (!transaction_.get()) + return; + + weak_factory_.InvalidateWeakPtrs(); + DestroyTransaction(); + URLRequestJob::Kill(); +} + void URLRequestHttpJob::NotifyHeadersComplete() { DCHECK(!response_info_); @@ -394,7 +476,7 @@ void URLRequestHttpJob::StartTransactionInternal() { DCHECK(request_->context()->http_transaction_factory()); rv = request_->context()->http_transaction_factory()->CreateTransaction( - request_->priority(), &transaction_, http_transaction_delegate_.get()); + priority_, &transaction_, http_transaction_delegate_.get()); if (rv == OK) { if (!throttling_entry_ || !throttling_entry_->ShouldRejectRequest(*request_)) { @@ -859,50 +941,6 @@ void URLRequestHttpJob::SetExtraRequestHeaders( request_info_.extra_headers.CopyFrom(headers); } -void URLRequestHttpJob::Start() { - DCHECK(!transaction_.get()); - - // Ensure that we do not send username and password fields in the referrer. - GURL referrer(request_->GetSanitizedReferrer()); - - request_info_.url = request_->url(); - request_info_.method = request_->method(); - request_info_.load_flags = request_->load_flags(); - request_info_.request_id = request_->identifier(); - - // Strip Referer from request_info_.extra_headers to prevent, e.g., plugins - // from overriding headers that are controlled using other means. Otherwise a - // plugin could set a referrer although sending the referrer is inhibited. - request_info_.extra_headers.RemoveHeader(HttpRequestHeaders::kReferer); - - // Our consumer should have made sure that this is a safe referrer. See for - // instance WebCore::FrameLoader::HideReferrer. - if (referrer.is_valid()) { - request_info_.extra_headers.SetHeader(HttpRequestHeaders::kReferer, - referrer.spec()); - } - - request_info_.extra_headers.SetHeaderIfMissing( - HttpRequestHeaders::kUserAgent, - http_user_agent_settings_ ? - http_user_agent_settings_->GetUserAgent(request_->url()) : - EmptyString()); - - AddExtraHeaders(); - AddCookieHeaderAndStart(); -} - -void URLRequestHttpJob::Kill() { - http_transaction_delegate_->OnDetachRequest(); - - if (!transaction_.get()) - return; - - weak_factory_.InvalidateWeakPtrs(); - DestroyTransaction(); - URLRequestJob::Kill(); -} - LoadState URLRequestHttpJob::GetLoadState() const { return transaction_.get() ? transaction_->GetLoadState() : LOAD_STATE_IDLE; @@ -1230,37 +1268,6 @@ HostPortPair URLRequestHttpJob::GetSocketAddress() const { return response_info_ ? response_info_->socket_address : HostPortPair(); } -URLRequestHttpJob::~URLRequestHttpJob() { - CHECK(!awaiting_callback_); - - DCHECK(!sdch_test_control_ || !sdch_test_activated_); - if (!is_cached_content_) { - if (sdch_test_control_) - RecordPacketStats(FilterContext::SDCH_EXPERIMENT_HOLDBACK); - if (sdch_test_activated_) - RecordPacketStats(FilterContext::SDCH_EXPERIMENT_DECODE); - } - // Make sure SDCH filters are told to emit histogram data while - // filter_context_ is still alive. - DestroyFilters(); - - if (sdch_dictionary_url_.is_valid()) { - // Prior to reaching the destructor, request_ has been set to a NULL - // pointer, so request_->url() is no longer valid in the destructor, and we - // use an alternate copy |request_info_.url|. - SdchManager* manager = SdchManager::Global(); - // To be extra safe, since this is a "different time" from when we decided - // to get the dictionary, we'll validate that an SdchManager is available. - // At shutdown time, care is taken to be sure that we don't delete this - // globally useful instance "too soon," so this check is just defensive - // coding to assure that IF the system is shutting down, we don't have any - // problem if the manager was deleted ahead of time. - if (manager) // Defensive programming. - manager->FetchDictionary(request_info_.url, sdch_dictionary_url_); - } - DoneWithRequest(ABORTED); -} - void URLRequestHttpJob::RecordTimer() { if (request_creation_time_.is_null()) { NOTREACHED() diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h index 8020625..73d216f 100644 --- a/net/url_request/url_request_http_job.h +++ b/net/url_request/url_request_http_job.h @@ -14,6 +14,7 @@ #include "base/time.h" #include "net/base/auth.h" #include "net/base/completion_callback.h" +#include "net/base/net_export.h" #include "net/cookies/cookie_store.h" #include "net/http/http_request_info.h" #include "net/url_request/url_request_job.h" @@ -30,7 +31,7 @@ class URLRequestContext; // A URLRequestJob subclass that is built on top of HttpTransaction. It // provides an implementation for both HTTP and HTTPS. -class URLRequestHttpJob : public URLRequestJob { +class NET_EXPORT_PRIVATE URLRequestHttpJob : public URLRequestJob { public: static URLRequestJob* Factory(URLRequest* request, NetworkDelegate* network_delegate, @@ -41,6 +42,28 @@ class URLRequestHttpJob : public URLRequestJob { NetworkDelegate* network_delegate, const HttpUserAgentSettings* http_user_agent_settings); + virtual ~URLRequestHttpJob(); + + // Overridden from URLRequestJob: + virtual void SetPriority(RequestPriority priority) OVERRIDE; + virtual void Start() OVERRIDE; + virtual void Kill() OVERRIDE; + + RequestPriority priority() const { + return priority_; + } + + private: + enum CompletionCause { + ABORTED, + FINISHED + }; + + typedef base::RefCountedData<bool> SharedBoolean; + + class HttpFilterContext; + class HttpTransactionDelegateImpl; + // Shadows URLRequestJob's version of this method so we can grab cookies. void NotifyHeadersComplete(); @@ -73,8 +96,6 @@ class URLRequestHttpJob : public URLRequestJob { virtual void SetUpload(UploadDataStream* upload) OVERRIDE; virtual void SetExtraRequestHeaders( const HttpRequestHeaders& headers) OVERRIDE; - virtual void Start() OVERRIDE; - virtual void Kill() OVERRIDE; virtual LoadState GetLoadState() const OVERRIDE; virtual UploadProgress GetUploadProgress() const OVERRIDE; virtual bool GetMimeType(std::string* mime_type) const OVERRIDE; @@ -99,60 +120,6 @@ class URLRequestHttpJob : public URLRequestJob { virtual HostPortPair GetSocketAddress() const OVERRIDE; virtual void NotifyURLRequestDestroyed() OVERRIDE; - HttpRequestInfo request_info_; - const HttpResponseInfo* response_info_; - - std::vector<std::string> response_cookies_; - size_t response_cookies_save_index_; - base::Time response_date_; - - // Auth states for proxy and origin server. - AuthState proxy_auth_state_; - AuthState server_auth_state_; - AuthCredentials auth_credentials_; - - CompletionCallback start_callback_; - CompletionCallback notify_before_headers_sent_callback_; - - bool read_in_progress_; - - // An URL for an SDCH dictionary as suggested in a Get-Dictionary HTTP header. - GURL sdch_dictionary_url_; - - scoped_ptr<HttpTransaction> transaction_; - - // This is used to supervise traffic and enforce exponential - // back-off. May be NULL. - scoped_refptr<URLRequestThrottlerEntryInterface> throttling_entry_; - - // Indicated if an SDCH dictionary was advertised, and hence an SDCH - // compressed response is expected. We use this to help detect (accidental?) - // proxy corruption of a response, which sometimes marks SDCH content as - // having no content encoding <oops>. - bool sdch_dictionary_advertised_; - - // For SDCH latency experiments, when we are able to do SDCH, we may enable - // either an SDCH latency test xor a pass through test. The following bools - // indicate what we decided on for this instance. - bool sdch_test_activated_; // Advertising a dictionary for sdch. - bool sdch_test_control_; // Not even accepting-content sdch. - - // For recording of stats, we need to remember if this is cached content. - bool is_cached_content_; - - private: - enum CompletionCause { - ABORTED, - FINISHED - }; - - typedef base::RefCountedData<bool> SharedBoolean; - - class HttpFilterContext; - class HttpTransactionDelegateImpl; - - virtual ~URLRequestHttpJob(); - void RecordTimer(); void ResetTimer(); @@ -199,6 +166,49 @@ class URLRequestHttpJob : public URLRequestJob { // Override of the private interface of URLRequestJob. virtual void OnDetachRequest() OVERRIDE; + RequestPriority priority_; + + HttpRequestInfo request_info_; + const HttpResponseInfo* response_info_; + + std::vector<std::string> response_cookies_; + size_t response_cookies_save_index_; + base::Time response_date_; + + // Auth states for proxy and origin server. + AuthState proxy_auth_state_; + AuthState server_auth_state_; + AuthCredentials auth_credentials_; + + CompletionCallback start_callback_; + CompletionCallback notify_before_headers_sent_callback_; + + bool read_in_progress_; + + // An URL for an SDCH dictionary as suggested in a Get-Dictionary HTTP header. + GURL sdch_dictionary_url_; + + scoped_ptr<HttpTransaction> transaction_; + + // This is used to supervise traffic and enforce exponential + // back-off. May be NULL. + scoped_refptr<URLRequestThrottlerEntryInterface> throttling_entry_; + + // Indicated if an SDCH dictionary was advertised, and hence an SDCH + // compressed response is expected. We use this to help detect (accidental?) + // proxy corruption of a response, which sometimes marks SDCH content as + // having no content encoding <oops>. + bool sdch_dictionary_advertised_; + + // For SDCH latency experiments, when we are able to do SDCH, we may enable + // either an SDCH latency test xor a pass through test. The following bools + // indicate what we decided on for this instance. + bool sdch_test_activated_; // Advertising a dictionary for sdch. + bool sdch_test_control_; // Not even accepting-content sdch. + + // For recording of stats, we need to remember if this is cached content. + bool is_cached_content_; + base::Time request_creation_time_; // Data used for statistics gathering. This data is only used for histograms diff --git a/net/url_request/url_request_http_job_unittest.cc b/net/url_request/url_request_http_job_unittest.cc new file mode 100644 index 0000000..9469a67 --- /dev/null +++ b/net/url_request/url_request_http_job_unittest.cc @@ -0,0 +1,120 @@ +// Copyright (c) 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 "net/url_request/url_request_http_job.h" + +#include <cstddef> + +#include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" +#include "googleurl/src/gurl.h" +#include "net/base/auth.h" +#include "net/http/http_transaction_factory.h" +#include "net/http/http_transaction_unittest.h" +#include "net/url_request/url_request_status.h" +#include "net/url_request/url_request_test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace net { + +namespace { + +// Inherit from URLRequestHttpJob to expose the priority and some +// other hidden functions. +class TestURLRequestHttpJob : public URLRequestHttpJob { + public: + explicit TestURLRequestHttpJob(URLRequest* request) + : URLRequestHttpJob(request, NULL, + request->context()->http_user_agent_settings()) {} + + using URLRequestHttpJob::SetPriority; + using URLRequestHttpJob::Start; + using URLRequestHttpJob::Kill; + using URLRequestHttpJob::priority; + + protected: + virtual ~TestURLRequestHttpJob() {} +}; + +class URLRequestHttpJobTest : public ::testing::Test { + protected: + URLRequestHttpJobTest() + : req_(GURL("http://www.example.com"), &delegate_, &context_, NULL) { + context_.set_http_transaction_factory(&network_layer_); + } + + MockNetworkLayer network_layer_; + TestURLRequestContext context_; + TestDelegate delegate_; + TestURLRequest req_; +}; + +// Make sure that SetPriority actually sets the URLRequestHttpJob's +// priority, both before and after start. +TEST_F(URLRequestHttpJobTest, SetPriorityBasic) { + scoped_refptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(&req_)); + EXPECT_EQ(DEFAULT_PRIORITY, job->priority()); + + job->SetPriority(LOWEST); + EXPECT_EQ(LOWEST, job->priority()); + + job->SetPriority(LOW); + EXPECT_EQ(LOW, job->priority()); + + job->Start(); + EXPECT_EQ(LOW, job->priority()); + + job->SetPriority(MEDIUM); + EXPECT_EQ(MEDIUM, job->priority()); +} + +// Make sure that URLRequestHttpJob passes on its priority to its +// transaction on start. +TEST_F(URLRequestHttpJobTest, SetTransactionPriorityOnStart) { + scoped_refptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(&req_)); + job->SetPriority(LOW); + + EXPECT_FALSE(network_layer_.last_transaction()); + + job->Start(); + + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); +} + +// Make sure that URLRequestHttpJob passes on its priority updates to +// its transaction. +TEST_F(URLRequestHttpJobTest, SetTransactionPriority) { + scoped_refptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(&req_)); + job->SetPriority(LOW); + job->Start(); + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); + + job->SetPriority(HIGHEST); + EXPECT_EQ(HIGHEST, network_layer_.last_transaction()->priority()); +} + +// Make sure that URLRequestHttpJob passes on its priority updates to +// newly-created transactions after the first one. +TEST_F(URLRequestHttpJobTest, SetSubsequentTransactionPriority) { + scoped_refptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(&req_)); + job->Start(); + + job->SetPriority(LOW); + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); + + job->Kill(); + network_layer_.ClearLastTransaction(); + + // Creates a second transaction. + job->Start(); + ASSERT_TRUE(network_layer_.last_transaction()); + EXPECT_EQ(LOW, network_layer_.last_transaction()->priority()); +} + +} // namespace + +} // namespace net diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc index 6bbaa29..3c470a4 100644 --- a/net/url_request/url_request_job.cc +++ b/net/url_request/url_request_job.cc @@ -45,6 +45,9 @@ void URLRequestJob::SetUpload(UploadDataStream* upload) { void URLRequestJob::SetExtraRequestHeaders(const HttpRequestHeaders& headers) { } +void URLRequestJob::SetPriority(RequestPriority priority) { +} + void URLRequestJob::Kill() { weak_factory_.InvalidateWeakPtrs(); // Make sure the request is notified that we are done. We assume that the diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h index 56e1307..ae95673 100644 --- a/net/url_request/url_request_job.h +++ b/net/url_request/url_request_job.h @@ -17,6 +17,7 @@ #include "net/base/host_port_pair.h" #include "net/base/load_states.h" #include "net/base/net_export.h" +#include "net/base/request_priority.h" #include "net/base/upload_progress.h" #include "net/cookies/canonical_cookie.h" @@ -53,9 +54,14 @@ class NET_EXPORT URLRequestJob : public base::RefCounted<URLRequestJob>, // Job types supporting upload data will override this. virtual void SetUpload(UploadDataStream* upload_data_stream); - // Sets extra request headers for Job types that support request headers. + // Sets extra request headers for Job types that support request + // headers. Called once before Start() is called. virtual void SetExtraRequestHeaders(const HttpRequestHeaders& headers); + // Sets the priority of the job. Called once before Start() is + // called, but also when the priority of the parent request changes. + virtual void SetPriority(RequestPriority priority); + // If any error occurs while starting the Job, NotifyStartError should be // called. // This helps ensure that all errors follow more similar notification code diff --git a/net/url_request/url_request_test_job.cc b/net/url_request/url_request_test_job.cc index 05b562b..6a4fa5b 100644 --- a/net/url_request/url_request_test_job.cc +++ b/net/url_request/url_request_test_job.cc @@ -89,6 +89,7 @@ URLRequestTestJob::URLRequestTestJob(URLRequest* request, : URLRequestJob(request, network_delegate), auto_advance_(false), stage_(WAITING), + priority_(DEFAULT_PRIORITY), offset_(0), async_buf_(NULL), async_buf_size_(0), @@ -101,6 +102,7 @@ URLRequestTestJob::URLRequestTestJob(URLRequest* request, : URLRequestJob(request, network_delegate), auto_advance_(auto_advance), stage_(WAITING), + priority_(DEFAULT_PRIORITY), offset_(0), async_buf_(NULL), async_buf_size_(0), @@ -115,6 +117,7 @@ URLRequestTestJob::URLRequestTestJob(URLRequest* request, : URLRequestJob(request, network_delegate), auto_advance_(auto_advance), stage_(WAITING), + priority_(DEFAULT_PRIORITY), response_headers_(new HttpResponseHeaders(response_headers)), response_data_(response_data), offset_(0), @@ -137,6 +140,10 @@ bool URLRequestTestJob::GetMimeType(std::string* mime_type) const { return response_headers_->GetMimeType(mime_type); } +void URLRequestTestJob::SetPriority(RequestPriority priority) { + priority_ = priority; +} + void URLRequestTestJob::Start() { // Start reading asynchronously so that all error reporting and data // callbacks happen as they would for network requests. @@ -226,7 +233,6 @@ bool URLRequestTestJob::IsRedirectResponse(GURL* location, return true; } - void URLRequestTestJob::Kill() { stage_ = DONE; URLRequestJob::Kill(); diff --git a/net/url_request/url_request_test_job.h b/net/url_request/url_request_test_job.h index 0c46d1b..c564d30a 100644 --- a/net/url_request/url_request_test_job.h +++ b/net/url_request/url_request_test_job.h @@ -90,10 +90,13 @@ class NET_EXPORT_PRIVATE URLRequestTestJob : public URLRequestJob { bool auto_advance() { return auto_advance_; } void set_auto_advance(bool auto_advance) { auto_advance_ = auto_advance; } + RequestPriority priority() const { return priority_; } + // Factory method for protocol factory registration if callers don't subclass static URLRequest::ProtocolFactory Factory; // Job functions + virtual void SetPriority(RequestPriority priority) OVERRIDE; virtual void Start() OVERRIDE; virtual bool ReadRawData(IOBuffer* buf, int buf_size, @@ -132,6 +135,8 @@ class NET_EXPORT_PRIVATE URLRequestTestJob : public URLRequestJob { Stage stage_; + RequestPriority priority_; + // The headers the job should return, will be set in Start() if not provided // in the explicit ctor. scoped_refptr<HttpResponseHeaders> response_headers_; diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index ef98a65..a6c23da 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -1404,6 +1404,60 @@ TEST_F(URLRequestTest, RequestCompletionForEmptyResponse) { EXPECT_EQ(1, default_network_delegate_.completed_requests()); } +// Make sure that SetPriority actually sets the URLRequest's priority +// correctly, both before and after start. +TEST_F(URLRequestTest, SetPriorityBasic) { + TestDelegate d; + URLRequest req(GURL("http://test_intercept/foo"), &d, &default_context_); + EXPECT_EQ(DEFAULT_PRIORITY, req.priority()); + + req.SetPriority(LOW); + EXPECT_EQ(LOW, req.priority()); + + req.Start(); + EXPECT_EQ(LOW, req.priority()); + + req.SetPriority(MEDIUM); + EXPECT_EQ(MEDIUM, req.priority()); +} + +// Make sure that URLRequest calls SetPriority on a job before calling +// Start on it. +TEST_F(URLRequestTest, SetJobPriorityBeforeJobStart) { + TestDelegate d; + URLRequest req(GURL("http://test_intercept/foo"), &d, &default_context_); + EXPECT_EQ(DEFAULT_PRIORITY, req.priority()); + + scoped_refptr<URLRequestTestJob> job = + new URLRequestTestJob(&req, &default_network_delegate_); + AddTestInterceptor()->set_main_intercept_job(job); + EXPECT_EQ(DEFAULT_PRIORITY, job->priority()); + + req.SetPriority(LOW); + + req.Start(); + EXPECT_EQ(LOW, job->priority()); +} + +// Make sure that URLRequest passes on its priority updates to its +// job. +TEST_F(URLRequestTest, SetJobPriority) { + TestDelegate d; + URLRequest req(GURL("http://test_intercept/foo"), &d, &default_context_); + + scoped_refptr<URLRequestTestJob> job = + new URLRequestTestJob(&req, &default_network_delegate_); + AddTestInterceptor()->set_main_intercept_job(job); + + req.SetPriority(LOW); + req.Start(); + EXPECT_EQ(LOW, job->priority()); + + req.SetPriority(MEDIUM); + EXPECT_EQ(MEDIUM, req.priority()); + EXPECT_EQ(MEDIUM, job->priority()); +} + // TODO(droger): Support TestServer on iOS (see http://crbug.com/148666). #if !defined(OS_IOS) // A subclass of TestServer that uses a statically-configured hostname. This is @@ -4085,6 +4139,34 @@ TEST_F(URLRequestTestHTTP, EmptyHttpUserAgentSettings) { } } +// Make sure that URLRequest passes on its priority updates to +// newly-created jobs after the first one. +TEST_F(URLRequestTestHTTP, SetSubsequentJobPriority) { + ASSERT_TRUE(test_server_.Start()); + + TestDelegate d; + URLRequest req(test_server_.GetURL("empty.html"), &d, &default_context_); + EXPECT_EQ(DEFAULT_PRIORITY, req.priority()); + + scoped_refptr<URLRequestRedirectJob> redirect_job = + new URLRequestRedirectJob( + &req, &default_network_delegate_, test_server_.GetURL("echo"), + URLRequestRedirectJob::REDIRECT_302_FOUND); + AddTestInterceptor()->set_main_intercept_job(redirect_job); + + req.SetPriority(LOW); + req.Start(); + EXPECT_TRUE(req.is_pending()); + + scoped_refptr<URLRequestTestJob> job = + new URLRequestTestJob(&req, &default_network_delegate_); + AddTestInterceptor()->set_main_intercept_job(job); + + // Should trigger |job| to be started. + MessageLoop::current()->Run(); + EXPECT_EQ(LOW, job->priority()); +} + class HTTPSRequestTest : public testing::Test { public: HTTPSRequestTest() : default_context_(true) { |