Move OnURLsDeleted from NTPSnippetsService to 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 <set>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/gfx/image/image.h"
 
 namespace ntp_snippets {
 
-ContentSuggestionsService::ContentSuggestionsService(State state)
-    : state_(state) {}
+ContentSuggestionsService::ContentSuggestionsService(
+    State state,
+    history::HistoryService* history_service)
+    : state_(state), history_service_observer_(this) {
+  // Can be null in tests.
+  if (history_service)
+    history_service_observer_.Add(history_service);
+}
 
 ContentSuggestionsService::~ContentSuggestionsService() {}
 
 void ContentSuggestionsService::Shutdown() {
   ntp_snippets_service_ = nullptr;
   id_category_map_.clear();
   suggestions_by_category_.clear();
   providers_by_category_.clear();
   categories_.clear();
   providers_.clear();
@@ skipping 193 matching lines @@
 
 void ContentSuggestionsService::OnSuggestionInvalidated(
     ContentSuggestionsProvider* provider,
     Category category,
     const std::string& suggestion_id) {
   RemoveSuggestionByID(category, suggestion_id);
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnSuggestionInvalidated(category, suggestion_id));
 }
 
+// history::HistoryServiceObserver implementation.
+void ContentSuggestionsService::OnURLsDeleted(
+    history::HistoryService* history_service,
+    bool all_history,
+    bool expired,
+    const history::URLRows& deleted_rows,
+    const std::set<GURL>& favicon_urls) {
+  // We don't care about expired entries.
+  if (expired)
+    return;
+
+  // Redirect to ClearHistory().
+  base::Time begin;
+  base::Time end;
+  base::Callback<bool(const GURL& url)> filter;
+  auto return_true = [](const GURL& url) { return true; };
+  auto set_contains = [](const std::set<GURL>& set, const GURL& url) {

[Marc Treib, 2016/09/02 14:45:19]

Just inline these two definitions into the respective Binds below?
Or at least move their definitions closer to where they're used.

[vitaliii, 2016/09/02 16:40:06]

Done.

+    return set.count(url) != 0 ? true : false;

[Marc Treib, 2016/09/02 14:45:19]

remove the " ? true : false", it doesn't do anything

[vitaliii, 2016/09/02 16:40:06]

|count| returns |unsigned int|.

[Marc Treib, 2016/09/02 16:50:35]

Yes, you can keep the "!= 0" (that's nicer than the static_cast you have now),
just the arithmetic if after that was pointless.

[vitaliii, 2016/09/02 17:14:10]

Done.

+  };
+  if (all_history) {
+    begin = base::Time::UnixEpoch();
+    end = base::Time::Now();
+    filter = base::Bind(return_true);
+  } else {
+    if (deleted_rows.empty())
+      return;
+    else {

[Marc Treib, 2016/09/02 14:45:19]

No else after return.
Or, if you initialize begin/end to suitable values below, you might not need the
early exit at all.

[vitaliii, 2016/09/02 16:40:06]

Done.

+      begin = deleted_rows[0].last_visit();
+      end = deleted_rows[0].last_visit();
+      std::set<GURL> deleted_urls;
+      for (const history::URLRow& row : deleted_rows) {
+        if (row.last_visit() < begin)
+          begin = row.last_visit();
+        if (row.last_visit() > end)
+          end = row.last_visit();
+        deleted_urls.insert(row.url());
+      }
+      filter = base::Bind(set_contains, deleted_urls);
+    }
+  }
+
+  for (const auto& provider : providers_) {

[Marc Treib, 2016/09/02 14:45:19]

Just call this class's ClearHistory

[vitaliii, 2016/09/02 16:40:06]

Done.

+    provider->ClearHistory(begin, end, filter);
+  }
+}
+
+void ContentSuggestionsService::HistoryServiceBeingDeleted(
+    history::HistoryService* history_service) {
+  history_service_observer_.RemoveAll();
+}
+
 bool ContentSuggestionsService::RegisterCategoryIfRequired(
     ContentSuggestionsProvider* provider,
     Category category) {
   auto it = providers_by_category_.find(category);
   if (it != providers_by_category_.end()) {
     DCHECK_EQ(it->second, provider);
     return false;
   }
 
   providers_by_category_[category] = provider;
@@ skipping 49 matching lines @@
           if (left.IsKnownCategory(KnownCategories::BOOKMARKS))
             return false;
           if (right.IsKnownCategory(KnownCategories::BOOKMARKS))
             return true;
         }
         return category_factory_.CompareCategories(left, right);
       });
 }
 
 }  // namespace ntp_snippets