Change SDCHDictionaryFetcher to use URLRequest instead of URLFetcher.

net/base/sdch_dictionary_fetcher.h

 // Copyright 2014 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.
 
 // Support modularity by calling to load a new SDCH filter dictionary.
 // Note that this sort of calling can't be done in the /net directory, as it has
 // no concept of the HTTP cache (which is only visible at the browser level).
 
 #ifndef NET_BASE_SDCH_DICTIONARY_FETCHER_H_
 #define NET_BASE_SDCH_DICTIONARY_FETCHER_H_
 
 #include <queue>
 #include <set>
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "net/base/sdch_manager.h"
 #include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_context.h"

[Ryan Sleevi, 2014/08/20 18:27:55]

Totes a layering violation here, and pre-existing unfortunately.

net/base should, in an ideal world, only depend on //net/base

Will keeps talking about wanting/needing to tighten this up.

I don't know if //net/http is much better, because //net/url_request depends on
//net/http, and if you're going to make a URL request, you should sit at that
level.

//net/sdch seems wrong, because it conflates the low-level bits (needed by
either //net/http or //net/url_request) with the high-level bits (the fetching
code)

I'm open to suggestions, but that's the general idea (which is to reduce
circular dependencies, and have //net/ subdirs be hermetic and tightly layered,
loosely coupled)

[Randy Smith (Not in Mondays), 2014/08/20 19:21:50]

Makes sense.  I'm not clear why I can't leave sdch_manager.* in net/base and put
sdch_dictionary_fetcher.* in net/.  Would that be ok with you?

 
 namespace net {
 
-class URLFetcher;
+class URLRequest;
 class URLRequestContextGetter;
+class URLRequestThrottlerEntryInterface;
 
 class NET_EXPORT SdchDictionaryFetcher
-    : public URLFetcherDelegate,
-      public SdchFetcher,
+    : public SdchFetcher,
+      public URLRequest::Delegate,
       public base::NonThreadSafe {
  public:
   // The consumer must guarantee that |*manager| outlives
   // this object.  The current implementation guarantees this by
   // the SdchManager owning this object.
   SdchDictionaryFetcher(SdchManager* manager,
                         scoped_refptr<URLRequestContextGetter> context);
   virtual ~SdchDictionaryFetcher();
 
   // Implementation of SdchFetcher class.
   virtual void Schedule(const GURL& dictionary_url) OVERRIDE;
   virtual void Cancel() OVERRIDE;
 
  private:
-  // 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;
+  // URLRequest::Delegate overrides
+  virtual void OnResponseStarted(URLRequest* request) OVERRIDE;
+  virtual void OnReadCompleted(URLRequest* request, int bytes_read) OVERRIDE;
 
-  // Ensure the download after the above delay.
-  void ScheduleDelayedRun();
+  // Dispatch the next waiting request, if any.
+  void DispatchRun();
 
   // Make sure we're processing (or waiting for) the the arrival of the next URL
   // in the  |fetch_queue_|.
   void StartFetching();
 
-  // Implementation of URLFetcherDelegate. Called after transmission
-  // completes (either successfully or with failure).
-  virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
+  // Actually dispatch the request for the next dictionary.
+  void StartURLRequest();
+
+  // Used for interactions with URLRequestThrottlingManager.
+  base::TimeTicks GetBackoffReleaseTime();
 
   SdchManager* const manager_;
 
   // A queue of URLs that are being used to download dictionaries.
   std::queue<GURL> fetch_queue_;
-  // The currently outstanding URL fetch of a dicitonary.
-  // If this is null, then there is no outstanding request.
-  scoped_ptr<URLFetcher> current_fetch_;
 
-  // Always spread out the dictionary fetches, so that they don't steal
-  // bandwidth from the actual page load.  Create delayed tasks to spread out
-  // the download.
-  base::WeakPtrFactory<SdchDictionaryFetcher> weak_factory_;
-  bool task_is_pending_;
+  // The request and buffer used for getting the current dictionary
+  // Both are null when a fetch is not in progress.
+  scoped_ptr<URLRequest> current_request_;
+  scoped_refptr<IOBuffer> buffer_;
+
+  // The currently accumulating dictionary.
+  std::string dictionary_;
+
+  // Used to determine how long to wait before making a request or doing a
+  // retry.
+  //
+  // Both of them can only be accessed on the IO thread.
+  //
+  // We need not only the throttler entry for |original_URL|, but also
+  // the one for |url|. For example, consider the case that URL A
+  // redirects to URL B, for which the server returns a 500
+  // response. In this case, the exponential back-off release time of
+  // URL A won't increase. If we retry without considering the
+  // back-off constraint of URL B, we may send out too many requests
+  // for URL A in a short period of time.
+  //
+  // Both of these will be NULL if
+  // URLRequestContext::throttler_manager() is NULL.
+  scoped_refptr<URLRequestThrottlerEntryInterface>
+      original_url_throttler_entry_;
+  scoped_refptr<URLRequestThrottlerEntryInterface> url_throttler_entry_;
 
   // Althought the SDCH spec does not preclude a server from using a single URL
   // to load several distinct dictionaries (by telling a client to load a
   // dictionary from an URL several times), current implementations seem to have
   // that 1-1 relationship (i.e., each URL points at a single dictionary, and
   // the dictionary content does not change over time, and hence is not worth
   // trying to load more than once).  In addition, some dictionaries prove
   // unloadable only after downloading them (because they are too large?  ...or
   // malformed?). As a protective element, Chromium will *only* load a
   // dictionary at most once from a given URL (so that it doesn't waste
   // bandwidth trying repeatedly).
   // The following set lists all the dictionary URLs that we've tried to load,
   // so that we won't try to load from an URL more than once.
   // TODO(jar): Try to augment the SDCH proposal to include this restiction.
   std::set<GURL> attempted_load_;
 
   // Store the system_url_request_context_getter to use it when we start
   // fetching.
   scoped_refptr<URLRequestContextGetter> context_;
 
+  base::WeakPtrFactory<SdchDictionaryFetcher> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(SdchDictionaryFetcher);
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_SDCH_DICTIONARY_FETCHER_H_