[NTP::SectionOrder] Propagate new order through ContentSuggestionsService.

components/ntp_snippets/content_suggestions_service.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_SERVICE_H_
 #define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_H_
 
 #include <map>
 #include <memory>
 #include <set>
@@ skipping 82 matching lines @@
                             std::unique_ptr<CategoryRanker> category_ranker);
   ~ContentSuggestionsService() override;
 
   // Inherited from KeyedService.
   void Shutdown() override;
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   State state() { return state_; }
 
-  // Gets all categories for which a provider is registered. The categories
-  // may or may not be available, see |GetCategoryStatus()|.
-  const std::vector<Category>& GetCategories() const { return categories_; }
+  // Gets all categories for which a provider is registered. The categories may
+  // or may not be available, see |GetCategoryStatus()|. The order is obtained
+  // from |category_ranker_| immediately.

[tschumann, 2016/12/19 09:06:38]

IMO, this comment is exposing too many implementation details. What's important
is that the order in which the categories are returned is the order in which
they should be displayed.
It doesn't matter how or with which members the service is computing this order.

[vitaliii, 2016/12/19 11:31:39]

Done.

+  const std::vector<Category>& GetCategories();
 
   // Gets the status of a category.
   CategoryStatus GetCategoryStatus(Category category) const;
 
   // Gets the meta information of a category.
   base::Optional<CategoryInfo> GetCategoryInfo(Category category) const;
 
   // Gets the available suggestions for a category. The result is empty if the
   // category is available and empty, but also if the category is unavailable
   // for any reason, see |GetCategoryStatus()|.
@@ skipping 162 matching lines @@
       providers_by_category_;
 
   // All dismissed categories and their providers. These may be restored by
   // RestoreDismissedCategories(). The provider can be null if the dismissed
   // category has received no updates since initialisation.
   // (see RestoreDismissedCategoriesFromPrefs())
   std::map<Category, ContentSuggestionsProvider*, Category::CompareByID>
       dismissed_providers_by_category_;
 
   // All current suggestion categories, in an order determined by the
-  // |category_factory_|. This vector contains exactly the same categories as
-  // |providers_by_category_|.
+  // |category_ranker_|. The order here may be stale, i.e. changes in the ranker

[tschumann, 2016/12/19 09:06:38]

nit: let's phrase this a bit clearer:
The order in categories_ is not specified.

Also, please don't add too many implementation details -- it'll get out of sync
with the implementation (no need to mention SortCategories() or the ranker).
The last sentence is worth keeping though.

[vitaliii, 2016/12/19 11:31:39]

Done.

+  // are not immediately propagated here (use SortCategories() to refresh). This
+  // vector contains exactly the same categories as |providers_by_category_|.
   std::vector<Category> categories_;
 
   // All current suggestions grouped by category. This contains an entry for
   // every category in |categories_| whose status is an available status. It may
   // contain an empty vector if the category is available but empty (or still
   // loading).
   std::map<Category, std::vector<ContentSuggestion>, Category::CompareByID>
       suggestions_by_category_;
 
   // Observer for the SigninManager. All observers are notified when the signin
@@ skipping 22 matching lines @@
 
   // Provides order for categories.
   std::unique_ptr<CategoryRanker> category_ranker_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentSuggestionsService);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_H_