[NTPSnippets] Show all downloads on the NTP (3/3): Downloads provider.

chrome/browser/ntp_snippets/download_suggestions_provider.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 "chrome/browser/ntp_snippets/download_suggestions_provider.h"
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/feature_list.h"
+#include "base/guid.h"
+#include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
+#include "components/ntp_snippets/pref_names.h"
+#include "components/ntp_snippets/pref_util.h"
+#include "components/offline_pages/client_namespace_constants.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "grit/components_strings.h"
+#include "net/base/filename_util.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/gfx/image/image.h"
+
+using content::DownloadItem;
+using content::DownloadManager;
+using ntp_snippets::Category;
+using ntp_snippets::CategoryInfo;
+using ntp_snippets::CategoryStatus;
+using ntp_snippets::ContentSuggestion;
+using ntp_snippets::prefs::kDismissedAssetDownloadSuggestions;
+using ntp_snippets::prefs::kDismissedOfflinePageDownloadSuggestions;
+using offline_pages::OfflinePageItem;
+
+namespace {
+
+// TODO(vitaliii): Make this configurable via a variation param. See
+// crbug.com/654800.
+const int kMaxSuggestionsCount = 5;
+const char kAssetDownloadsPrefix = 'D';
+const char kOfflinePageDownloadsPrefix = 'O';
+
+std::string GetOfflinePagePerCategoryID(int64_t raw_offline_page_id) {
+  // Raw ID is prefixed in order to avoid conflicts with asset downloads.
+  return std::string(1, kOfflinePageDownloadsPrefix) +
+         base::IntToString(raw_offline_page_id);
+}
+
+std::string GetAssetDownloadPerCategoryID(uint32_t raw_download_id) {
+  // Raw ID is prefixed in order to avoid conflicts with offline page downloads.
+  return std::string(1, kAssetDownloadsPrefix) +
+         base::UintToString(raw_download_id);
+}
+
+// Determines whether |suggestion_id| corresponds to offline page suggestion or
+// asset download based on |id_within_category| prefix.
+bool CorrespondsToOfflinePage(const ContentSuggestion::ID& suggestion_id) {
+  const std::string& id_within_category = suggestion_id.id_within_category();
+  if (!id_within_category.empty()) {
+    if (id_within_category[0] == kOfflinePageDownloadsPrefix)
+      return true;
+    if (id_within_category[0] == kAssetDownloadsPrefix)
+      return false;
+  }
+  NOTREACHED() << "Unknown id_within_category " << id_within_category;
+  return false;
+}
+
+bool IsOfflinePageDownload(const offline_pages::ClientId& client_id) {
+  DCHECK((client_id.name_space == offline_pages::kAsyncNamespace ||

[Marc Treib, 2016/10/13 12:11:25]

You can DCHECK the IsValidGUID, but not the namespace part.

[vitaliii, 2016/10/15 18:36:30]

Actually I DCHECK'ed on the device, so no need for it here.

[Marc Treib, 2016/10/17 10:18:40]

"on the device"? (Not DCHECKing here is fine, I just don't understand your
comment)

[vitaliii, 2016/10/26 00:07:54]

I meant that I run the app on the device while having the DCHECK and it did not
fire.

+          client_id.name_space == offline_pages::kDownloadNamespace ||
+          client_id.name_space == offline_pages::kNTPSuggestionsNamespace) &&
+         base::IsValidGUID(client_id.id));
+
+  return (client_id.name_space == offline_pages::kAsyncNamespace ||
+          client_id.name_space == offline_pages::kDownloadNamespace ||
+          client_id.name_space == offline_pages::kNTPSuggestionsNamespace);
+}
+
+bool IsDownloadCompleted(const DownloadItem& item) {
+  return item.GetState() == DownloadItem::DownloadState::COMPLETE &&
+         !item.GetFileExternallyRemoved();
+}
+
+struct OrderDownloadsMostRecentlyDownloadedFirst {
+  bool operator()(const DownloadItem* left, const DownloadItem* right) const {
+    return left->GetEndTime() > right->GetEndTime();
+  }
+};
+
+struct SuggestionItemWrapper {

[Marc Treib, 2016/10/13 12:11:25]

Not used anymore :)

[vitaliii, 2016/10/15 18:36:30]

Done.

+  base::Time time;
+  bool is_offline_page;
+  int index;
+  bool operator<(const SuggestionItemWrapper& other) const {
+    return time > other.time;
+  }
+};
+
+}  // namespace
+
+DownloadSuggestionsProvider::DownloadSuggestionsProvider(
+    ContentSuggestionsProvider::Observer* observer,
+    ntp_snippets::CategoryFactory* category_factory,
+    const scoped_refptr<ntp_snippets::OfflinePageProxy>& offline_page_proxy,
+    content::DownloadManager* download_manager,
+    PrefService* pref_service,
+    bool download_manager_ui_enabled)
+    : ContentSuggestionsProvider(observer, category_factory),
+      category_status_(CategoryStatus::AVAILABLE_LOADING),
+      provided_category_(category_factory->FromKnownCategory(
+          ntp_snippets::KnownCategories::DOWNLOADS)),
+      offline_page_proxy_(offline_page_proxy),
+      download_manager_notifier_(download_manager, this),
+      pref_service_(pref_service),
+      download_manager_ui_enabled_(download_manager_ui_enabled),
+      weak_ptr_factory_(this) {
+  observer->OnCategoryStatusChanged(this, provided_category_, category_status_);
+  offline_page_proxy_->AddObserver(this);
+  AsyncronouslyFetchAllDownloadsAndSubmitSuggestions();
+}
+
+DownloadSuggestionsProvider::~DownloadSuggestionsProvider() {
+  offline_page_proxy_->RemoveObserver(this);
+}
+
+CategoryStatus DownloadSuggestionsProvider::GetCategoryStatus(
+    Category category) {
+  DCHECK_EQ(provided_category_, category);
+  return category_status_;
+}
+
+CategoryInfo DownloadSuggestionsProvider::GetCategoryInfo(Category category) {
+  DCHECK_EQ(provided_category_, category);
+  return CategoryInfo(
+      l10n_util::GetStringUTF16(IDS_NTP_DOWNLOAD_SUGGESTIONS_SECTION_HEADER),
+      ntp_snippets::ContentSuggestionsCardLayout::MINIMAL_CARD,
+      /*has_more_button=*/download_manager_ui_enabled_,
+      /*show_if_empty=*/false);
+}
+
+void DownloadSuggestionsProvider::DismissSuggestion(
+    const ContentSuggestion::ID& suggestion_id) {
+  DCHECK_EQ(provided_category_, suggestion_id.category());
+  std::set<std::string> dismissed_ids =
+      ReadDismissedIDsFromPrefs(CorrespondsToOfflinePage(suggestion_id));
+  dismissed_ids.insert(suggestion_id.id_within_category());
+  StoreDismissedIDsToPrefs(CorrespondsToOfflinePage(suggestion_id),
+                           dismissed_ids);
+
+  RemoveSuggestionFromCacheAndRetrieveMoreIfNeeded(suggestion_id);
+}
+
+void DownloadSuggestionsProvider::FetchSuggestionImage(
+    const ContentSuggestion::ID& suggestion_id,
+    const ImageFetchedCallback& callback) {
+  // TODO(vitaliii): Fetch proper thumbnail from OfflinePageModel once it is
+  // available there.
+  // TODO(vitaliii): Provide site's favicon for assets downloads. See
+  // crbug.com/631447.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(callback, gfx::Image()));
+}
+
+void DownloadSuggestionsProvider::ClearHistory(
+    base::Time begin,
+    base::Time end,
+    const base::Callback<bool(const GURL& url)>& filter) {
+  ClearDismissedSuggestionsForDebugging(provided_category_);
+}
+
+void DownloadSuggestionsProvider::ClearCachedSuggestions(Category category) {
+  DCHECK_EQ(provided_category_, category);
+  // Ignored.

[Marc Treib, 2016/10/13 12:11:26]

Maybe a comment on why it's safe to ignore?

[vitaliii, 2016/10/15 18:36:30]

Done.

+}
+
+void DownloadSuggestionsProvider::GetDismissedSuggestionsForDebugging(
+    Category category,
+    const DismissedSuggestionsCallback& callback) {
+  DCHECK_EQ(provided_category_, category);
+
+  offline_page_proxy_->GetAllPages(
+      base::Bind(&DownloadSuggestionsProvider::
+                     GetAllPagesCallbackForGetDismissedSuggestions,
+                 weak_ptr_factory_.GetWeakPtr(), callback));
+}
+
+void DownloadSuggestionsProvider::ClearDismissedSuggestionsForDebugging(
+    Category category) {
+  DCHECK_EQ(provided_category_, category);
+  StoreAssetDismissedIDsToPrefs(std::set<std::string>());
+  StoreOfflinePageDismissedIDsToPrefs(std::set<std::string>());
+  AsyncronouslyFetchAllDownloadsAndSubmitSuggestions();
+}
+
+// static
+void DownloadSuggestionsProvider::RegisterProfilePrefs(
+    PrefRegistrySimple* registry) {
+  registry->RegisterListPref(kDismissedAssetDownloadSuggestions);
+  registry->RegisterListPref(kDismissedOfflinePageDownloadSuggestions);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Private methods
+
+void DownloadSuggestionsProvider::GetAllPagesCallbackForGetDismissedSuggestions(
+    const DismissedSuggestionsCallback& callback,
+    const std::vector<OfflinePageItem>& offline_pages) const {
+  std::set<std::string> dismissed_ids = ReadOfflinePageDismissedIDsFromPrefs();
+  std::vector<ContentSuggestion> suggestions;
+  for (const OfflinePageItem& item : offline_pages) {
+    if (dismissed_ids.count(GetOfflinePagePerCategoryID(item.offline_id)))
+      suggestions.push_back(ConvertOfflinePage(item));
+  }
+
+  DownloadManager* manager = download_manager_notifier_.GetManager();
+  if (manager) {
+    std::vector<DownloadItem*> all_downloads;
+    manager->GetAllDownloads(&all_downloads);
+
+    dismissed_ids = ReadAssetDismissedIDsFromPrefs();
+
+    for (const DownloadItem* item : all_downloads) {
+      if (dismissed_ids.count(GetAssetDownloadPerCategoryID(item->GetId())))
+        suggestions.push_back(ConvertDownloadItem(*item));
+    }
+  }
+
+  callback.Run(std::move(suggestions));
+}
+
+void DownloadSuggestionsProvider::OfflinePageModelChanged(
+    const std::vector<offline_pages::OfflinePageItem>& offline_pages) {
+  NotifyStatusChanged(CategoryStatus::AVAILABLE);

[Marc Treib, 2016/10/13 12:11:25]

Not required, SubmitContentSuggestions will do that (synchronously!)

[vitaliii, 2016/10/15 18:36:30]

Done.

+  ProcessAllOfflinePages(offline_pages);
+  SubmitContentSuggestions();
+}
+
+void DownloadSuggestionsProvider::OfflinePageDeleted(
+    int64_t offline_id,
+    const offline_pages::ClientId& client_id) {
+  if (IsOfflinePageDownload(client_id)) {
+    InvalidateSuggestion(GetOfflinePagePerCategoryID(offline_id));
+  }
+}
+
+void DownloadSuggestionsProvider::OnDownloadCreated(DownloadManager* manager,
+                                                    DownloadItem* item) {
+  // This is called when new downloads are started and on startup for existing
+  // ones.

[Marc Treib, 2016/10/13 12:11:25]

Hm, so on startup we'll notify our observer once per download? Not too great,
but probably nothing we can do about it.

[vitaliii, 2016/10/15 18:36:30]

Yeap unless DownloadManager changes their Observer interface (e.g. by adding
OnStartupCompleted).

+  if (CacheAssetDownloadIfNeeded(item))
+    SubmitContentSuggestions();
+}
+
+void DownloadSuggestionsProvider::OnDownloadUpdated(DownloadManager* manager,
+                                                    DownloadItem* item) {
+  // Unfinished downloads may become completed, therefore, this call cannot be
+  // ignored.
+  if (CacheAssetDownloadIfNeeded(item))
+    SubmitContentSuggestions();
+}
+
+void DownloadSuggestionsProvider::OnDownloadOpened(DownloadManager* manager,
+                                                   DownloadItem* item) {
+  // Ignored.
+}
+
+void DownloadSuggestionsProvider::OnDownloadRemoved(DownloadManager* manager,
+                                                    DownloadItem* item) {
+  if (!IsDownloadCompleted(*item))
+    return;
+  // TODO(vitaliii): Implement a better way to clean up dismissed IDs (in case
+  // some calls are missed).
+  InvalidateSuggestion(GetAssetDownloadPerCategoryID(item->GetId()));
+}
+
+void DownloadSuggestionsProvider::NotifyStatusChanged(
+    CategoryStatus new_status) {
+  DCHECK_NE(CategoryStatus::NOT_PROVIDED, category_status_);
+  DCHECK_NE(CategoryStatus::NOT_PROVIDED, new_status);
+  if (category_status_ == new_status)
+    return;
+  category_status_ = new_status;
+  observer()->OnCategoryStatusChanged(this, provided_category_,
+                                      category_status_);
+}
+
+void DownloadSuggestionsProvider::AsyncronouslyFetchOfflinePagesDownloads() {
+  offline_page_proxy_->GetAllPages(
+      base::Bind(&DownloadSuggestionsProvider::ProcessAllOfflinePages,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void DownloadSuggestionsProvider::
+    AsyncronouslyFetchOfflinePagesDownloadsAndSubmitSuggestions() {

[Marc Treib, 2016/10/13 12:11:25]

Add a "bool notify" parameter and merge with
AsyncronouslyFetchOfflinePagesDownloads? Then also add that parameter to
ProcessAllOfflinePages, then you can just forward it there.

[vitaliii, 2016/10/15 18:36:30]

Done.

+  offline_page_proxy_->GetAllPages(
+      base::Bind(&DownloadSuggestionsProvider::OfflinePageModelChanged,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void DownloadSuggestionsProvider::SyncronouslyFetchAssetsDownloads() {
+  DownloadManager* manager = download_manager_notifier_.GetManager();
+  if (!manager) {
+    // The manager has gone down.
+    return;
+  }
+
+  std::vector<DownloadItem*> all_downloads;
+  manager->GetAllDownloads(&all_downloads);
+
+  std::set<std::string> old_dismissed_ids = ReadAssetDismissedIDsFromPrefs();
+  std::set<std::string> retained_dismissed_ids;
+  std::vector<const DownloadItem*> downloads;
+  for (const DownloadItem* item : all_downloads) {
+    std::string within_category_id =
+        GetAssetDownloadPerCategoryID(item->GetId());
+    if (!old_dismissed_ids.count(within_category_id)) {
+      if (IsDownloadCompleted(*item))
+        downloads.push_back(item);
+    } else {
+      retained_dismissed_ids.insert(within_category_id);
+    }
+  }
+
+  if (old_dismissed_ids.size() != retained_dismissed_ids.size()) {
+    StoreAssetDismissedIDsToPrefs(retained_dismissed_ids);
+  }
+
+  if (static_cast<int>(downloads.size()) > kMaxSuggestionsCount) {
+    // Partially sorts |downloads| such that:
+    // 1) The element at the index |kMaxSuggestionsCount| is changed to the
+    //    element which would occur on this position if |downloads| was sorted;
+    // 2) All of the elements before index |kMaxSuggestionsCount| are less than
+    //    or equal to the elements after it.
+    std::nth_element(downloads.begin(),
+                     downloads.begin() + kMaxSuggestionsCount, downloads.end(),
+                     OrderDownloadsMostRecentlyDownloadedFirst());
+    downloads.resize(kMaxSuggestionsCount);
+  }
+
+  cached_asset_downloads_ = std::move(downloads);

[Marc Treib, 2016/10/13 12:11:25]

You could put the items directly into cached_asset_downloads_ and remove the
downloads variable.

[vitaliii, 2016/10/15 18:36:30]

Done.

+}
+
+void DownloadSuggestionsProvider::
+    AsyncronouslyFetchAllDownloadsAndSubmitSuggestions() {
+  SyncronouslyFetchAssetsDownloads();
+  AsyncronouslyFetchOfflinePagesDownloadsAndSubmitSuggestions();
+}
+
+void DownloadSuggestionsProvider::SubmitContentSuggestions() {
+  NotifyStatusChanged(CategoryStatus::AVAILABLE);
+
+  std::vector<ContentSuggestion> suggestions;
+  for (const OfflinePageItem& item : cached_offline_page_downloads_)
+    suggestions.push_back(ConvertOfflinePage(item));
+
+  for (const DownloadItem* item : cached_asset_downloads_)
+    suggestions.push_back(ConvertDownloadItem(*item));
+
+  std::sort(suggestions.begin(), suggestions.end(),
+            [](const ContentSuggestion& left, const ContentSuggestion& right) {
+              return left.publish_date() > right.publish_date();
+            });
+
+  // |resize()| cannot be used here (does not compile), because
+  // |ContentSuggestion|'s move constructor is not marked |noexcept|. |resize()|
+  // provides a strong guarantee that it either succeeds or the state of the
+  // vector is left unchanged, therefore, the only option left is to copy, but
+  // |ContentSuggestion| is not copyable either.
+  // On the contrary, |pop_back()| does not reallocate any memory.
+  while (suggestions.size() > kMaxSuggestionsCount)
+    suggestions.pop_back();

[Marc Treib, 2016/10/13 12:11:25]

I think .erase would also work, something like
suggestions.erase(suggestions.begin() + kMaxSuggestionsCount,
suggestions.end());
(but it's not any prettier, so.. eh)

[vitaliii, 2016/10/15 18:36:30]

Acknowledged.

+
+  observer()->OnNewSuggestions(this, provided_category_,
+                               std::move(suggestions));
+}
+
+ContentSuggestion DownloadSuggestionsProvider::ConvertOfflinePage(
+    const OfflinePageItem& offline_page) const {
+  // TODO(vitaliii): Make sure the URL is actually opened as an offline URL even
+  // when the user is online. See crbug.com/641568.
+  ContentSuggestion suggestion(
+      ContentSuggestion::ID(provided_category_, GetOfflinePagePerCategoryID(
+                                                    offline_page.offline_id)),
+      offline_page.url);
+
+  if (offline_page.title.empty()) {
+    // TODO(vitaliii): Remove this fallback once the OfflinePageModel provides
+    // titles for all (relevant) OfflinePageItems.
+    suggestion.set_title(base::UTF8ToUTF16(offline_page.url.spec()));
+  } else {
+    suggestion.set_title(offline_page.title);
+  }
+  suggestion.set_publish_date(offline_page.creation_time);
+  suggestion.set_publisher_name(base::UTF8ToUTF16(offline_page.url.host()));
+  return suggestion;
+}
+
+ContentSuggestion DownloadSuggestionsProvider::ConvertDownloadItem(
+    const DownloadItem& download_item) const {
+  // TODO(vitaliii): Ensure that files are opened in browser, but not downloaded
+  // again. See crbug.com/641568.
+  ContentSuggestion suggestion(
+      ContentSuggestion::ID(provided_category_, GetAssetDownloadPerCategoryID(
+                                                    download_item.GetId())),
+      net::FilePathToFileURL(download_item.GetTargetFilePath()));
+  // TODO(vitaliii): Set proper title.
+  DCHECK(
+      base::IsStringUTF8(download_item.GetTargetFilePath().BaseName().value()));
+  suggestion.set_title(
+      base::UTF8ToUTF16(download_item.GetTargetFilePath().BaseName().value()));
+  suggestion.set_publish_date(download_item.GetEndTime());
+  suggestion.set_publisher_name(
+      base::UTF8ToUTF16(download_item.GetURL().host()));
+  // TODO(vitaliii): Set suggestion icon.
+  return suggestion;
+}
+
+bool DownloadSuggestionsProvider::CacheAssetDownloadIfNeeded(
+    const content::DownloadItem* item) {
+  if (!IsDownloadCompleted(*item))
+    return false;
+
+  if (base::ContainsValue(cached_asset_downloads_, item))
+    return false;

[Marc Treib, 2016/10/13 12:11:25]

Hm, is this always safe? Could the download have changed in the meantime
(updated title or whatever)?

[vitaliii, 2016/10/15 18:36:30]

Good point. I leave this check here, because the function is used in
|OnDownloadCreated|, but I changed |OnDownloadUpdated|. As far as I saw in
DownloadItem's code, something we show should not change, but just in case I
submit suggestions if an item from cache has been updated.
Also I remove an item if its file gets removed. Thanks!

+
+  std::set<std::string> dismissed_ids = ReadAssetDismissedIDsFromPrefs();
+  if (dismissed_ids.count(GetAssetDownloadPerCategoryID(item->GetId())))
+    return false;
+
+  DCHECK_LE(static_cast<int>(cached_asset_downloads_.size()),
+            kMaxSuggestionsCount);
+  if (cached_asset_downloads_.size() == kMaxSuggestionsCount) {
+    auto oldest = std::max_element(cached_asset_downloads_.begin(),
+                                   cached_asset_downloads_.end(),
+                                   OrderDownloadsMostRecentlyDownloadedFirst());
+    if (item->GetEndTime() <= (*oldest)->GetEndTime())
+      return false;
+
+    *oldest = item;
+  } else {
+    cached_asset_downloads_.push_back(item);
+  }
+
+  return true;
+}
+
+bool DownloadSuggestionsProvider::RemoveSuggestionFromCacheIfPresent(
+    const ContentSuggestion::ID& suggestion_id) {
+  DCHECK_EQ(provided_category_, suggestion_id.category());
+  if (CorrespondsToOfflinePage(suggestion_id)) {
+    auto matching =
+        std::find_if(cached_offline_page_downloads_.begin(),
+                     cached_offline_page_downloads_.end(),
+                     [&suggestion_id](const OfflinePageItem& item) {
+                       return GetOfflinePagePerCategoryID(item.offline_id) ==
+                              suggestion_id.id_within_category();
+                     });
+    if (matching != cached_offline_page_downloads_.end()) {
+      cached_offline_page_downloads_.erase(matching);
+      return true;
+    }
+    return false;
+  }
+
+  auto matching = std::find_if(
+      cached_asset_downloads_.begin(), cached_asset_downloads_.end(),
+      [&suggestion_id](const DownloadItem* item) {
+        return GetAssetDownloadPerCategoryID(item->GetId()) ==
+               suggestion_id.id_within_category();
+      });
+  if (matching != cached_asset_downloads_.end()) {
+    cached_asset_downloads_.erase(matching);
+    return true;
+  }
+  return false;
+}
+
+void DownloadSuggestionsProvider::
+    RemoveSuggestionFromCacheAndRetrieveMoreIfNeeded(
+        const ContentSuggestion::ID& suggestion_id) {
+  DCHECK_EQ(provided_category_, suggestion_id.category());
+  if (!RemoveSuggestionFromCacheIfPresent(suggestion_id))
+    return;
+
+  if (CorrespondsToOfflinePage(suggestion_id)) {
+    if (cached_offline_page_downloads_.size() == kMaxSuggestionsCount - 1) {
+      // Previously there were |kMaxSuggestionsCount| cached suggestion,
+      // therefore, overall there may be more than |kMaxSuggestionsCount|
+      // suggestions in the model and now one of them may be cached instead of
+      // the removed one. Even though, the suggestions are not immediately
+      // used the cache has to be kept up to date, because it may be used when
+      // other data source is updated.
+      AsyncronouslyFetchOfflinePagesDownloads();
+    }
+  } else {
+    if (cached_asset_downloads_.size() == kMaxSuggestionsCount - 1) {
+      // The same as the case above.
+      SyncronouslyFetchAssetsDownloads();
+    }
+  }
+}
+
+void DownloadSuggestionsProvider::ProcessAllOfflinePages(
+    const std::vector<offline_pages::OfflinePageItem>& all_offline_pages) {
+  std::set<std::string> old_dismissed_ids =
+      ReadOfflinePageDismissedIDsFromPrefs();
+  std::set<std::string> retained_dismissed_ids;
+  std::vector<const OfflinePageItem*> items;
+  // Filtering out dismissed items and pruning dismissed IDs.
+  for (const OfflinePageItem& item : all_offline_pages) {
+    if (!IsOfflinePageDownload(item.client_id))
+      continue;
+
+    std::string per_category_id = GetOfflinePagePerCategoryID(item.offline_id);
+    if (!old_dismissed_ids.count(per_category_id))
+      items.push_back(&item);
+    else
+      retained_dismissed_ids.insert(per_category_id);
+  }
+
+  if (static_cast<int>(items.size()) > kMaxSuggestionsCount) {
+    // Partially sorts |items| such that:
+    // 1) The element at the index |kMaxSuggestionsCount| is changed to the
+    //    element which would occur on this position if |items| was sorted;
+    // 2) All of the elements before index |kMaxSuggestionsCount| are less than
+    //    or equal to the elements after it.
+    std::nth_element(
+        items.begin(), items.begin() + kMaxSuggestionsCount, items.end(),
+        [](const OfflinePageItem* left, const OfflinePageItem* right) {
+          return left->creation_time > right->creation_time;
+        });
+    items.resize(kMaxSuggestionsCount);
+  }
+
+  cached_offline_page_downloads_.clear();
+  for (const OfflinePageItem* item : items) {
+    cached_offline_page_downloads_.push_back(*item);
+  }
+
+  if (old_dismissed_ids.size() != retained_dismissed_ids.size()) {
+    StoreOfflinePageDismissedIDsToPrefs(retained_dismissed_ids);
+  }
+}
+
+void DownloadSuggestionsProvider::InvalidateSuggestion(
+    const std::string& per_category_id) {

[Marc Treib, 2016/10/13 12:11:25]

s/per_category_id/id_in_category/ to match the header (or in any case, use the
same term everywhere - ContentSuggestion::ID itself uses id_within_category,
probably that'd be best)

[vitaliii, 2016/10/15 18:36:30]

Done.

+  ContentSuggestion::ID suggestion_id(provided_category_, per_category_id);
+  observer()->OnSuggestionInvalidated(this, suggestion_id);
+
+  std::set<std::string> dismissed_ids =
+      ReadDismissedIDsFromPrefs(CorrespondsToOfflinePage(suggestion_id));
+  auto it = dismissed_ids.find(per_category_id);
+  if (it != dismissed_ids.end()) {
+    dismissed_ids.erase(it);
+    StoreDismissedIDsToPrefs(CorrespondsToOfflinePage(suggestion_id),
+                             dismissed_ids);
+  }
+
+  RemoveSuggestionFromCacheAndRetrieveMoreIfNeeded(suggestion_id);
+}
+
+std::set<std::string>
+DownloadSuggestionsProvider::ReadAssetDismissedIDsFromPrefs() const {
+  return ntp_snippets::prefs::ReadDismissedIDsFromPrefs(
+      *pref_service_, kDismissedAssetDownloadSuggestions);
+}
+
+void DownloadSuggestionsProvider::StoreAssetDismissedIDsToPrefs(
+    const std::set<std::string>& dismissed_ids) {
+  DCHECK(std::all_of(
+      dismissed_ids.begin(), dismissed_ids.end(),
+      [](const std::string& id) { return id[0] == kAssetDownloadsPrefix; }));
+  ntp_snippets::prefs::StoreDismissedIDsToPrefs(
+      pref_service_, kDismissedAssetDownloadSuggestions, dismissed_ids);
+}
+
+std::set<std::string>
+DownloadSuggestionsProvider::ReadOfflinePageDismissedIDsFromPrefs() const {
+  return ntp_snippets::prefs::ReadDismissedIDsFromPrefs(
+      *pref_service_, kDismissedOfflinePageDownloadSuggestions);
+}
+
+void DownloadSuggestionsProvider::StoreOfflinePageDismissedIDsToPrefs(
+    const std::set<std::string>& dismissed_ids) {
+  DCHECK(std::all_of(dismissed_ids.begin(), dismissed_ids.end(),
+                     [](const std::string& id) {
+                       return id[0] == kOfflinePageDownloadsPrefix;
+                     }));
+  ntp_snippets::prefs::StoreDismissedIDsToPrefs(
+      pref_service_, kDismissedOfflinePageDownloadSuggestions, dismissed_ids);
+}
+
+std::set<std::string> DownloadSuggestionsProvider::ReadDismissedIDsFromPrefs(
+    bool for_offline_page_downloads) const {
+  if (for_offline_page_downloads) {
+    return ReadOfflinePageDismissedIDsFromPrefs();
+  }
+  return ReadAssetDismissedIDsFromPrefs();
+}
+
+void DownloadSuggestionsProvider::StoreDismissedIDsToPrefs(
+    bool for_offline_page_downloads,
+    const std::set<std::string>& dismissed_ids) {
+  if (for_offline_page_downloads)
+    StoreOfflinePageDismissedIDsToPrefs(dismissed_ids);
+  else
+    StoreAssetDismissedIDsToPrefs(dismissed_ids);
+}