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

BUG=587819

Review URL: https://codereview.chromium.org/1737563003

Cr-Commit-Position: refs/heads/master@{#378234}

chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc

@@ -68,7 +68,8 @@ KeyedService* NTPSnippetsServiceFactory::BuildServiceInstanceFor(
               base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
 
   return new ntp_snippets::NTPSnippetsService(
-      task_runner, g_browser_process->GetApplicationLocale(), scheduler,
+      profile->GetPrefs(), task_runner,
+      g_browser_process->GetApplicationLocale(), scheduler,
       make_scoped_ptr(new ntp_snippets::NTPSnippetsFetcher(
           task_runner, signin_manager, token_service, request_context,
           profile->GetPath())));

chrome/browser/prefs/browser_prefs.cc

@@ -73,6 +73,7 @@
 #include "components/flags_ui/pref_service_flags_storage.h"
 #include "components/gcm_driver/gcm_channel_status_syncer.h"
 #include "components/network_time/network_time_tracker.h"
+#include "components/ntp_snippets/ntp_snippets_service.h"
 #include "components/omnibox/browser/zero_suggest_provider.h"
 #include "components/password_manager/core/browser/password_bubble_experiment.h"
 #include "components/password_manager/core/browser/password_manager.h"
@@ -424,6 +425,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
   MediaDeviceIDSalt::RegisterProfilePrefs(registry);
   MediaStreamDevicesController::RegisterProfilePrefs(registry);
   NetPrefObserver::RegisterProfilePrefs(registry);
+  ntp_snippets::NTPSnippetsService::RegisterProfilePrefs(registry);
   password_bubble_experiment::RegisterPrefs(registry);
   password_manager::PasswordManager::RegisterProfilePrefs(registry);
   PrefProxyConfigTrackerImpl::RegisterProfilePrefs(registry);

components/ntp_snippets.gypi

@@ -25,6 +25,8 @@
         'ntp_snippets/ntp_snippets_scheduler.h',
         'ntp_snippets/ntp_snippets_service.cc',
         'ntp_snippets/ntp_snippets_service.h',
+        'ntp_snippets/pref_names.cc',
+        'ntp_snippets/pref_names.h',
       ],
     },
   ],

components/ntp_snippets/BUILD.gn

@@ -10,13 +10,18 @@ source_set("ntp_snippets") {
     "ntp_snippet.h",
     "ntp_snippets_fetcher.cc",
     "ntp_snippets_fetcher.h",
+    "ntp_snippets_scheduler.h",
     "ntp_snippets_service.cc",
     "ntp_snippets_service.h",
+    "pref_names.cc",
+    "pref_names.h",
   ]
 
   public_deps = [
     "//base",
     "//components/keyed_service/core",
+    "//components/pref_registry",
+    "//components/prefs",
     "//components/signin/core/browser",
     "//google_apis",
     "//net",

components/ntp_snippets/DEPS

@@ -1,5 +1,7 @@
 include_rules = [
   "+components/keyed_service/core",
+  "+components/pref_registry",
+  "+components/prefs",
   "+components/signin",
   "+net/base",
   "+net/http",

components/ntp_snippets/ntp_snippet.cc

@@ -3,43 +3,59 @@
 // found in the LICENSE file.
 
 #include "components/ntp_snippets/ntp_snippet.h"
+
 #include "base/values.h"
 
+namespace {
+
+const char kUrl[] = "url";
+const char kSiteTitle[] = "site_title";
+const char kTitle[] = "title";
+const char kFaviconUrl[] = "favicon_url";
+const char kSalientImageUrl[] = "thumbnailUrl";
+const char kSnippet[] = "snippet";
+const char kPublishDate[] = "creationTimestampSec";
+const char kExpiryDate[] = "expiryTimestampSec";
+
+}  // namespace
+
 namespace ntp_snippets {
 
 NTPSnippet::NTPSnippet(const GURL& url) : url_(url) {
-  DCHECK(url_.is_valid() && !url.is_empty());
+  DCHECK(url_.is_valid());
 }
 
 NTPSnippet::~NTPSnippet() {}
 
 // static
-scoped_ptr<NTPSnippet> NTPSnippet::NTPSnippetFromDictionary(
+scoped_ptr<NTPSnippet> NTPSnippet::CreateFromDictionary(
     const base::DictionaryValue& dict) {
   // Need at least the url.
   std::string url;
   if (!dict.GetString("url", &url))
     return nullptr;
 
+  // TODO(treib,noyau): Need to check that the URL is valid first, or remove
+  // the DCHECK in the constructor.
   scoped_ptr<NTPSnippet> snippet(new NTPSnippet(GURL(url)));
 
   std::string site_title;
-  if (dict.GetString("site_title", &site_title))
+  if (dict.GetString(kSiteTitle, &site_title))
     snippet->set_site_title(site_title);
-  std::string favicon_url;
-  if (dict.GetString("favicon_url", &favicon_url))
-    snippet->set_favicon_url(GURL(favicon_url));
   std::string title;
-  if (dict.GetString("title", &title))
+  if (dict.GetString(kTitle, &title))
     snippet->set_title(title);
-  std::string snippet_str;
-  if (dict.GetString("snippet", &snippet_str))
-    snippet->set_snippet(snippet_str);
+  std::string favicon_url;
+  if (dict.GetString(kFaviconUrl, &favicon_url))
+    snippet->set_favicon_url(GURL(favicon_url));
   std::string salient_image_url;
-  if (dict.GetString("thumbnailUrl", &salient_image_url))
+  if (dict.GetString(kSalientImageUrl, &salient_image_url))
     snippet->set_salient_image_url(GURL(salient_image_url));
+  std::string snippet_str;
+  if (dict.GetString(kSnippet, &snippet_str))
+    snippet->set_snippet(snippet_str);
   int creation_timestamp;
-  if (dict.GetInteger("creationTimestampSec", &creation_timestamp)) {
+  if (dict.GetInteger(kPublishDate, &creation_timestamp)) {
     snippet->set_publish_date(base::Time::UnixEpoch() +
                               base::TimeDelta::FromSeconds(creation_timestamp));
   }
@@ -48,4 +64,30 @@ scoped_ptr<NTPSnippet> NTPSnippet::NTPSnippetFromDictionary(
   return snippet;
 }
 
+scoped_ptr<base::DictionaryValue> NTPSnippet::ToDictionary() const {
+  scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
+
+  dict->SetString(kUrl, url_.spec());
+  if (!site_title_.empty())
+    dict->SetString(kSiteTitle, site_title_);
+  if (!title_.empty())
+    dict->SetString(kTitle, title_);
+  if (favicon_url_.is_valid())
+    dict->SetString(kFaviconUrl, favicon_url_.spec());
+  if (salient_image_url_.is_valid())
+    dict->SetString(kSalientImageUrl, salient_image_url_.spec());
+  if (!snippet_.empty())
+    dict->SetString(kSnippet, snippet_);
+  if (!publish_date_.is_null()) {
+    dict->SetInteger(kPublishDate,
+                     (publish_date_ - base::Time::UnixEpoch()).InSeconds());
+  }
+  if (!expiry_date_.is_null()) {
+    dict->SetInteger(kExpiryDate,
+                     (expiry_date_ - base::Time::UnixEpoch()).InSeconds());
+  }
+
+  return dict;
+}
+
 }  // namespace ntp_snippets

components/ntp_snippets/ntp_snippet.h

@@ -1,6 +1,7 @@
 // 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_SNIPPET_H_
 #define COMPONENTS_NTP_SNIPPETS_NTP_SNIPPET_H_
 
@@ -29,9 +30,11 @@ class NTPSnippet {
   // dictionary doesn't contain at least a url. The keys in the dictionary are
   // expected to be the same as the property name, with exceptions documented in
   // the property comment.
-  static scoped_ptr<NTPSnippet> NTPSnippetFromDictionary(
+  static scoped_ptr<NTPSnippet> CreateFromDictionary(
       const base::DictionaryValue& dict);
 
+  scoped_ptr<base::DictionaryValue> ToDictionary() const;
+
   // URL of the page described by this snippet.
   const GURL& url() const { return url_; }
 
@@ -54,15 +57,15 @@ class NTPSnippet {
   void set_snippet(const std::string& snippet) { snippet_ = snippet; }
 
   // Link to an image representative of the content. Do not fetch this image
-  // directly. If initialized by NTPSnippetFromDictionary() the relevant key is
-  // 'thumbnailURL'
+  // directly. If initialized by CreateFromDictionary() the relevant key is
+  // 'thumbnailUrl'
   const GURL& salient_image_url() const { return salient_image_url_; }
   void set_salient_image_url(const GURL& salient_image_url) {
     salient_image_url_ = salient_image_url;
   }
 
   // When the page pointed by this snippet was published.  If initialized by
-  // NTPSnippetFromDictionary() the relevant key is 'creationTimestampSec'
+  // CreateFromDictionary() the relevant key is 'creationTimestampSec'
   const base::Time& publish_date() const { return publish_date_; }
   void set_publish_date(const base::Time& publish_date) {
     publish_date_ = publish_date;

components/ntp_snippets/ntp_snippets_service.cc

@@ -11,6 +11,9 @@
 #include "base/path_service.h"
 #include "base/task_runner_util.h"
 #include "base/values.h"
+#include "components/ntp_snippets/pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/prefs/pref_service.h"
 
 namespace {
 
@@ -25,16 +28,20 @@ bool ReadFileToString(const base::FilePath& path, std::string* data) {
   return success;
 }
 
+const char kContentInfo[] = "contentInfo";
+
 }  // namespace
 
 namespace ntp_snippets {
 
 NTPSnippetsService::NTPSnippetsService(
+    PrefService* pref_service,
     scoped_refptr<base::SequencedTaskRunner> file_task_runner,
     const std::string& application_language_code,
     NTPSnippetsScheduler* scheduler,
     scoped_ptr<NTPSnippetsFetcher> snippets_fetcher)
-    : loaded_(false),
+    : pref_service_(pref_service),
+      loaded_(false),
       file_task_runner_(file_task_runner),
       application_language_code_(application_language_code),
       scheduler_(scheduler),
@@ -47,11 +54,21 @@ NTPSnippetsService::NTPSnippetsService(
 
 NTPSnippetsService::~NTPSnippetsService() {}
 
+// static
+void NTPSnippetsService::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterListPref(prefs::kSnippets);
+}
+
 void NTPSnippetsService::Init(bool enabled) {
-  // If enabled, get snippets immediately. If we've downloaded them before,
-  // this will just read from disk.
-  if (enabled)
-    FetchSnippets(false);
+  // If enabled, get any existing snippets immediately from prefs.
+  if (enabled) {
+    LoadFromPrefs();
+
+    // If we don't have any snippets yet, start a fetch.
+    if (snippets_.empty())
+      FetchSnippets(false);
+  }
 
   // The scheduler only exists on Android so far, it's null on other platforms.
   if (!scheduler_)
@@ -83,12 +100,23 @@ void NTPSnippetsService::RemoveObserver(NTPSnippetsServiceObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void NTPSnippetsService::OnSnippetsDownloaded(
+    const base::FilePath& download_path) {
+  std::string* downloaded_data = new std::string;
+  base::PostTaskAndReplyWithResult(
+      file_task_runner_.get(), FROM_HERE,
+      base::Bind(&ReadFileToString, download_path, downloaded_data),
+      base::Bind(&NTPSnippetsService::OnFileReadDone,
+                 weak_ptr_factory_.GetWeakPtr(), base::Owned(downloaded_data)));
+}
+
 void NTPSnippetsService::OnFileReadDone(const std::string* json, bool success) {
   if (!success)
     return;
 
   DCHECK(json);
   LoadFromJSONString(*json);
+  StoreToPrefs();
 }
 
 bool NTPSnippetsService::LoadFromJSONString(const std::string& str) {
@@ -104,16 +132,19 @@ bool NTPSnippetsService::LoadFromJSONString(const std::string& str) {
   if (!top_dict->GetList("recos", &list))
     return false;
 
-  for (const base::Value* const value : *list) {
+  return LoadFromJSONList(*list);
+}
+
+bool NTPSnippetsService::LoadFromJSONList(const base::ListValue& list) {
+  for (const base::Value* const value : list) {
     const base::DictionaryValue* dict = nullptr;
     if (!value->GetAsDictionary(&dict))
       return false;
 
     const base::DictionaryValue* content = nullptr;
-    if (!dict->GetDictionary("contentInfo", &content))
+    if (!dict->GetDictionary(kContentInfo, &content))
       return false;
-    scoped_ptr<NTPSnippet> snippet =
-        NTPSnippet::NTPSnippetFromDictionary(*content);
+    scoped_ptr<NTPSnippet> snippet = NTPSnippet::CreateFromDictionary(*content);
     if (!snippet)
       return false;
 
@@ -136,14 +167,25 @@ bool NTPSnippetsService::LoadFromJSONString(const std::string& str) {
   return true;
 }
 
-void NTPSnippetsService::OnSnippetsDownloaded(
-    const base::FilePath& download_path) {
-  std::string* downloaded_data = new std::string;
-  base::PostTaskAndReplyWithResult(
-      file_task_runner_.get(), FROM_HERE,
-      base::Bind(&ReadFileToString, download_path, downloaded_data),
-      base::Bind(&NTPSnippetsService::OnFileReadDone,
-                 weak_ptr_factory_.GetWeakPtr(), base::Owned(downloaded_data)));
+void NTPSnippetsService::LoadFromPrefs() {
+  // |pref_service_| can be null in tests.
+  if (!pref_service_)
+    return;
+  if (!LoadFromJSONList(*pref_service_->GetList(prefs::kSnippets)))
+    LOG(ERROR) << "Failed to parse snippets from prefs";
+}
+
+void NTPSnippetsService::StoreToPrefs() {
+  // |pref_service_| can be null in tests.
+  if (!pref_service_)
+    return;
+  base::ListValue list;
+  for (const auto& snippet : snippets_) {
+    scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
+    dict->Set(kContentInfo, snippet->ToDictionary());
+    list.Append(std::move(dict));
+  }
+  pref_service_->Set(prefs::kSnippets, list);
 }
 
 }  // namespace ntp_snippets