Add ContentSuggestionsService

components/ntp_snippets/content_suggestions_service.cc

+// 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(Enabled enabled)
+    : enabled_(enabled) {}
+
+ContentSuggestionsService::~ContentSuggestionsService() {}
+
+void ContentSuggestionsService::Shutdown() {
+  DCHECK(providers_.empty());
+  DCHECK(categories_.empty());
+  DCHECK(suggestions_by_category_.empty());
+  DCHECK(suggestions_by_category_.empty());

[Marc Treib, 2016/07/05 13:17:12]

Double DCHECK

[Philipp Keck, 2016/07/05 13:51:59]

Done.

+  enabled_ = Enabled::DISABLE;
+  FOR_EACH_OBSERVER(Observer, observers_, ContentSuggestionsServiceShutdown());
+  observers_.Clear();
+}
+
+ContentSuggestionsState ContentSuggestionsService::GetCategoryState(
+    ContentSuggestionCategory category) const {
+  if (enabled_ == Enabled::DISABLE)
+    return ContentSuggestionsState::ALL_SUGGESTIONS_EXPLICITLY_DISABLED;
+
+  auto iterator = providers_.find(category);
+  if (iterator == providers_.end())
+    return ContentSuggestionsState::NOT_PROVIDED;
+
+  return iterator->second->GetCategoryState(category);
+}
+
+const std::vector<ContentSuggestion>&
+ContentSuggestionsService::GetSuggestionsForCategory(
+    ContentSuggestionCategory category) const {
+  auto iterator = suggestions_by_category_.find(category);
+  if (iterator == suggestions_by_category_.end()) {
+    return empty_categories_list_;
+  }
+  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());
+  }
+  ContentSuggestionCategory 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_) {
+    provider.second->ClearCachedSuggestionsForDebugging();
+  }
+  FOR_EACH_OBSERVER(Observer, observers_, OnNewSuggestions());
+}
+
+void ContentSuggestionsService::ClearDiscardedSuggestionsForDebugging() {
+  for (auto& provider : providers_) {
+    provider.second->ClearDiscardedSuggestionsForDebugging();
+  }
+}
+
+void ContentSuggestionsService::DiscardSuggestion(
+    const std::string& suggestion_id) {
+  if (!id_category_map_.count(suggestion_id)) {
+    LOG(WARNING) << "Requested image for unknown suggestion " << suggestion_id;

[Marc Treib, 2016/07/05 13:17:12]

Too much copypasta :)

[Philipp Keck, 2016/07/05 13:51:59]

Done. And a return statement was missing, too.

+  }
+  ContentSuggestionCategory 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();
+                   });
+  if (position != suggestions->end()) {

[Marc Treib, 2016/07/05 13:17:12]

DCHECK that it was found? It should never happen that it's not found, right?

[Philipp Keck, 2016/07/05 13:51:59]

Done.

+    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 (enabled_ == Enabled::DISABLE)
+    return;
+
+  for (ContentSuggestionCategory 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
+    // (maybe constant, at least consistent) ordering for the UI.
+    categories_.push_back(category);
+    if (provider->GetCategoryState(category) <=
+        ContentSuggestionsState::AVAILABLE) {

[Marc Treib, 2016/07/05 13:17:12]

No numeric comparisons on enums please - make a helper function and use a
switch.

[Philipp Keck, 2016/07/05 13:51:59]

Yep, I wanted a helper function but wasn't sure where to place it. Initially, it
was ContentSuggestionsProvider::IsCategoryAvailable(category), which called
::GetCategoryState(). But then I started needing the same logic in the service
and also in other places. It doesn't seem possible to place helper functions in
the enum class? In Java, "provider.GetCategoryState(category).isAvailable()"
would work...

[Marc Treib, 2016/07/05 14:50:50]

I'd suggest a public function, next to the enum definition.

[Philipp Keck, 2016/07/05 16:53:34]

Done.

+      suggestions_by_category_[category].clear();
+    }
+    provider->SetObserver(this);
+    FOR_EACH_OBSERVER(
+        Observer, observers_,
+        OnCategoryStateChanged(category, provider->GetCategoryState(category)));
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Private methods
+
+void ContentSuggestionsService::OnNewSuggestions(
+    ContentSuggestionCategory changed_category,
+    std::vector<ContentSuggestion> new_suggestions) {
+  DCHECK(CategoryExists(changed_category));
+
+  for (const ContentSuggestion& suggestion :
+       suggestions_by_category_[changed_category]) {
+    id_category_map_.erase(suggestion.id());
+  }
+
+  suggestions_by_category_[changed_category] = std::move(new_suggestions);
+
+  for (const ContentSuggestion& suggestion : new_suggestions) {
+    id_category_map_[suggestion.id()] = suggestion.category();
+  }
+
+  FOR_EACH_OBSERVER(Observer, observers_, OnNewSuggestions());
+}
+
+void ContentSuggestionsService::OnCategoryStateChanged(
+    ContentSuggestionCategory changed_category,
+    ContentSuggestionsState new_state) {
+  if (new_state <= ContentSuggestionsState::AVAILABLE) {

[Marc Treib, 2016/07/05 13:17:12]

Also here: Helper please

[Philipp Keck, 2016/07/05 13:51:59]

See above.

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

[Marc Treib, 2016/07/05 13:17:12]

Can it ever be not found?

[Philipp Keck, 2016/07/05 13:51:59]

It's a precaution for providers that irreponsibly keep their Observer instance
around even after they shut down (maybe even tied to some network callbacks or
sth like that). With good design, that shouldn't happen, so this check could
indeed be removed.

+      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_,
+        OnCategoryStateChanged(category, GetCategoryState(category)));
+  }
+}
+
+bool ContentSuggestionsService::CategoryExists(
+    ContentSuggestionCategory category) const {
+  return std::find(categories_.begin(), categories_.end(), category) !=
+         categories_.end();
+}
+
+}  // namespace ntp_snippets