Fetch SDCH dictionary as soon as current URL fetch completes

Rather than just waiting for 15 seconds, this patch fetches the
"suggested dicitionary" as soon as the current download completes.
This avoids stealing bandwidth from the original download, but 
works as quickly as possible to get the dictionary, so that the
user can receive compression results asap.

r=wtc
Review URL: http://codereview.chromium.org/20254

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@9597 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 27 insertions, 15 deletions

diff --git a/chrome/browser/net/sdch_dictionary_fetcher.cc b/chrome/browser/net/sdch_dictionary_fetcher.cc
index 5208f37..9bce5f3 100644
--- a/chrome/browser/net/sdch_dictionary_fetcher.cc
+++ b/chrome/browser/net/sdch_dictionary_fetcher.cc
@@ -18,8 +18,6 @@ void SdchDictionaryFetcher::Schedule(const GURL& dictionary_url) {
   ScheduleDelayedRun();
 }
 
-// TODO(jar): If QOS low priority is supported, switch to using that instead of
-// just waiting to do the fetch.
 void SdchDictionaryFetcher::ScheduleDelayedRun() {
   if (fetch_queue_.empty() || current_fetch_.get() || task_is_pending_)
     return;
diff --git a/chrome/browser/net/sdch_dictionary_fetcher.h b/chrome/browser/net/sdch_dictionary_fetcher.h
index b943a24..a0a9e2f 100644
--- a/chrome/browser/net/sdch_dictionary_fetcher.h
+++ b/chrome/browser/net/sdch_dictionary_fetcher.h
@@ -13,6 +13,7 @@
 #include <string>
 
 #include "base/compiler_specific.h"
+#include "base/scoped_ptr.h"
 #include "base/task.h"
 #include "chrome/browser/net/url_fetcher.h"
 #include "net/base/sdch_manager.h"
@@ -20,7 +21,7 @@
 class SdchDictionaryFetcher : public URLFetcher::Delegate,
                               public SdchFetcher {
  public:
-  SdchDictionaryFetcher() : 
+  SdchDictionaryFetcher() :
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       task_is_pending_(false) {}
   virtual ~SdchDictionaryFetcher() {}
@@ -31,8 +32,11 @@ class SdchDictionaryFetcher : public URLFetcher::Delegate,
   virtual void Schedule(const GURL& dictionary_url);
 
  private:
-  // Delay between Schedule and actual download.
-  static const int kMsDelayFromRequestTillDownload = 15000;
+  // Delay in ms between Schedule and actual download.
+  // This leaves the URL in a queue, which is de-duped, so that there is less
+  // chance we'll try to load the same URL multiple times when a pile of
+  // page subresources (or tabs opened in parallel) all suggest the dictionary.
+  static const int kMsDelayFromRequestTillDownload = 100;
 
   // Ensure the download after the above delay.
   void ScheduleDelayedRun();
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index be8ae13..9097f88 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -172,10 +172,7 @@ bool SdchManager::CanFetchDictionary(const GURL& referring_url,
   return true;
 }
 
-void SdchManager::FetchDictionary(const GURL& referring_url,
-                                  const GURL& dictionary_url) {
-  if (!CanFetchDictionary(referring_url, dictionary_url))
-    return;
+void SdchManager::FetchDictionary(const GURL& dictionary_url) {
   if (fetcher_.get())
     fetcher_->Schedule(dictionary_url);
 }
@@ -379,6 +376,11 @@ bool SdchManager::Dictionary::CanSet(const std::string& domain,
     5. If the dictionary has a Port attribute and the referer URL's port was not
       in the list.
   */
+
+  // TODO(jar): Redirects in dictionary fetches might plausibly be problematic,
+  // and hence the conservative approach is to not allow any redirects (if there
+  // were any... then don't allow the dictionary to be set).
+
   if (domain.empty()) {
     SdchErrorRecovery(DICTIONARY_MISSING_DOMAIN_SPECIFIER);
     return false;  // Domain is required.
diff --git a/net/base/sdch_manager.h b/net/base/sdch_manager.h
index 0f7cdeb..2a4e5b5 100644
--- a/net/base/sdch_manager.h
+++ b/net/base/sdch_manager.h
@@ -243,13 +243,13 @@ class SdchManager {
   // by 1 the number of times it will be reported as blacklisted.
   const bool IsInSupportedDomain(const GURL& url);
 
-  // Schedule the URL fetching to load a dictionary. This will generally return
-  // long before the dictionary is actually loaded and added.
+  // Schedule the URL fetching to load a dictionary. This will always return
+  // 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& referring_url, const GURL& dictionary_url);
+  void FetchDictionary(const GURL& dictionary_url);
 
-  // Security test function used before initiating a fetch.
+  // Security test function used before initiating a FetchDictionary.
   // Return true if fetch is legal.
   bool CanFetchDictionary(const GURL& referring_url,
                           const GURL& dictionary_url) const;
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index d29b31e..c7fc435 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -64,6 +64,9 @@ URLRequestHttpJob::URLRequestHttpJob(URLRequest* request)
 }
 
 URLRequestHttpJob::~URLRequestHttpJob() {
+  if (sdch_dictionary_url_.is_valid()) {
+    SdchManager::Global()->FetchDictionary(sdch_dictionary_url_);
+  }
 }
 
 void URLRequestHttpJob::SetUpload(net::UploadData* upload) {
@@ -445,7 +448,6 @@ void URLRequestHttpJob::NotifyHeadersComplete() {
     }
   }
 
-  // Get list of SDCH dictionary requests, and schedule them to be loaded.
   if (SdchManager::Global() &&
       SdchManager::Global()->IsInSupportedDomain(request_->url())) {
     static const std::string name = "Get-Dictionary";
@@ -459,7 +461,9 @@ void URLRequestHttpJob::NotifyHeadersComplete() {
     // 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);
-      SdchManager::Global()->FetchDictionary(request_->url(), dictionary_url);
+      if (SdchManager::Global()->CanFetchDictionary(request_->url(),
+                                                    dictionary_url))
+         sdch_dictionary_url_ = dictionary_url;
     }
   }
 
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index e53db48..a77d462 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/scoped_ptr.h"
 #include "net/base/completion_callback.h"
 #include "net/http/http_request_info.h"
 #include "net/url_request/url_request_job.h"
@@ -80,6 +81,9 @@ class URLRequestHttpJob : public URLRequestJob {
 
   bool read_in_progress_;
 
+  // An URL for an SDCH dictionary as suggested in a Get-Dictionary HTTP header.
+  GURL sdch_dictionary_url_;
+
   // Keep a reference to the url request context to be sure it's not deleted
   // before us.
   scoped_refptr<URLRequestContext> context_;