Restructure SDCH layering to allow more separation (observer/1->[0,n]

net/base/sdch_manager.h

 // Copyright (c) 2011 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.
 
-// Provides global database of differential decompression dictionaries for the
-// SDCH filter (processes sdch enconded content).
-
-// Exactly one instance of SdchManager is built, and all references are made
-// into that collection.
-//
-// The SdchManager maintains a collection of memory resident dictionaries. It
-// can find a dictionary (based on a server specification of a hash), store a
-// dictionary, and make judgements about what URLs can use, set, etc. a
-// dictionary.
-
-// These dictionaries are acquired over the net, and include a header
-// (containing metadata) as well as a VCDIFF dictionary (for use by a VCDIFF
-// module) to decompress data.
 

[mmenke, 2014/11/06 15:30:12]

nit:  Remove extra blank line.

[Randy Smith (Not in Mondays), 2014/11/06 16:39:04]

Done.

 #ifndef NET_BASE_SDCH_MANAGER_H_
 #define NET_BASE_SDCH_MANAGER_H_
 
 #include <map>
 #include <set>
 #include <string>
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/threading/non_thread_safe.h"
+#include "base/observer_list.h"
+#include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "net/base/net_export.h"
 #include "url/gurl.h"
 
 namespace net {
 
-//------------------------------------------------------------------------------
-// Create a public interface to help us load SDCH dictionaries.
-// The SdchManager class allows registration to support this interface.
-// A browser may register a fetcher that is used by the dictionary managers to
-// get data from a specified URL. This allows us to use very high level browser
-// functionality in this base (when the functionality can be provided).
-class NET_EXPORT SdchFetcher {
- public:
-  class NET_EXPORT Delegate {
-   public:
-    virtual ~Delegate() {}
+class SdchObserver;
 
-    // Called whenever the SdchFetcher has successfully retrieved a
-    // dictionary.  |dictionary_text| contains the body of the dictionary
-    // retrieved from |dictionary_url|.
-    virtual void AddSdchDictionary(const std::string& dictionary_text,
-                                   const GURL& dictionary_url) = 0;
-  };
+// Provides global database of differential decompression dictionaries for the
+// SDCH filter (processes sdch enconded content).
+//
+// The SdchManager maintains a collection of memory resident dictionaries. It
+// can find a dictionary (based on a server specification of a hash), store a
+// dictionary, and make judgements about what URLs can use, set, etc. a
+// dictionary.
 
-  SdchFetcher() {}
-  virtual ~SdchFetcher() {}
-
-  // The Schedule() method is called when there is a need to get a dictionary
-  // from a server. The callee is responsible for getting that dictionary_text,
-  // and then calling back to AddSdchDictionary() in the Delegate instance.
-  virtual void Schedule(const GURL& dictionary_url) = 0;
-
-  // The Cancel() method is called to cancel all pending dictionary fetches.
-  // This is used for implementation of ClearData() below.
-  virtual void Cancel() = 0;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(SdchFetcher);
-};
-
-//------------------------------------------------------------------------------
-
-class NET_EXPORT SdchManager
-    : public SdchFetcher::Delegate,
-      public NON_EXPORTED_BASE(base::NonThreadSafe) {
+// These dictionaries are acquired over the net, and include a header
+// (containing metadata) as well as a VCDIFF dictionary (for use by a VCDIFF
+// module) to decompress data.
+class NET_EXPORT SdchManager {
  public:
   // A list of errors that appeared and were either resolved, or used to turn
   // off sdch encoding.
   enum ProblemCodes {
     MIN_PROBLEM_CODE,
 
     // Content-encoding correction problems.
     ADDED_CONTENT_ENCODING = 1,
     FIXED_CONTENT_ENCODING = 2,
     FIXED_CONTENT_ENCODINGS = 3,
@@ skipping 158 matching lines @@
     // of the dictionary. The following are the known headers.
     const std::string domain_;
     const std::string path_;
     const base::Time expiration_;  // Implied by max-age.
     const std::set<int> ports_;
 
     DISALLOW_COPY_AND_ASSIGN(Dictionary);
   };
 
   SdchManager();
-  ~SdchManager() override;
+  virtual ~SdchManager();

[mmenke, 2014/11/06 15:30:12]

There aren't any subclasses of this, are there?

[Randy Smith (Not in Mondays), 2014/11/06 16:39:04]

Not any more.  Done.

 
   // Clear data (for browser data removal).
   void ClearData();
 
   // Record stats on various errors.
   static void SdchErrorRecovery(ProblemCodes problem);
 
-  // Register a fetcher that this class can use to obtain dictionaries.
-  void set_sdch_fetcher(scoped_ptr<SdchFetcher> fetcher);
-
   // Enables or disables SDCH compression.
   static void EnableSdchSupport(bool enabled);
 
   static bool sdch_enabled() { return g_sdch_enabled_; }
 
   // Enables or disables SDCH compression over secure connection.
   static void EnableSecureSchemeSupport(bool enabled);
 
   static bool secure_scheme_supported() { return g_secure_scheme_supported_; }
 
@@ skipping 22 matching lines @@
 
   // Unit test only: Indicate what current blacklist increment is for a domain.
   int BlacklistDomainExponential(const std::string& domain);
 
   // Check to see if SDCH is enabled (globally), and the given URL is in a
   // supported domain (i.e., not blacklisted, and either the specific supported
   // domain, or all domains were assumed supported). If it is blacklist, reduce
   // by 1 the number of times it will be reported as blacklisted.
   bool IsInSupportedDomain(const GURL& url);
 
-  // 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& request_url, const GURL& dictionary_url);
-
-  // 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;
+  // Send out appropriate events notifying observers that a Get-Dictionary
+  // header has been seen.
+  void OnGetDictionary(const GURL& request_url, const GURL& dictionary_url);

[mmenke, 2014/11/06 15:30:12]

The reason for an observer-type rather than using an explicit delegate/fetcher
class is primarily for shutdown?  Or are their other reasons, too?

Not objecting, just trying to dig up context.

[Randy Smith (Not in Mondays), 2014/11/06 16:39:04]

It's breaking dependencies.  That's primarily for shutdown, but helps a bit on
initialization too.  But it's mostly to make reasoning about dependencies
simpler. 

We had a conversation about this in an email thread entitled "SDCH dictionary
fetching, expiration, and layering", if you *really* want context :-}.  I think
the delegate/interface issue was discussed in the first 2-3 messages on that
thread.

 
   // Find the vcdiff dictionary (the body of the sdch dictionary that appears
   // after the meta-data headers like Domain:...) with the given |server_hash|
   // to use to decompreses data that arrived as SDCH encoded content. Check to
   // be sure the returned |dictionary| can be used for decoding content supplied
   // in response to a request for |referring_url|.
   // Return null in |dictionary| if there is no matching legal dictionary.
   void GetVcdiffDictionary(const std::string& server_hash,
                            const GURL& referring_url,
                            scoped_refptr<Dictionary>* dictionary);
@@ skipping 10 matching lines @@
                            std::string* client_hash, std::string* server_hash);
 
   // For Latency testing only, we need to know if we've succeeded in doing a
   // round trip before starting our comparative tests. If ever we encounter
   // problems with SDCH, we opt-out of the test unless/until we perform a
   // complete SDCH decoding.
   bool AllowLatencyExperiment(const GURL& url) const;
 
   void SetAllowLatencyExperiment(const GURL& url, bool enable);
 
-  int GetFetchesCountForTesting() const {
-    return fetches_count_for_testing_;
-  }
-
-  // Implementation of SdchFetcher::Delegate.
-
   // Add an SDCH dictionary to our list of availible
   // dictionaries. This addition will fail if addition is illegal
   // (data in the dictionary is not acceptable from the
   // dictionary_url; dictionary already added, etc.).
   void AddSdchDictionary(const std::string& dictionary_text,
-                         const GURL& dictionary_url) override;
+                         const GURL& dictionary_url);
+
+  // Registration for events generated by the SDCH subsystem.
+  virtual void AddObserver(SdchObserver* observer);
+  virtual void RemoveObserver(SdchObserver* observer);
 
  private:
   struct BlacklistInfo {
     BlacklistInfo()
         : count(0),
           exponential_count(0),
           reason(MIN_PROBLEM_CODE) {}
 
     int count;                   // # of times to refuse SDCH advertisement.
     int exponential_count;       // Current exponential backoff ratchet.
     ProblemCodes reason;         // Why domain was blacklisted.
 
   };
   typedef std::map<std::string, BlacklistInfo> DomainBlacklistInfo;
   typedef std::set<std::string> ExperimentSet;
 
+  // Determines whether a "Get-Dictionary" header is legal (dictionary
+  // url has appropriate relationship to referrer url) in the SDCH
+  // protocol.  Return true if fetch is legal.
+  bool CanFetchDictionary(const GURL& referring_url,
+                          const GURL& dictionary_url) const;
+
   // A map of dictionaries info indexed by the hash that the server provides.
   typedef std::map<std::string, scoped_refptr<Dictionary> > DictionaryMap;
 
   // Support SDCH compression, by advertising in headers.
   static bool g_sdch_enabled_;
 
   // Support SDCH compression for HTTPS requests and responses. When supported,
   // HTTPS applicable dictionaries MUST have been acquired securely via HTTPS.
   static bool g_secure_scheme_supported_;
 
   // A simple implementation of a RFC 3548 "URL safe" base64 encoder.
   static void UrlSafeBase64Encode(const std::string& input,
                                   std::string* output);
   DictionaryMap dictionaries_;
 
-  // An instance that can fetch a dictionary given a URL.
-  scoped_ptr<SdchFetcher> fetcher_;
-
   // List domains where decode failures have required disabling sdch.
   DomainBlacklistInfo blacklisted_domains_;
 
   // List of hostnames for which a latency experiment is allowed (because a
   // round trip test has recently passed).
   ExperimentSet allow_latency_experiment_;
 
-  int fetches_count_for_testing_;
+  // Observers that want to be notified of SDCH events.
+  // Assert list is empty on destruction since if there is an observer
+  // that hasn't removed itself from the list, that observer probably
+  // has a reference to the SdchManager.
+  ObserverList<SdchObserver, true> observers_;
+
+  base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(SdchManager);
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_SDCH_MANAGER_H_