diff options
-rw-r--r-- | chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc | 3 | ||||
-rw-r--r-- | chrome/browser/prefs/browser_prefs.cc | 2 | ||||
-rw-r--r-- | components/ntp_snippets.gypi | 2 | ||||
-rw-r--r-- | components/ntp_snippets/BUILD.gn | 5 | ||||
-rw-r--r-- | components/ntp_snippets/DEPS | 2 | ||||
-rw-r--r-- | components/ntp_snippets/ntp_snippet.cc | 66 | ||||
-rw-r--r-- | components/ntp_snippets/ntp_snippet.h | 11 | ||||
-rw-r--r-- | components/ntp_snippets/ntp_snippets_service.cc | 76 | ||||
-rw-r--r-- | components/ntp_snippets/ntp_snippets_service.h | 34 | ||||
-rw-r--r-- | components/ntp_snippets/ntp_snippets_service_unittest.cc | 10 | ||||
-rw-r--r-- | components/ntp_snippets/pref_names.cc | 13 | ||||
-rw-r--r-- | components/ntp_snippets/pref_names.h | 16 | ||||
-rw-r--r-- | ios/chrome/browser/ntp_snippets/ios_chrome_ntp_snippets_service_factory.cc | 3 | ||||
-rw-r--r-- | ios/chrome/browser/prefs/browser_prefs.mm | 2 |
14 files changed, 200 insertions, 45 deletions
diff --git a/chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc b/chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc index 50a2294..d9c1439 100644 --- a/chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc +++ b/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()))); diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc index 1b37e18..74cc366 100644 --- a/chrome/browser/prefs/browser_prefs.cc +++ b/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); diff --git a/components/ntp_snippets.gypi b/components/ntp_snippets.gypi index d6a0d63..7bf22e8 100644 --- a/components/ntp_snippets.gypi +++ b/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', ], }, ], diff --git a/components/ntp_snippets/BUILD.gn b/components/ntp_snippets/BUILD.gn index 222ad18..614dc29 100644 --- a/components/ntp_snippets/BUILD.gn +++ b/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", diff --git a/components/ntp_snippets/DEPS b/components/ntp_snippets/DEPS index 830c20f..e520527 100644 --- a/components/ntp_snippets/DEPS +++ b/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", diff --git a/components/ntp_snippets/ntp_snippet.cc b/components/ntp_snippets/ntp_snippet.cc index 9ee2029..b120e7b 100644 --- a/components/ntp_snippets/ntp_snippet.cc +++ b/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 diff --git a/components/ntp_snippets/ntp_snippet.h b/components/ntp_snippets/ntp_snippet.h index 6dd2ad04..daa1d52 100644 --- a/components/ntp_snippets/ntp_snippet.h +++ b/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; diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc index e540f73..50d8883 100644 --- a/components/ntp_snippets/ntp_snippets_service.cc +++ b/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 diff --git a/components/ntp_snippets/ntp_snippets_service.h b/components/ntp_snippets/ntp_snippets_service.h index 1b46bad..a37b08e 100644 --- a/components/ntp_snippets/ntp_snippets_service.h +++ b/components/ntp_snippets/ntp_snippets_service.h @@ -20,6 +20,17 @@ #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; @@ -35,12 +46,15 @@ class NTPSnippetsService : public KeyedService, NTPSnippetsFetcher::Observer { // '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, + 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 @@ -80,14 +94,24 @@ class NTPSnippetsService : public KeyedService, NTPSnippetsFetcher::Observer { } private: - void OnFileReadDone(const std::string* json, bool success); void OnSnippetsDownloaded(const base::FilePath& download_path); + void OnFileReadDone(const std::string* json, bool success); - // 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); + + // TODO(treib): Investigate a better storage, maybe LevelDB or SQLite? + void LoadFromPrefs(); + void StoreToPrefs(); + + PrefService* pref_service_; + // True if the suggestions are loaded. bool loaded_; diff --git a/components/ntp_snippets/ntp_snippets_service_unittest.cc b/components/ntp_snippets/ntp_snippets_service_unittest.cc index c845b1b..f667d01 100644 --- a/components/ntp_snippets/ntp_snippets_service_unittest.cc +++ b/components/ntp_snippets/ntp_snippets_service_unittest.cc @@ -60,11 +60,11 @@ class NTPSnippetsServiceTest : public testing::Test { FakeSigninManagerBase* signin_manager = new FakeSigninManagerBase( signin_client_.get(), account_tracker_.get()); - scoped_ptr<NTPSnippetsService> service( - new NTPSnippetsService(task_runner.get(), std::string("fr"), nullptr, - make_scoped_ptr(new NTPSnippetsFetcher(task_runner.get(), - signin_manager, token_service, request_context_getter, - base::FilePath())))); + scoped_ptr<NTPSnippetsService> service(new NTPSnippetsService( + nullptr, task_runner.get(), std::string("fr"), nullptr, + make_scoped_ptr(new NTPSnippetsFetcher( + task_runner.get(), signin_manager, token_service, + request_context_getter, base::FilePath())))); return service; } diff --git a/components/ntp_snippets/pref_names.cc b/components/ntp_snippets/pref_names.cc new file mode 100644 index 0000000..63085e2 --- /dev/null +++ b/components/ntp_snippets/pref_names.cc @@ -0,0 +1,13 @@ +// Copyright 2016 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. + +#include "components/ntp_snippets/pref_names.h" + +namespace ntp_snippets { +namespace prefs { + +const char kSnippets[] = "ntp_snippets.snippets"; + +} // namespace prefs +} // namespace ntp_snippets diff --git a/components/ntp_snippets/pref_names.h b/components/ntp_snippets/pref_names.h new file mode 100644 index 0000000..d988d48 --- /dev/null +++ b/components/ntp_snippets/pref_names.h @@ -0,0 +1,16 @@ +// Copyright 2016 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_PREF_NAMES_H_ +#define COMPONENTS_NTP_SNIPPETS_PREF_NAMES_H_ + +namespace ntp_snippets { +namespace prefs { + +extern const char kSnippets[]; + +} // namespace prefs +} // namespace ntp_snippets + +#endif diff --git a/ios/chrome/browser/ntp_snippets/ios_chrome_ntp_snippets_service_factory.cc b/ios/chrome/browser/ntp_snippets/ios_chrome_ntp_snippets_service_factory.cc index 319d1f8..954a3b3 100644 --- a/ios/chrome/browser/ntp_snippets/ios_chrome_ntp_snippets_service_factory.cc +++ b/ios/chrome/browser/ntp_snippets/ios_chrome_ntp_snippets_service_factory.cc @@ -62,7 +62,8 @@ IOSChromeNTPSnippetsServiceFactory::BuildServiceInstanceFor( base::SequencedWorkerPool::GetSequenceToken(), base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN); return make_scoped_ptr(new ntp_snippets::NTPSnippetsService( - task_runner, GetApplicationContext()->GetApplicationLocale(), scheduler, + chrome_browser_state->GetPrefs(), task_runner, + GetApplicationContext()->GetApplicationLocale(), scheduler, make_scoped_ptr(new ntp_snippets::NTPSnippetsFetcher( task_runner, (SigninManagerBase*)signin_manager, token_service, request_context, browser_state->GetStatePath())))); diff --git a/ios/chrome/browser/prefs/browser_prefs.mm b/ios/chrome/browser/prefs/browser_prefs.mm index aafd2e1..3bcb2bb 100644 --- a/ios/chrome/browser/prefs/browser_prefs.mm +++ b/ios/chrome/browser/prefs/browser_prefs.mm @@ -11,6 +11,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_manager.h" #include "components/pref_registry/pref_registry_syncable.h" @@ -85,6 +86,7 @@ void RegisterBrowserStatePrefs(user_prefs::PrefRegistrySyncable* registry) { gcm::GCMChannelStatusSyncer::RegisterProfilePrefs(registry); HostContentSettingsMap::RegisterProfilePrefs(registry); HttpServerPropertiesManagerFactory::RegisterProfilePrefs(registry); + ntp_snippets::NTPSnippetsService::RegisterProfilePrefs(registry); password_manager::PasswordManager::RegisterProfilePrefs(registry); PrefProxyConfigTrackerImpl::RegisterProfilePrefs(registry); sync_driver::SyncPrefs::RegisterProfilePrefs(registry); |