From d55ad15d8c3e118d6f03dbf236d6b98c62d66fe9 Mon Sep 17 00:00:00 2001 From: "jar@chromium.org" Date: Tue, 17 Feb 2009 19:40:50 +0000 Subject: Improve modularity of recent SDCH dictionary load checkin I felt guilty that I allowed url_request_job to have some logic in it about whether it can load an SDCH dictinary or not. This patch pushed the logic back into sdch_manager (where it belongs). r=wtc Review URL: http://codereview.chromium.org/20379 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@9897 0039d316-1c4b-4281-b951-d872f2087c98 --- net/base/sdch_manager.cc | 6 ++++-- net/base/sdch_manager.h | 2 +- net/url_request/url_request_http_job.cc | 15 ++++++++++----- 3 files changed, 15 insertions(+), 8 deletions(-) (limited to 'net') diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc index 9097f88..1f1ff0a 100644 --- a/net/base/sdch_manager.cc +++ b/net/base/sdch_manager.cc @@ -172,8 +172,10 @@ bool SdchManager::CanFetchDictionary(const GURL& referring_url, return true; } -void SdchManager::FetchDictionary(const GURL& dictionary_url) { - if (fetcher_.get()) +void SdchManager::FetchDictionary(const GURL& request_url, + const GURL& dictionary_url) { + if (SdchManager::Global()->CanFetchDictionary(request_url, dictionary_url) && + fetcher_.get()) fetcher_->Schedule(dictionary_url); } diff --git a/net/base/sdch_manager.h b/net/base/sdch_manager.h index 2a4e5b5..d94ea7d 100644 --- a/net/base/sdch_manager.h +++ b/net/base/sdch_manager.h @@ -247,7 +247,7 @@ class SdchManager { // before the dictionary is actually loaded and added. // After the implied task does completes, the dictionary will have been // cached in memory. - void FetchDictionary(const GURL& dictionary_url); + void FetchDictionary(const GURL& request_url, const GURL& dictionary_url); // Security test function used before initiating a FetchDictionary. // Return true if fetch is legal. diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index c4d0563..e7e8b20 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -65,7 +65,11 @@ URLRequestHttpJob::URLRequestHttpJob(URLRequest* request) URLRequestHttpJob::~URLRequestHttpJob() { if (sdch_dictionary_url_.is_valid()) { - SdchManager::Global()->FetchDictionary(sdch_dictionary_url_); + // 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::Global()->FetchDictionary(request_info_.url, + sdch_dictionary_url_); } } @@ -454,10 +458,11 @@ void URLRequestHttpJob::NotifyHeadersComplete() { // Eventually we should wait until a dictionary is requested several times // before we even download it (so that we don't waste memory or bandwidth). if (response_info_->headers->EnumerateHeader(&iter, name, &url_text)) { - GURL dictionary_url = request_->url().Resolve(url_text); - if (SdchManager::Global()->CanFetchDictionary(request_->url(), - dictionary_url)) - sdch_dictionary_url_ = dictionary_url; + // request_->url() won't be valid in the destructor, so we use an + // alternate copy. + DCHECK(request_->url() == request_info_.url); + // Resolve suggested URL relative to request url. + sdch_dictionary_url_ = request_info_.url.Resolve(url_text); } } -- cgit v1.1