Add ContentSuggestionsCategoryFactory; Store categories as ints

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 <stddef.h>
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/observer_list.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/ntp_snippets/content_suggestions_category_factory.h"
 #include "components/ntp_snippets/content_suggestions_category_status.h"
 #include "components/ntp_snippets/content_suggestions_provider.h"
 
 namespace gfx {
 class Image;
 }
 
 namespace ntp_snippets {
 
 // Retrieves suggestions from a number of ContentSuggestionsProviders and serves
@@ skipping 28 matching lines @@
 
    protected:
     virtual ~Observer() {}
   };
 
   enum State {
     ENABLED,
     DISABLED,
   };
 
+  // This is just an arbitrary ordering by ID, used by the maps in this class,
+  // because the ordering needs to be constant for maps.
+  struct CompareCategoriesByID {

[Marc Treib, 2016/07/28 11:41:46]

This should be private.

[Philipp Keck, 2016/07/28 13:50:55]

Done.

+    bool operator()(const ContentSuggestionsCategory& left,
+                    const ContentSuggestionsCategory& right) const;
+  };
+
   ContentSuggestionsService(State state);
   ~ContentSuggestionsService() override;
 
   // Inherited from KeyedService.
   void Shutdown() override;
 
   State state() { return state_; }
 
   // Gets all categories for which a provider is registered. The categories
   // may or may not be available, see |GetCategoryStatus()|.
@@ skipping 37 matching lines @@
   // same results when they fetch the next time. In particular, calling this
   // method will not mark any suggestions as dismissed.
   void ClearCachedSuggestionsForDebugging();
 
   // Only for debugging use through the internals page. Some providers
   // internally store a list of dismissed suggestions to prevent them from
   // reappearing. This function clears all such lists in all providers, making
   // dismissed suggestions reappear (only for certain providers).
   void ClearDismissedSuggestionsForDebugging();
 
+  ContentSuggestionsCategoryFactory* category_factory() {

[Marc Treib, 2016/07/28 11:41:46]

Clients aren't supposed to register new categories with this, right? And also
they shouldn't keep the pointer around? In that case, you could return a const
ref.

[Philipp Keck, 2016/07/28 13:50:54]

Yes, the providers use this to register categories. And they do keep the pointer
around (none of the current providers does, but the server-provider will have to
do so, as it's the only way to access the factory).

[Marc Treib, 2016/07/28 14:31:17]

Ah - technically the providers just get a Factory ptr passed in, but the
ProviderFactory gets it from here. Alright, carry on then.

[Philipp Keck, 2016/07/28 14:55:47]

Acknowledged.

+    return &category_factory_;
+  }
+
  private:
   friend class ContentSuggestionsServiceTest;
 
   // Implementation of ContentSuggestionsProvider::Observer.
   void OnNewSuggestions(ContentSuggestionsCategory changed_category,
                         std::vector<ContentSuggestion> suggestions) override;
   void OnCategoryStatusChanged(
       ContentSuggestionsCategory changed_category,
       ContentSuggestionsCategoryStatus new_status) override;
   void OnProviderShutdown(ContentSuggestionsProvider* provider) override;
 
   // Checks whether a provider for the given |category| is registered.
   bool IsCategoryRegistered(ContentSuggestionsCategory category) const;
 
   // Fires the OnCategoryStatusChanged event for the given |category|.
   void NotifyCategoryStatusChanged(ContentSuggestionsCategory category);
 
   // Whether the content suggestions feature is enabled.
   State state_;
 
+  // Provides new categories and an order for them.

[Marc Treib, 2016/07/28 11:41:46]

Provides all categories really, no?

[Philipp Keck, 2016/07/28 13:50:55]

True. I put "new and existing" because "all" could confusingly also mean that
all categories are in there, even the inactive ones.

+  ContentSuggestionsCategoryFactory category_factory_;
+
   // All registered providers. A provider may be contained multiple times, if it
   // provides multiple categories. The keys of this map are exactly the entries
   // of |categories_|.
-  std::map<ContentSuggestionsCategory, ContentSuggestionsProvider*> providers_;
+  std::map<ContentSuggestionsCategory,
+           ContentSuggestionsProvider*,
+           CompareCategoriesByID>
+      providers_;
 
-  // All current suggestion categories, in an order determined by the service.
-  // Currently, this is simply the order in which the providers were registered.
-  // This vector contains exactly the same categories as |providers_|.
-  // TODO(pke): Implement a useful and consistent ordering for categories.
+  // All current suggestion categories, in an order determined by the
+  // |category_factory_|. This vector contains exactly the same categories as
+  // |providers_|.
   std::vector<ContentSuggestionsCategory> 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<ContentSuggestionsCategory, std::vector<ContentSuggestion>>
+  std::map<ContentSuggestionsCategory,
+           std::vector<ContentSuggestion>,
+           CompareCategoriesByID>
       suggestions_by_category_;
 
   // Map used to determine the category of a suggestion (of which only the ID
   // is available). This also determines the provider that delivered the
   // suggestion.
   std::map<std::string, ContentSuggestionsCategory> id_category_map_;
 
   base::ObserverList<Observer> observers_;
 
   const std::vector<ContentSuggestion> no_suggestions_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentSuggestionsService);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_H_