Make SDCH dictionaries persistent across browser restart.

net/url_request/sdch_dictionary_fetcher.h

Index: net/url_request/sdch_dictionary_fetcher.h
diff --git a/net/url_request/sdch_dictionary_fetcher.h b/net/url_request/sdch_dictionary_fetcher.h
index 4ca25034e8e5467a7082c9efd18b40e77017eb20..3c9f2cc50e19fa7ebef4287614034fabd435993f 100644
--- a/net/url_request/sdch_dictionary_fetcher.h
+++ b/net/url_request/sdch_dictionary_fetcher.h
@@ -31,6 +31,10 @@ class BoundNetLog;
 class URLRequest;
 class URLRequestThrottlerEntryInterface;
 
+namespace {
+class UniqueFetchQueue;

[mmenke, 2015/02/26 17:27:21]

Shouldn't put anonymous namespaces in header files.  Instead, should make this a
private class of the fetcher.  Could either define the class in the header, per
Randy's suggestion, or just forward declare it.  I'm fine with either.

[Elly Fong-Jones, 2015/03/03 21:37:48]

Done.

+}
+
 // This class is used by embedder SDCH policy object to fetch
 // dictionaries. It queues requests for dictionaries and dispatches
 // them serially, implementing the URLRequest::Delegate interface to
@@ -45,18 +49,23 @@ class NET_EXPORT SdchDictionaryFetcher : public URLRequest::Delegate,
       OnDictionaryFetchedCallback;
 
   // The consumer must guarantee that |*context| outlives this object.
-  // |callback| will be called on successful dictionary fetch
-  // requested through Schedule().  |callback| will not be called
-  // after object destruction.
-  SdchDictionaryFetcher(URLRequestContext* context,
-                        const OnDictionaryFetchedCallback& callback);
+  explicit SdchDictionaryFetcher(URLRequestContext* context);
   ~SdchDictionaryFetcher() override;
 
-  // Request a new dictionary fetch.
-  void Schedule(const GURL& dictionary_url);
+  // Request a new dictionary fetch.  The callback will be called
+  // only if the dictionary is successfully fetched. Returns true if a
+  // request for |dictionary_url| has been scheduled, and false otherwise.
+  virtual bool Schedule(const GURL& dictionary_url,
+                        const OnDictionaryFetchedCallback& callback);
+
+  // Request a dictionary fetch from cache only.  The callback will be called
+  // only if the dictionary is successfully fetched. Returns true if a request
+  // for |dictionary_url| has been scheduled, and false otherwise.
+  virtual bool ScheduleReload(const GURL& dictionary_url,
+                              const OnDictionaryFetchedCallback& callback);
 
   // Cancel any in-progress requests.
-  void Cancel();
+  virtual void Cancel();
 
   // Implementation of URLRequest::Delegate methods.
   void OnResponseStarted(URLRequest* request) override;
@@ -72,6 +81,16 @@ class NET_EXPORT SdchDictionaryFetcher : public URLRequest::Delegate,
     STATE_REQUEST_COMPLETE,
   };
 
+  // Schedule implementation. Returns true if a request for |dictionary_url| has
+  // been added to the queue, and false otherwise.
+  bool ScheduleInternal(const GURL& dictionary_url,
+                        bool reload,
+                        const OnDictionaryFetchedCallback& callback);
+
+  // Null out the current request and push the state machine to the
+  // next request, if any.
+  void ResetRequest();
+
   // State machine implementation.
   int DoLoop(int rv);
   int DoSendRequest(int rv);
@@ -84,37 +103,21 @@ class NET_EXPORT SdchDictionaryFetcher : public URLRequest::Delegate,
   bool in_loop_;
 
   // A queue of URLs that are being used to download dictionaries.
-  std::queue<GURL> fetch_queue_;
+  scoped_ptr<UniqueFetchQueue> fetch_queue_;

[Randy Smith (Not in Mondays), 2015/02/23 21:23:49]

nit, suggestion: If you declare UniqueFetchQueue inline as a nested class, you
don't need to make this a scoped_ptr<> but can instead just have a
UniqueFetchQueue as a member variable.

[Elly Fong-Jones, 2015/03/03 21:37:48]

I used the scoped_ptr to avoid inlining it. I've moved the forward declaration
of UniqueFetchQueue into this class, though.

 
-  // The request and buffer used for getting the current dictionary
-  // Both are null when a fetch is not in progress.
+  // The request, buffer, and consumer supplied data used for getting
+  // the current dictionary.  All are null when a fetch is not in progress.
   scoped_ptr<URLRequest> current_request_;
   scoped_refptr<IOBuffer> buffer_;
+  OnDictionaryFetchedCallback current_callback_;
 
   // 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
-  // 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 URLRequestContext associated with the owning SdchManager for
   // use while fetching.
   URLRequestContext* const context_;
 
-  const OnDictionaryFetchedCallback dictionary_fetched_callback_;
-
   base::WeakPtrFactory<SdchDictionaryFetcher> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SdchDictionaryFetcher);