Change SDCHDictionaryFetcher to use URLRequest instead of URLFetcher.

net/base/sdch_dictionary_fetcher.h

Index: net/base/sdch_dictionary_fetcher.h
diff --git a/net/base/sdch_dictionary_fetcher.h b/net/base/sdch_dictionary_fetcher.h
index 82c0b10c6e48a19733021be751986ac5990a60b3..d1f0f11dc052e33c6b3eaeb2dbd70a6f0ff26dd0 100644
--- a/net/base/sdch_dictionary_fetcher.h
+++ b/net/base/sdch_dictionary_fetcher.h
@@ -2,9 +2,11 @@
 // 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).
+// TODO(rdsmith): This class needs to be moved out to the net/ embedder and
+// hooked into whatever mechanisms the embedder uses for authentication.
+// Specifically, this class needs methods overriding
+// URLRequest::Delegate::{OnAuthRequired,OnCertificateRequested} and can't
+// implement them at the net/ layer.
 
 #ifndef NET_BASE_SDCH_DICTIONARY_FETCHER_H_
 #define NET_BASE_SDCH_DICTIONARY_FETCHER_H_
@@ -18,22 +20,28 @@
 #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"
 
 namespace net {
 
-class URLFetcher;
-class URLRequestContextGetter;
+class URLRequest;
+class URLRequestThrottlerEntryInterface;
 
+// This class implements the SdchFetcher interface.  It queues requests
+// for dictionaries and dispatches them serially, implementing
+// the URLRequest::Delegate interface to handle callbacks (but see above
+// TODO).  It tracks all requests, only attempting to fetch each dictionary
+// once.
 class NET_EXPORT SdchDictionaryFetcher
-    : public URLFetcherDelegate,
-      public SdchFetcher,
+    : public SdchFetcher,
+      public URLRequest::Delegate,
       public base::NonThreadSafe {
  public:
-  // The consumer must guarantee that |*manager| outlives
+  // The consumer must guarantee that |*manager| and |*context| outlive
   // this object.  The current implementation guarantees this by
-  // the SdchManager owning this object.
-  SdchDictionaryFetcher(SdchManager* manager,
-                        scoped_refptr<URLRequestContextGetter> context);
+  // the SdchManager owning this object, and |*context| owning SdchManager,
+  // so this object will be destroyed before either |*manager| or |*context|.

[Ryan Sleevi, 2014/09/02 20:04:19]

"The current implementation guarantees this" is likely a comment representing a
layering violation (nobody is guaranteeing this comment is kept up to date, nor
that these invariants will always hold)

[Randy Smith (Not in Mondays), 2014/09/02 23:34:32]

Hmmm.  From my perspective, the normative part of the comment is the first
sentence, which I think is a fine specification of an interface contract and not
a layering violation.  I agree that the second sentence refers to things beyond
the ken of this class, and so I've removed it.  I don't see any layering
violations in the code.  Does removing that comment address your concern?

+  SdchDictionaryFetcher(SdchManager* manager, URLRequestContext* context);
   virtual ~SdchDictionaryFetcher();
 
   // Implementation of SdchFetcher class.
@@ -41,36 +49,38 @@ class NET_EXPORT SdchDictionaryFetcher
   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;
-
-  // Ensure the download after the above delay.
-  void ScheduleDelayedRun();
-
-  // 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;
+  enum State {
+    STATE_NONE,
+    STATE_IDLE,
+    STATE_REQUEST_READING,
+    STATE_REQUEST_COMPLETE,
+  };
+
+  // State machine implementation.
+  int DoLoop(int rv);
+  int DoDispatchRequest();
+  int DoRead();
+  int DoCompleteRequest();
+  void HandleResult(int rv);
+
+  // URLRequest::Delegate overrides
+  virtual void OnResponseStarted(URLRequest* request) OVERRIDE;
+  virtual void OnReadCompleted(URLRequest* request, int bytes_read) OVERRIDE;
+
+  State next_state_;
 
   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_;
 
   // 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
@@ -87,9 +97,11 @@ class NET_EXPORT SdchDictionaryFetcher
   // 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_;
+  // Store the URLRequestContext associated with the owning SdchManager for
+  // use while fetching.
+  URLRequestContext* context_;
+
+  base::WeakPtrFactory<SdchDictionaryFetcher> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SdchDictionaryFetcher);
 };