[NTP::PhysicalWeb] Implement suggestion dismissal.

components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc

Index: components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
diff --git a/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
index 93179bd19c533fa3fd238a4f18df88bd9cd228e0..118b837a76a35a15d1ce9218f9187fb2db990af8 100644
--- a/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
+++ b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 
@@ -14,6 +15,10 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "components/ntp_snippets/pref_names.h"
+#include "components/ntp_snippets/pref_util.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image.h"
@@ -43,12 +48,14 @@ std::string GetPageId(const DictionaryValue& page_dictionary) {
 PhysicalWebPageSuggestionsProvider::PhysicalWebPageSuggestionsProvider(
     ContentSuggestionsProvider::Observer* observer,
     CategoryFactory* category_factory,
-    physical_web::PhysicalWebDataSource* physical_web_data_source)
+    physical_web::PhysicalWebDataSource* physical_web_data_source,
+    PrefService* pref_service)
     : ContentSuggestionsProvider(observer, category_factory),
       category_status_(CategoryStatus::AVAILABLE),
       provided_category_(category_factory->FromKnownCategory(
           KnownCategories::PHYSICAL_WEB_PAGES)),
-      physical_web_data_source_(physical_web_data_source) {
+      physical_web_data_source_(physical_web_data_source),
+      pref_service_(pref_service) {
   observer->OnCategoryStatusChanged(this, provided_category_, category_status_);
   physical_web_data_source_->RegisterListener(this);
   // TODO(vitaliii): Rewrite initial fetch once crbug.com/667754 is resolved.
@@ -80,8 +87,10 @@ CategoryInfo PhysicalWebPageSuggestionsProvider::GetCategoryInfo(
 
 void PhysicalWebPageSuggestionsProvider::DismissSuggestion(
     const ContentSuggestion::ID& suggestion_id) {
-  // TODO(vitaliii): Implement this and then
-  // ClearDismissedSuggestionsForDebugging. (crbug.com/667766)
+  DCHECK_EQ(provided_category_, suggestion_id.category());
+  std::set<std::string> dismissed_ids = ReadDismissedIDsFromPrefs();
+  dismissed_ids.insert(suggestion_id.id_within_category());
+  StoreDismissedIDsToPrefs(dismissed_ids);
 }
 
 void PhysicalWebPageSuggestionsProvider::FetchSuggestionImage(
@@ -119,13 +128,37 @@ void PhysicalWebPageSuggestionsProvider::ClearCachedSuggestions(
 void PhysicalWebPageSuggestionsProvider::GetDismissedSuggestionsForDebugging(
     Category category,
     const DismissedSuggestionsCallback& callback) {
-  // Not implemented.
-  callback.Run(std::vector<ContentSuggestion>());
+  DCHECK_EQ(provided_category_, category);
+  std::unique_ptr<ListValue> page_values =
+      physical_web_data_source_->GetMetadata();
+  const std::set<std::string> dismissed_ids = ReadDismissedIDsFromPrefs();
+  std::vector<ContentSuggestion> suggestions;
+  for (const std::unique_ptr<Value>& page_value : *page_values) {
+    const DictionaryValue* page_dictionary;
+    if (!page_value->GetAsDictionary(&page_dictionary)) {
+      LOG(DFATAL) << "Physical Web page is not a dictionary.";
+      continue;
+    }
+
+    if (dismissed_ids.count(GetPageId(*page_dictionary))) {
+      suggestions.push_back(ConvertPhysicalWebPage(*page_dictionary));
+    }
+  }
+
+  callback.Run(std::move(suggestions));
 }
 
 void PhysicalWebPageSuggestionsProvider::ClearDismissedSuggestionsForDebugging(
     Category category) {
-  // TODO(vitaliii): Implement when dismissed suggestions are supported.
+  DCHECK_EQ(provided_category_, category);
+  StoreDismissedIDsToPrefs(std::set<std::string>());
+  FetchPhysicalWebPages();
+}
+
+// static
+void PhysicalWebPageSuggestionsProvider::RegisterProfilePrefs(
+    PrefRegistrySimple* registry) {
+  registry->RegisterListPref(prefs::kDismissedPhysicalWebPageSuggestions);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -155,6 +188,8 @@ PhysicalWebPageSuggestionsProvider::GetMostRecentPhysicalWebPagesWithFilter(
   std::unique_ptr<ListValue> page_values =
       physical_web_data_source_->GetMetadata();
 
+  const std::set<std::string> old_dismissed_ids = ReadDismissedIDsFromPrefs();

[jkrcal, 2016/12/07 14:20:01]

nit: Please add a comment like "Filter out dismissed suggestions (and also prune
dismissed suggestions that are not physically around; needed if we miss some
OnLost() event)."

[vitaliii, 2016/12/08 07:13:41]

Done.

+  std::set<std::string> new_dismissed_ids;
   std::vector<const DictionaryValue*> page_dictionaries;
   for (const std::unique_ptr<Value>& page_value : *page_values) {
     const DictionaryValue* page_dictionary;
@@ -162,9 +197,19 @@ PhysicalWebPageSuggestionsProvider::GetMostRecentPhysicalWebPagesWithFilter(
       LOG(DFATAL) << "Physical Web page is not a dictionary.";
       continue;
     }
-    if (!excluded_ids.count(GetPageId(*page_dictionary))) {
+
+    const std::string page_id = GetPageId(*page_dictionary);
+    if (!excluded_ids.count(page_id) && !old_dismissed_ids.count(page_id)) {
       page_dictionaries.push_back(page_dictionary);
     }
+
+    if (old_dismissed_ids.count(page_id)) {
+      new_dismissed_ids.insert(page_id);
+    }
+  }
+
+  if (old_dismissed_ids.size() != new_dismissed_ids.size()) {
+    StoreDismissedIDsToPrefs(new_dismissed_ids);
   }
 
   std::sort(page_dictionaries.begin(), page_dictionaries.end(),
@@ -229,8 +274,7 @@ void PhysicalWebPageSuggestionsProvider::OnFound(const std::string& url) {
 }
 
 void PhysicalWebPageSuggestionsProvider::OnLost(const std::string& url) {
-  // TODO(vitaliii): Do not refetch, but just update the current state.
-  FetchPhysicalWebPages();
+  InvalidateSuggestion(url);
 }
 
 void PhysicalWebPageSuggestionsProvider::OnDistanceChanged(
@@ -239,4 +283,30 @@ void PhysicalWebPageSuggestionsProvider::OnDistanceChanged(
   FetchPhysicalWebPages();
 }
 
+void PhysicalWebPageSuggestionsProvider::InvalidateSuggestion(
+    const std::string& page_id) {
+  observer()->OnSuggestionInvalidated(
+      this, ContentSuggestion::ID(provided_category_, page_id));
+
+  std::set<std::string> dismissed_ids = ReadDismissedIDsFromPrefs();

[jkrcal, 2016/12/07 14:20:01]

nit: Could you add a comment saying "Remove page_id from dismissed suggestions
if it is there."?

[vitaliii, 2016/12/08 07:13:41]

Done.

+  auto it = dismissed_ids.find(page_id);
+  if (it != dismissed_ids.end()) {
+    dismissed_ids.erase(it);
+    StoreDismissedIDsToPrefs(dismissed_ids);
+  }
+}
+
+std::set<std::string>
+PhysicalWebPageSuggestionsProvider::ReadDismissedIDsFromPrefs() const {
+  return prefs::ReadDismissedIDsFromPrefs(
+      *pref_service_, prefs::kDismissedPhysicalWebPageSuggestions);
+}
+
+void PhysicalWebPageSuggestionsProvider::StoreDismissedIDsToPrefs(
+    const std::set<std::string>& dismissed_ids) {
+  prefs::StoreDismissedIDsToPrefs(pref_service_,
+                                  prefs::kDismissedPhysicalWebPageSuggestions,
+                                  dismissed_ids);
+}
+
 }  // namespace ntp_snippets