Make SDCH dictionaries persistent across browser restart.

net/sdch/sdch_owner.h

Index: net/sdch/sdch_owner.h
diff --git a/net/sdch/sdch_owner.h b/net/sdch/sdch_owner.h
index 9620326118c661bedc13eb6785980520aa5171de..773ae86bf172043805c5503ce4e81ecd4e9d393b 100644
--- a/net/sdch/sdch_owner.h
+++ b/net/sdch/sdch_owner.h
@@ -8,10 +8,14 @@
 #include <string>
 
 #include "base/memory/memory_pressure_listener.h"
+#include "base/prefs/pref_store.h"
 #include "net/base/sdch_observer.h"
 #include "net/url_request/sdch_dictionary_fetcher.h"
 
 class GURL;
+class PersistentPrefStore;
+class ValueMapPrefStore;
+class WriteablePrefStore;
 
 namespace base {
 class Clock;
@@ -25,7 +29,8 @@ class URLRequestContext;
 // exposes interface for setting SDCH policy.  It should be instantiated by
 // the net/ embedder.
 // TODO(rdsmith): Implement dictionary prioritization.
-class NET_EXPORT SdchOwner : public net::SdchObserver {
+class NET_EXPORT SdchOwner : public net::SdchObserver,
+                             public PrefStore::Observer {
  public:
   static const size_t kMaxTotalDictionarySize;
   static const size_t kMinSpaceForDictionaryFetch;
@@ -35,6 +40,13 @@ class NET_EXPORT SdchOwner : public net::SdchObserver {
   SdchOwner(net::SdchManager* sdch_manager, net::URLRequestContext* context);
   ~SdchOwner() override;
 
+  // Enables use of pref persistence.  Dictionaries added to the SdchManager
+  // before this call is made will not be added to the pref store.
+  // Note that |pref_store| is owned by the caller, but must be guaranteed
+  // to outlive SdchOwner.
+  // This routine may only be called once per SdchOwner instance.
+  void EnablePersistentStorage(PersistentPrefStore* pref_store);
+
   // Defaults to kMaxTotalDictionarySize.
   void SetMaxTotalDictionarySize(size_t max_total_dictionary_size);
 
@@ -49,11 +61,16 @@ class NET_EXPORT SdchOwner : public net::SdchObserver {
                        const GURL& dictionary_url) override;
   void OnClearDictionaries(net::SdchManager* manager) override;
 
+  // PrefStore::Observer implementation.
+  void OnPrefValueChanged(const std::string& key) override;
+  void OnInitializationCompleted(bool succeeded) override;
+
   // Implementation detail--this is the pathway through which the
   // fetcher informs the SdchOwner that it's gotten the dictionary.
   // Public for testing.
   void OnDictionaryFetched(const std::string& dictionary_text,
                            const GURL& dictionary_url,
+                           scoped_ptr<SdchDictionaryFetcher::Data> extra_data,
                            const net::BoundNetLog& net_log);
 
   void SetClockForTesting(scoped_ptr<base::Clock> clock);
@@ -76,6 +93,9 @@ class NET_EXPORT SdchOwner : public net::SdchObserver {
   void OnMemoryPressure(
       base::MemoryPressureListener::MemoryPressureLevel level);
 
+  // Returns false if the load fails for some reason.
+  bool LoadPersistedDictionaries(const base::DictionaryValue& persisted_info);
+
   net::SdchManager* manager_;
   net::SdchDictionaryFetcher fetcher_;
 
@@ -87,6 +107,17 @@ class NET_EXPORT SdchOwner : public net::SdchObserver {
   size_t max_total_dictionary_size_;
   size_t min_space_for_dictionary_fetch_;
 
+  // Dictionary persistence machinery.  |temporary_pref_store_|
+  // is created on construction and used in the absence of any call
+  // to EnablePersistentStorage().  |persistent_pref_store_| holds the
+  // preference store specified by EnablePersistentStorage() (if any).
+  // |pref_store_| is shifted from the first to the second after the
+  // persistent pref store information is fully read in.
+  scoped_refptr<ValueMapPrefStore> temporary_pref_store_;

[mmenke, 2015/01/30 20:18:41]

I think "in_memory_pref_sort_" is more accurate - temporary could be interpreted
to mean it's always just until persistent_pref_store_ is ready, but that's not
always the case.

[Randy Smith (Not in Mondays), 2015/01/31 02:53:29]

Done.

+  PersistentPrefStore* persistent_pref_store_;

[Bernhard Bauer, 2015/01/29 17:42:23]

Can't this be a scoped_refptr?

[Randy Smith (Not in Mondays), 2015/01/30 20:11:10]

Hmmm.  My orientation (as backed up by bludgeoning by senior members of the
network stack, e.g. willchan@) is to avoid relying on ref-counting as much as
possible, and instead have clear ownership semantics.  If I was building the
infrastructure I need from the ground up I wouldn't use refcounting.  given that
PrefStore is RC, I'm using "scoped_refptr<>" to indicate "owned by" and a pure
pointer (and comments in the constructor) to indicate "not owned by; caller has
responsibility for maintaining ownership".  If you feel strongly, I can convert
to scoped_refptr<>, but I do have a rationale for not using it.

[mmenke, 2015/01/30 20:18:41]

The general bias of the network stack team is to avoid scoped_refptrs unless
necessary, since they make shutdown and dependencies pretty much impossible to
reason about.

[Randy Smith (Not in Mondays), 2015/01/31 02:53:29]

Acknowledged.

+
+  WriteablePrefStore* pref_store_;
+
   base::MemoryPressureListener memory_pressure_listener_;
 
   DISALLOW_COPY_AND_ASSIGN(SdchOwner);