[NTP::SectionOrder] Add EM oriented order.

components/ntp_snippets/category_rankers/constant_category_ranker.cc

Index: components/ntp_snippets/category_rankers/constant_category_ranker.cc
diff --git a/components/ntp_snippets/category_rankers/constant_category_ranker.cc b/components/ntp_snippets/category_rankers/constant_category_ranker.cc
index 60f040dcf53d7f6e8988c3ab3dbe0a9011902562..9ab04cfe56c33446c3d99d9c50c8c53d1281d927 100644
--- a/components/ntp_snippets/category_rankers/constant_category_ranker.cc
+++ b/components/ntp_snippets/category_rankers/constant_category_ranker.cc
@@ -5,6 +5,7 @@
 #include "components/ntp_snippets/category_rankers/constant_category_ranker.h"
 
 #include "base/stl_util.h"
+#include "components/ntp_snippets/features.h"
 
 namespace ntp_snippets {
 
@@ -68,24 +69,37 @@ void ConstantCategoryRanker::OnCategoryDismissed(Category category) {
 std::vector<KnownCategories>
 ConstantCategoryRanker::GetKnownCategoriesDefaultOrder() {
   std::vector<KnownCategories> categories;
+  CategoryOrderChoice choice = GetSelectedCategoryOrder();
+  switch (choice) {
+    case CategoryOrderChoice::GENERAL:
+      categories.push_back(KnownCategories::PHYSICAL_WEB_PAGES);
+      categories.push_back(KnownCategories::DOWNLOADS);
+      categories.push_back(KnownCategories::RECENT_TABS);
+      categories.push_back(KnownCategories::FOREIGN_TABS);
+      categories.push_back(KnownCategories::BOOKMARKS);
+      categories.push_back(KnownCategories::ARTICLES);

[Marc Treib, 2017/02/13 14:25:23]

ARTICLES wasn't listed here before. Are you sure this won't break anything?

[vitaliii, 2017/02/14 09:32:32]

It was (see line 87 in the old code).
Under this case, |categories| is exactly the same as it was previously.

[Marc Treib, 2017/02/14 09:48:54]

Ah, indeed. I overlooked that, sorry.

[vitaliii, 2017/02/14 12:13:05]

Acknowledged.

+      break;
+    case CategoryOrderChoice::EMERGING_MARKETS_ORIENTED:
+      categories.push_back(KnownCategories::ARTICLES);
+      categories.push_back(KnownCategories::DOWNLOADS);
+      categories.push_back(KnownCategories::BOOKMARKS);
+
+      categories.push_back(KnownCategories::PHYSICAL_WEB_PAGES);
+      categories.push_back(KnownCategories::RECENT_TABS);
+      categories.push_back(KnownCategories::FOREIGN_TABS);
+      break;
+    default:

[Marc Treib, 2017/02/13 14:25:23]

Don't list a "default" case. Then, when a new enum entry is added without also
updating the switch, there'll be a compile error.

[vitaliii, 2017/02/14 09:32:32]

Done.

Cool, I did not know this.

+      NOTREACHED();
+  }
 
-  // Add all local categories in a fixed order.
-  categories.push_back(KnownCategories::PHYSICAL_WEB_PAGES);
-  categories.push_back(KnownCategories::DOWNLOADS);
-  categories.push_back(KnownCategories::RECENT_TABS);
-  categories.push_back(KnownCategories::FOREIGN_TABS);
-  categories.push_back(KnownCategories::BOOKMARKS);
-
-  DCHECK_EQ(static_cast<size_t>(KnownCategories::LOCAL_CATEGORIES_COUNT),
-            categories.size());
-
-  // Known remote categories come after. Other remote categories will be ordered
-  // after these depending on when providers notify us about them using
-  // AppendCategoryIfNecessary.
-  // TODO(treib): Consider not adding ARTICLES here, so that providers can
-  // define the order themselves.
-  categories.push_back(KnownCategories::ARTICLES);
+  DCHECK_EQ(
+      static_cast<size_t>(KnownCategories::LOCAL_CATEGORIES_COUNT) +
+          static_cast<size_t>(KnownCategories::LAST_KNOWN_REMOTE_CATEGORY) -
+          static_cast<size_t>(KnownCategories::REMOTE_CATEGORIES_OFFSET),

[Marc Treib, 2017/02/13 14:25:23]

This isn't really correct - it works now, but it's not guaranteed that all known
remote categories will always have contiguous IDs. (In fact, as soon as we add
any other known remote category, they won't be contiguous anymore.)

I'm not sure what's best to do here - we do want to make sure any local
categories are listed here. Maybe a "static_assert(LOCAL_CATEGORIES_COUNT ==
<current known value>)" with a proper message is enough? WDYT?

[vitaliii, 2017/02/14 09:32:32]

Done.

+      categories.size());
 
+  // Other remote categories will be ordered after these depending on when
+  // providers notify us about them using AppendCategoryIfNecessary.
   return categories;
 }