[NTP Snippets] Persist non-article remote suggestions in the DB

components/ntp_snippets/remote/ntp_snippet.cc

Index: components/ntp_snippets/remote/ntp_snippet.cc
diff --git a/components/ntp_snippets/remote/ntp_snippet.cc b/components/ntp_snippets/remote/ntp_snippet.cc
index 496e1fa5ba8bfac72fc8880d53f6063e64fb7e6a..4c69412ec1225b2b6944ac55415d52e5e066dec9 100644
--- a/components/ntp_snippets/remote/ntp_snippet.cc
+++ b/components/ntp_snippets/remote/ntp_snippet.cc
@@ -8,11 +8,19 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
-
+#include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/remote/proto/ntp_snippets.pb.h"
 
+namespace ntp_snippets {
+
 namespace {
 
+const int kArticlesRemoteId = 1;

[Bernhard Bauer, 2016/10/10 13:54:54]

I'm starting to get the feeling that just using a pair<bool, int> might have
been easier :)

[Marc Treib, 2016/10/10 15:54:47]

...I don't quite follow? You mean essentially store an "Optional<int>
remote_id_" in NTPSnippet?

[Bernhard Bauer, 2016/10/10 16:03:12]

No, I meant that maybe we should have stored category IDs as pairs of
(local/remote x ID) instead of encoding both in a single int value.

[Marc Treib, 2016/10/10 16:04:14]

Aah - yes, that might have made sense...

+static_assert(int(KnownCategories::ARTICLES) -

[Bernhard Bauer, 2016/10/10 13:54:54]

Should this be a static_cast<int>()?

[Marc Treib, 2016/10/10 15:54:47]

Sure, done.

+                      int(KnownCategories::REMOTE_CATEGORIES_OFFSET) ==
+                  kArticlesRemoteId,
+              "kArticlesRemoteId has a wrong value?!");
+
 // dict.Get() specialization for base::Time values
 bool GetTimeValue(const base::DictionaryValue& dict,
                   const std::string& key,
@@ -36,10 +44,12 @@ bool GetURLValue(const base::DictionaryValue& dict,
 
 }  // namespace
 
-namespace ntp_snippets {
-
-NTPSnippet::NTPSnippet(const std::string& id)
-    : id_(id), score_(0), is_dismissed_(false), best_source_index_(0) {}
+NTPSnippet::NTPSnippet(const std::string& id, int remote_category_id)
+    : id_(id),
+      score_(0),
+      is_dismissed_(false),
+      remote_category_id_(remote_category_id),
+      best_source_index_(0) {}
 
 NTPSnippet::~NTPSnippet() = default;
 
@@ -55,7 +65,7 @@ std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromChromeReaderDictionary(
   if (!content->GetString("url", &id) || id.empty())
     return nullptr;
 
-  std::unique_ptr<NTPSnippet> snippet(new NTPSnippet(id));
+  std::unique_ptr<NTPSnippet> snippet(new NTPSnippet(id, kArticlesRemoteId));
 
   std::string title;
   if (content->GetString("title", &title))
@@ -138,7 +148,8 @@ std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromChromeReaderDictionary(
 
 // static
 std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromContentSuggestionsDictionary(
-    const base::DictionaryValue& dict) {
+    const base::DictionaryValue& dict,
+    int remote_category_id) {
   const base::ListValue* ids;
   std::string id;
   if (!(dict.GetList("ids", &ids) &&
@@ -146,7 +157,7 @@ std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromContentSuggestionsDictionary(
     return nullptr;
   }
 
-  auto snippet = base::MakeUnique<NTPSnippet>(id);
+  auto snippet = base::MakeUnique<NTPSnippet>(id, remote_category_id);
   snippet->sources_.emplace_back(GURL(), std::string(), GURL());
   auto* source = &snippet->sources_.back();
   snippet->best_source_index_ = 0;
@@ -175,7 +186,12 @@ std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromProto(
   if (!proto.has_id() || proto.id().empty())
     return nullptr;
 
-  std::unique_ptr<NTPSnippet> snippet(new NTPSnippet(proto.id()));
+  int remote_category_id = kArticlesRemoteId;
+  if (proto.has_remote_category_id())
+    remote_category_id = proto.remote_category_id();

[Bernhard Bauer, 2016/10/10 13:54:54]

Nit: Maybe use a ternary operator so you only assign the variable once?

[Marc Treib, 2016/10/10 15:54:47]

Done.

+
+  std::unique_ptr<NTPSnippet> snippet(
+      new NTPSnippet(proto.id(), remote_category_id));
 
   snippet->set_title(proto.title());
   snippet->set_snippet(proto.snippet());
@@ -231,6 +247,7 @@ SnippetProto NTPSnippet::ToProto() const {
     result.set_expiry_date(expiry_date_.ToInternalValue());
   result.set_score(score_);
   result.set_dismissed(is_dismissed_);
+  result.set_remote_category_id(remote_category_id_);
 
   for (const SnippetSource& source : sources_) {
     SnippetSourceProto* source_proto = result.add_sources();