[NTP Snippets] Persist the set of snippets in a pref

components/ntp_snippets/ntp_snippets_service.h

 // Copyright 2015 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.
 
 #ifndef COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_
 #define COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_
 
 #include <stddef.h>
 
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/sequenced_task_runner.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/ntp_snippets/inner_iterator.h"
 #include "components/ntp_snippets/ntp_snippet.h"
 #include "components/ntp_snippets/ntp_snippets_fetcher.h"
 #include "components/ntp_snippets/ntp_snippets_scheduler.h"
 
+class PrefService;
+
+namespace base {
+class FilePath;
+class ListValue;
+}
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
 namespace ntp_snippets {
 
 class NTPSnippetsServiceObserver;
 
 // Stores and vend fresh content data for the NTP.
 class NTPSnippetsService : public KeyedService, NTPSnippetsFetcher::Observer {
  public:
   using NTPSnippetStorage = std::vector<scoped_ptr<NTPSnippet>>;
   using const_iterator =
       InnerIterator<NTPSnippetStorage::const_iterator, const NTPSnippet>;
 
   // |application_language_code| should be a ISO 639-1 compliant string. Aka
   // 'en' or 'en-US'. Note that this code should only specify the language, not
   // the locale, so 'en_US' (english language with US locale) and 'en-GB_US'
   // (British english person in the US) are not language code.
-  NTPSnippetsService(scoped_refptr<base::SequencedTaskRunner> file_task_runner,
+  NTPSnippetsService(PrefService* pref_service,

[Bernhard Bauer, 2016/02/25 17:16:33]

Can you please add a TODO to move this to a more suited storage eventually?
Otherwise this seems like the kind of thing that would be in danger of staying
here for far too long...

[Marc Treib, 2016/02/26 17:06:12]

What would be a "more suited storage"? IMO prefs isn't actually a bad place for
this.

[Bernhard Bauer, 2016/02/26 17:30:48]

Prefs are loaded synchronously from disk at startup and kept in memory, so an
increase in size means longer startup and more memory usage (we do have crash
reports of Chrome running out of memory while serializing prefs). I think
something like LevelDB or SQLite might be better here in the long term.

(SQLite has the interesting additional property that Android has Java APIs for
accessing databases, so that might be helpful if we want to manipulate them
while the native browser process isn't initialized?)

I mean, I don't want to block rapid iteration on this, which is why I'd be happy
with a TODO for now, but I would like not to accumulate technical debt here for
too long.

[Marc Treib, 2016/02/29 10:04:42]

OTOH, storing this in prefs mean adding a few kB to an existing file read, which
will be much cheaper than even a single seek. Memory use is definitely a valid
concern though.
This is indeed interesting!
Done, added a TODO below.

+                     scoped_refptr<base::SequencedTaskRunner> file_task_runner,
                      const std::string& application_language_code,
                      NTPSnippetsScheduler* scheduler,
                      scoped_ptr<NTPSnippetsFetcher> snippets_fetcher);
   ~NTPSnippetsService() override;
 
+  static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
   void Init(bool enabled);
 
   // Fetches snippets from the server. |overwrite| is true if existing snippets
   // should be overwritten.
   void FetchSnippets(bool overwrite);
 
   // Inherited from KeyedService.
   void Shutdown() override;
 
   // True once the data is loaded in memory and available to loop over.
@@ skipping 19 matching lines @@
   const_iterator begin() {
     DCHECK(loaded_);
     return const_iterator(snippets_.begin());
   }
   const_iterator end() {
     DCHECK(loaded_);
     return const_iterator(snippets_.end());
   }
 
  private:
+  void OnSnippetsDownloaded(const base::FilePath& download_path);
   void OnFileReadDone(const std::string* json, bool success);
-  void OnSnippetsDownloaded(const base::FilePath& download_path);
 
-  // Expects the JSON to be a list of dictionaries with keys matching the
-  // properties of a snippet (url, title, site_title, etc...). The url is the
-  // only mandatory value.
+  // Expects a top-level dictionary containing a "recos" list, which will be
+  // passed to |LoadFromJSONList|.
   bool LoadFromJSONString(const std::string& str);
 
+  // Expects a list of dictionaries each containing a "contentInfo" dictionary
+  // with keys matching the properties of a  snippet (url, title, site_title,
+  // etc...). The url is the only mandatory value.
+  bool LoadFromJSONList(const base::ListValue& list);
+
+  bool LoadFromPrefs();
+  void StoreToPrefs();
+
+  PrefService* pref_service_;
+
   // True if the suggestions are loaded.
   bool loaded_;
 
   // The SequencedTaskRunner on which file system operations will be run.
   scoped_refptr<base::SequencedTaskRunner> file_task_runner_;
 
   // All the suggestions.
   NTPSnippetStorage snippets_;
 
   // The ISO 639-1 code of the language used by the application.
@@ skipping 30 matching lines @@
   // Send when the service is shutting down.
   virtual void NTPSnippetsServiceShutdown(NTPSnippetsService* service) = 0;
 
  protected:
   virtual ~NTPSnippetsServiceObserver() {}
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_