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

3 files changed, 15 insertions, 8 deletions

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);
     }
   }