Add ContentSuggestionsCategoryFactory; Store categories as ints

components/ntp_snippets/content_suggestions_category.h

 // 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_CONTENT_SUGGESTIONS_CATEGORY_H_
 #define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_CATEGORY_H_
 
+#include <ostream>
+
 namespace ntp_snippets {
 
-// A category groups ContentSuggestions which belong together.
-enum class ContentSuggestionsCategory { ARTICLES, OFFLINE_PAGES, COUNT };
+class ContentSuggestionsCategoryFactory;
+
+// These are the categories that the client knows about.
+// The values before LOCAL_CATEGORIES_COUNT are the categories that are provided
+// locally on the device. Those values need to be listed in the
+// ContentSuggestionsCategoryFactory constructor!
+// Categories provided by the server (IDs strictly larger than
+// REMOTE_CATEGORIES_OFFSET) only need to be hard-coded here if they need to be
+// recognized by the client implementation.
+enum class KnownSuggestionsCategories {
+  OFFLINE_PAGES,
+  LOCAL_CATEGORIES_COUNT,
+
+  REMOTE_CATEGORIES_OFFSET = 10000,

[Bernhard Bauer, 2016/07/28 15:39:27]

Stupid question: If the local categories are fixed anyway, why do we need the
gap between local and remote categories? Are we going to store the values on
disk?

[Philipp Keck, 2016/07/28 16:59:58]

It's because they're also hard-coded on the server (as ARTICLES = 1, ...) and
they need to match the numbers here, regardless of what Chromium version it is
(and we may have added other local categories in the meantime, which would shift
the values).

[Marc Treib, 2016/07/29 08:20:35]

We don't actually need a gap between them - the first remote category could be
the same as LOCAL_CATEGORIES_COUNT. Since we don't store these numbers anywhere,
that'd be fine - if a new local category is introduced, that implies a restart
of Chrome, and so all the numbers will get reassigned.
The gap might be helpful for debugging, since you can tell immediately if a
category is local or not.

[Philipp Keck, 2016/07/29 08:24:28]

Right. There is a second reason why it's helpful for debugging: 10005 looks
almost like 5, so it's easier to tell which one it is.

On the other hand, removing the gap has the obvious advantage that there is no
gap.

+  ARTICLES = REMOTE_CATEGORIES_OFFSET + 1,
+};
+
+// A category groups ContentSuggestions which belong together. Use the
+// ContentSuggestionsCategoryFactory to obtain instances.
+class ContentSuggestionsCategory {
+ public:
+  int id() const { return id_; }
+
+  bool IsKnownCategory(KnownSuggestionsCategories known_category) const;
+
+ private:
+  friend class ContentSuggestionsCategoryFactory;
+
+  ContentSuggestionsCategory(int id);
+
+  int id_;

[Bernhard Bauer, 2016/07/28 15:39:26]

Nit: It's quite common to add a comment that copying and assignment are allowed,
just to make sure that no one will try and "fix" the lack of a
DISALLOW_COPY_AND_ASSIGN :)

[Philipp Keck, 2016/07/28 16:59:58]

I added it, not sure about the wording and location of the comment.

+};
+
+bool operator==(const ContentSuggestionsCategory& left,
+                const ContentSuggestionsCategory& right);
+
+bool operator!=(const ContentSuggestionsCategory& left,
+                const ContentSuggestionsCategory& right);
+
+std::ostream& operator<<(std::ostream& os,
+                         const ContentSuggestionsCategory& obj);
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_CATEGORY_H_