Fetch snippets from ChromeReader and show them on the NTP

components/ntp_snippets/ntp_snippets_service.cc

Index: components/ntp_snippets/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc
index f3c0670f7af808486dbfd6cd80572c73bfc971f5..a16f7cd8b266f605b95343e8239736eece614dd5 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -4,16 +4,40 @@
 
 #include "components/ntp_snippets/ntp_snippets_service.h"
 
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/json/json_string_value_serializer.h"
+#include "base/path_service.h"
+#include "base/task_runner_util.h"
 #include "base/values.h"
 
 namespace ntp_snippets {
 
+extern base::FilePath GetSnippetsSuggestionsPath();

[Bernhard Bauer, 2016/02/08 18:19:27]

Wait, what's this doing here? This seems like it should be declared in the
header of either this class or NTPSnippetsFetcher. Probably the latter, now that
I think about it.

[May, 2016/02/09 17:38:54]

I removed this, since I changed the callback model.

+
+std::string ReadFileToString(const base::FilePath& path) {
+  std::string data;
+  bool success = base::ReadFileToString(path, &data);
+  if (!success) {

[Bernhard Bauer, 2016/02/08 18:19:26]

No braces. Also, you could use DLOG_IF().

[May, 2016/02/09 17:38:54]

Done.

+    DLOG(ERROR) << "Error reading file " << path.LossyDisplayName();
+  }
+  return data;
+}
+
 NTPSnippetsService::NTPSnippetsService(
-    const std::string& application_language_code)
-    : loaded_(false), application_language_code_(application_language_code) {}
+    scoped_refptr<base::SequencedTaskRunner> file_task_runner,
+    const std::string& application_language_code,
+    NTPSnippetsFetcher* snippets_fetcher)
+    : loaded_(false),
+      file_task_runner_(file_task_runner),
+      application_language_code_(application_language_code) {
+  snippets_fetcher_.reset(snippets_fetcher);

[Bernhard Bauer, 2016/02/08 18:19:27]

You could directly initialize this in the initializer list.

[May, 2016/02/09 17:38:53]

Done.

+  snippets_fetcher_->AddObserver(this);
+}
 
-NTPSnippetsService::~NTPSnippetsService() {}
+NTPSnippetsService::~NTPSnippetsService() {
+  snippets_fetcher_->RemoveObserver(this);
+}
 
 void NTPSnippetsService::Shutdown() {
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
@@ -21,6 +45,10 @@ void NTPSnippetsService::Shutdown() {
   loaded_ = false;
 }
 
+void NTPSnippetsService::FetchSnippets(bool overwrite) {
+  snippets_fetcher_->FetchSnippets(overwrite);
+}
+
 void NTPSnippetsService::AddObserver(NTPSnippetsServiceObserver* observer) {
   observers_.AddObserver(observer);
   if (loaded_)
@@ -31,6 +59,10 @@ void NTPSnippetsService::RemoveObserver(NTPSnippetsServiceObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void NTPSnippetsService::OnFileReadDone(std::string& json) {
+  LoadFromJSONString(json);
+}
+
 bool NTPSnippetsService::LoadFromJSONString(const std::string& str) {
   JSONStringValueDeserializer deserializer(str);
   int error_code;
@@ -38,8 +70,9 @@ bool NTPSnippetsService::LoadFromJSONString(const std::string& str) {
 
   scoped_ptr<base::Value> deserialized =
       deserializer.Deserialize(&error_code, &error_message);
-  if (!deserialized)
+  if (!deserialized) {

[Bernhard Bauer, 2016/02/08 18:19:26]

Eh, undo please 😃

[May, 2016/02/09 17:38:54]

Done.

     return false;
+  }
 
   const base::DictionaryValue* top_dict = NULL;
   if (!deserialized->GetAsDictionary(&top_dict))
@@ -64,9 +97,18 @@ bool NTPSnippetsService::LoadFromJSONString(const std::string& str) {
     snippets_.push_back(std::move(snippet));
   }
   loaded_ = true;
+
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceLoaded(this));
   return true;
 }
 
+// NTPSnippetsFetcher::Observer overrides:
+void NTPSnippetsService::OnNTPSnippetsDownloaded() {
+  base::PostTaskAndReplyWithResult(
+      file_task_runner_.get(), FROM_HERE,
+      base::Bind(&ReadFileToString, GetSnippetsSuggestionsPath()),
+      base::Bind(&NTPSnippetsService::OnFileReadDone, base::Unretained(this)));

[Marc Treib, 2016/02/09 09:17:54]

I don't think base::Unretained is safe here. You probably need to add a
WeakPtrFactory and make this weak_ptr_factory_.GetWeakPtr()

[May, 2016/02/09 17:38:53]

Done.

+}
+
 }  // namespace ntp_snippets