Add ContentSuggestionsService

components/ntp_snippets/content_suggestions_service.cc

Index: components/ntp_snippets/content_suggestions_service.cc
diff --git a/components/ntp_snippets/content_suggestions_service.cc b/components/ntp_snippets/content_suggestions_service.cc
new file mode 100644
index 0000000000000000000000000000000000000000..aadb464c434026f4a52076d9ca5b297ebdca60f3
--- /dev/null
+++ b/components/ntp_snippets/content_suggestions_service.cc
@@ -0,0 +1,203 @@
+// 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.
+
+#include "components/ntp_snippets/content_suggestions_service.h"
+
+#include <algorithm>
+#include <iterator>
+
+#include "base/bind.h"
+#include "base/strings/string_number_conversions.h"
+#include "ui/gfx/image/image.h"
+
+namespace ntp_snippets {
+
+ContentSuggestionsService::ContentSuggestionsService(State state)
+    : state_(state) {}
+
+ContentSuggestionsService::~ContentSuggestionsService() {}
+
+void ContentSuggestionsService::Shutdown() {
+  DCHECK(providers_.empty());
+  DCHECK(categories_.empty());
+  DCHECK(suggestions_by_category_.empty());
+  DCHECK(id_category_map_.empty());
+  state_ = State::DISABLED;
+  FOR_EACH_OBSERVER(Observer, observers_, ContentSuggestionsServiceShutdown());
+}
+
+ContentSuggestionsCategoryStatus ContentSuggestionsService::GetCategoryStatus(
+    ContentSuggestionsCategory category) const {
+  if (state_ == State::DISABLED)
+    return ContentSuggestionsCategoryStatus::

[Marc Treib, 2016/07/07 09:37:51]

nit: braces if the body doesn't fit on one line

[Philipp Keck, 2016/07/07 12:22:30]

Done.

+        ALL_SUGGESTIONS_EXPLICITLY_DISABLED;
+
+  auto iterator = providers_.find(category);
+  if (iterator == providers_.end())
+    return ContentSuggestionsCategoryStatus::NOT_PROVIDED;
+
+  return iterator->second->GetCategoryStatus(category);
+}
+
+const std::vector<ContentSuggestion>&
+ContentSuggestionsService::GetSuggestionsForCategory(
+    ContentSuggestionsCategory category) const {
+  auto iterator = suggestions_by_category_.find(category);
+  if (iterator == suggestions_by_category_.end()) {

[Bernhard Bauer, 2016/07/07 10:02:19]

And no braces here ☺

[Philipp Keck, 2016/07/07 12:22:29]

Done.

+    return no_suggestions_;
+  }
+  return iterator->second;
+}
+
+void ContentSuggestionsService::FetchSuggestionImage(
+    const std::string& suggestion_id,
+    const ImageFetchedCallback& callback) {
+  if (!id_category_map_.count(suggestion_id)) {
+    LOG(WARNING) << "Requested image for unknown suggestion " << suggestion_id;
+    callback.Run(suggestion_id, gfx::Image());
+    return;
+  }
+  ContentSuggestionsCategory category = id_category_map_[suggestion_id];
+  if (!providers_.count(category)) {
+    LOG(WARNING) << "Requested image for suggestion " << suggestion_id
+                 << " for unavailable category " << int(category);
+    callback.Run(suggestion_id, gfx::Image());
+    return;
+  }
+  providers_[category]->FetchSuggestionImage(suggestion_id, callback);
+}
+
+void ContentSuggestionsService::ClearCachedSuggestionsForDebugging() {
+  suggestions_by_category_.clear();
+  id_category_map_.clear();
+  for (auto& provider : providers_) {

[Marc Treib, 2016/07/07 09:37:51]

nit: The elements are not providers, but category+provider pairs, right? So
maybe call the variable category_and_provider or something like that?
(Rule of thumb: Only use "auto" when the type is obvious or not interesting
(e.g. iterator). Neither is really the case here, but spelling out the pair is
also ugly...)

[Philipp Keck, 2016/07/07 12:22:30]

Done.

+    provider.second->ClearCachedSuggestionsForDebugging();
+  }
+  FOR_EACH_OBSERVER(Observer, observers_, OnNewSuggestions());
+}
+
+void ContentSuggestionsService::ClearDiscardedSuggestionsForDebugging() {
+  for (auto& provider : providers_) {

[Marc Treib, 2016/07/07 09:37:51]

Same here

[Philipp Keck, 2016/07/07 12:22:30]

Done.

+    provider.second->ClearDiscardedSuggestionsForDebugging();
+  }
+}
+
+void ContentSuggestionsService::DiscardSuggestion(
+    const std::string& suggestion_id) {
+  if (!id_category_map_.count(suggestion_id)) {
+    LOG(WARNING) << "Discarded unknown suggestion " << suggestion_id;
+    return;
+  }
+  ContentSuggestionsCategory category = id_category_map_[suggestion_id];
+  if (!providers_.count(category)) {
+    LOG(WARNING) << "Discarded suggestion " << suggestion_id
+                 << " for unavailable category " << int(category);
+    return;
+  }
+  providers_[category]->DiscardSuggestion(suggestion_id);
+
+  // Remove the suggestion locally.
+  id_category_map_.erase(suggestion_id);
+  std::vector<ContentSuggestion>* suggestions =
+      &suggestions_by_category_[category];
+  auto position =
+      std::find_if(suggestions->begin(), suggestions->end(),
+                   [&suggestion_id](const ContentSuggestion& suggestion) {
+                     return suggestion_id == suggestion.id();
+                   });
+  DCHECK(position != suggestions->end());
+  suggestions->erase(position);
+}
+
+void ContentSuggestionsService::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void ContentSuggestionsService::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void ContentSuggestionsService::RegisterProvider(
+    ContentSuggestionsProvider* provider) {
+  if (state_ == State::DISABLED)

[Marc Treib, 2016/07/07 09:37:51]

Are we sure about this? We'll have to change it once the service state can
change at runtime. (Then again, it can't yet, so maybe this is best for now.)

[Philipp Keck, 2016/07/07 12:22:30]

Yes. It is here because the NTPSnippetsService will still be enabled for a while
and will deliver to UI directly, but if the ContentSuggestionsService is
additionally enabled, it also delivers data there (initially only for debugging,
later because the UI reads the data from the new service). This "if" makes it
clear also to the NTPSnippetsService whether the new service is active or not
(because if it's disabled, it won't register its observer, so the
NTPSnippetsService knows that it doesn't need to deliver any data).

In a later stage, providers will just use the ::state() getter on the
ContentSuggestionsService to see if they need to produce data or not. And then
this can be removed, at the latest when the state can change at runtime, like
you said.

[Marc Treib, 2016/07/07 12:32:16]

Alright. Add a TODO maybe?

[Philipp Keck, 2016/07/07 12:44:06]

Done.

+    return;
+
+  for (ContentSuggestionsCategory category : provider->provided_categories()) {
+    DCHECK_EQ(0ul, providers_.count(category));
+    providers_[category] = provider;
+    // TODO(pke) In the future, make sure that the categories have some useful

[Bernhard Bauer, 2016/07/07 10:02:19]

Nit: The exact format for TODOs is TODO(ldap): ☺

[Philipp Keck, 2016/07/07 12:22:29]

Done.

+    // (maybe constant, at least consistent) ordering for the UI.
+    categories_.push_back(category);
+    if (IsContentSuggestionsCategoryStatusAvailable(
+            provider->GetCategoryStatus(category))) {
+      suggestions_by_category_[category] = std::vector<ContentSuggestion>();
+    }
+    provider->SetObserver(this);

[Marc Treib, 2016/07/07 09:37:51]

This should go out of the loop

[Philipp Keck, 2016/07/07 12:22:29]

Done.

+    FOR_EACH_OBSERVER(Observer, observers_,
+                      OnCategoryStatusChanged(
+                          category, provider->GetCategoryStatus(category)));
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Private methods
+
+void ContentSuggestionsService::OnNewSuggestions(
+    ContentSuggestionsCategory changed_category,
+    std::vector<ContentSuggestion> new_suggestions) {
+  DCHECK(IsCategoryRegistered(changed_category));
+
+  for (const ContentSuggestion& suggestion :
+       suggestions_by_category_[changed_category]) {
+    id_category_map_.erase(suggestion.id());
+  }
+
+  for (const ContentSuggestion& suggestion : new_suggestions) {
+    id_category_map_[suggestion.id()] = changed_category;
+  }
+
+  suggestions_by_category_[changed_category] = std::move(new_suggestions);
+
+  FOR_EACH_OBSERVER(Observer, observers_, OnNewSuggestions());
+}
+
+void ContentSuggestionsService::OnCategoryStatusChanged(
+    ContentSuggestionsCategory changed_category,
+    ContentSuggestionsCategoryStatus new_status) {
+  if (!IsContentSuggestionsCategoryStatusAvailable(new_status)) {
+    for (const ContentSuggestion& suggestion :
+         suggestions_by_category_[changed_category]) {
+      id_category_map_.erase(suggestion.id());
+    }
+    suggestions_by_category_.erase(changed_category);
+  }

[Marc Treib, 2016/07/07 09:37:51]

Do we also need to do something if it is now available? Like add an entry to
categories_ and/or suggestions_by_category_?

[Philipp Keck, 2016/07/07 12:22:30]

No. id_category_map_ and suggestions_by_category_ are filled by OnNewSuggestions
and otherwise behave as if they were empty - even if an empty entry is not
explicitly added.
categories_ always contains the category, even if it is not available (but only
if a provider for it is present). The categories_ vector is supposed to inform
which categories exist (i.e. which you can query the status for, and if the
status is "available" then query the suggestions).

+  FOR_EACH_OBSERVER(Observer, observers_,
+                    OnCategoryStatusChanged(changed_category, new_status));
+}
+
+void ContentSuggestionsService::OnProviderShutdown(
+    ContentSuggestionsProvider* provider) {
+  for (ContentSuggestionsCategory category : provider->provided_categories()) {
+    auto iterator = std::find(categories_.begin(), categories_.end(), category);
+    DCHECK(iterator != categories_.end());
+    categories_.erase(iterator);
+    for (const ContentSuggestion& suggestion :
+         suggestions_by_category_[category]) {
+      id_category_map_.erase(suggestion.id());
+    }
+    suggestions_by_category_.erase(category);
+    providers_.erase(category);
+    FOR_EACH_OBSERVER(
+        Observer, observers_,
+        OnCategoryStatusChanged(category, GetCategoryStatus(category)));

[Marc Treib, 2016/07/07 09:37:51]

optional: You could replace all the OnCategoryStatusChanged calls by a 
NotifyCategoryStatusChanged method (the calls aren't exactly the same, but looks
they could all just call GetCategoryStatus(category) to get the new status).

[Philipp Keck, 2016/07/07 12:22:29]

Done.

+  }
+}
+
+bool ContentSuggestionsService::IsCategoryRegistered(
+    ContentSuggestionsCategory category) const {
+  return std::find(categories_.begin(), categories_.end(), category) !=
+         categories_.end();
+}
+
+}  // namespace ntp_snippets