[NTP::Downloads] Fetch assets once the manager is loaded.

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 <utility>
 
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/guid.h"
 #include "base/memory/ptr_util.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 "base/values.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/ntp_snippets/features.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/pref_util.h"
 #include "components/offline_pages/core/offline_page_model_query.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/variations/variations_associated_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image.h"
 
-using base::ContainsValue;
 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;
 using offline_pages::OfflinePageModelQuery;
 using offline_pages::OfflinePageModelQueryBuilder;
 
 namespace {
 
 const int kDefaultMaxSuggestionsCount = 5;
 const char kAssetDownloadsPrefix = 'D';
 const char kOfflinePageDownloadsPrefix = 'O';
 
 const char* kMaxSuggestionsCountParamName = "downloads_max_count";
 
-// Maximal number of dismissed asset download IDs stored at any time.
-const int kMaxDismissedIdCount = 100;
-
 bool CompareOfflinePagesMostRecentlyCreatedFirst(const OfflinePageItem& left,
                                                  const OfflinePageItem& right) {
   return left.creation_time > right.creation_time;
 }
 
 int GetMaxSuggestionsCount() {
   bool assets_enabled =
       base::FeatureList::IsEnabled(features::kAssetDownloadSuggestionsFeature);
   return variations::GetVariationParamByFeatureAsInt(
       assets_enabled ? features::kAssetDownloadSuggestionsFeature
@@ skipping 57 matching lines @@
       OfflinePageModelQuery::Requirement::INCLUDE_MATCHING);
   return builder.Build(model->GetPolicyController());
 }
 
 }  // namespace
 
 DownloadSuggestionsProvider::DownloadSuggestionsProvider(
     ContentSuggestionsProvider::Observer* observer,
     offline_pages::OfflinePageModel* offline_page_model,
     content::DownloadManager* download_manager,
+    DownloadHistory* download_history,
     PrefService* pref_service,
     bool download_manager_ui_enabled)
     : ContentSuggestionsProvider(observer),
       category_status_(CategoryStatus::AVAILABLE_LOADING),
       provided_category_(Category::FromKnownCategory(
           ntp_snippets::KnownCategories::DOWNLOADS)),
       offline_page_model_(offline_page_model),
       download_manager_(download_manager),
+      download_history_(download_history),
       pref_service_(pref_service),
       download_manager_ui_enabled_(download_manager_ui_enabled),
+      is_asset_downloads_initialization_complete_(false),
       weak_ptr_factory_(this) {
   observer->OnCategoryStatusChanged(this, provided_category_, category_status_);
 
   DCHECK(offline_page_model_ || download_manager_);
   if (offline_page_model_) {
     offline_page_model_->AddObserver(this);
   }
 
   if (download_manager_) {
-    download_manager_->AddObserver(this);
+    // We will start listening to download manager once it is loaded.
+    // May be nullptr in tests.
+    if (download_history_) {
+      download_history_->AddObserver(this);
+    }
+  } else {
+    download_history_ = nullptr;
   }
 
-  // We explicitly fetch the asset downloads in case some of |OnDownloadCreated|
-  // happened earlier and, therefore, were missed.
-  AsynchronouslyFetchAllDownloadsAndSubmitSuggestions();
+  if (!download_manager_) {
+    // Usually, all downloads are fetched when the download manager is loaded,
+    // but now it is disabled, so offline pages are fetched here instead.
+    AsynchronouslyFetchOfflinePagesDownloads(/*notify=*/true);
+  }
 }
 
 DownloadSuggestionsProvider::~DownloadSuggestionsProvider() {
-  if (offline_page_model_) {
-    offline_page_model_->RemoveObserver(this);
+  if (download_history_) {
+    download_history_->RemoveObserver(this);
   }
 
   if (download_manager_) {
     download_manager_->RemoveObserver(this);
     UnregisterDownloadItemObservers();
   }
+
+  if (offline_page_model_) {
+    offline_page_model_->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_action=*/false,
       /*has_reload_action=*/false,
       /*has_view_all_action=*/download_manager_ui_enabled_,
       /*show_if_empty=*/false,
       l10n_util::GetStringUTF16(IDS_NTP_DOWNLOADS_SUGGESTIONS_SECTION_EMPTY));
 }
 
 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);
 
-  AddToDismissedStorageIfNeeded(suggestion_id);
   RemoveSuggestionFromCacheAndRetrieveMoreIfNeeded(suggestion_id);
 }
 
 void DownloadSuggestionsProvider::FetchSuggestionImage(
     const ContentSuggestion::ID& suggestion_id,
     const ntp_snippets::ImageFetchedCallback& callback) {
   // TODO(vitaliii): Fetch proper thumbnail from OfflinePageModel once it is
   // available there.
   // TODO(vitaliii): Provide site's favicon for assets downloads or file type.
   // See crbug.com/631447.
@@ skipping 50 matching lines @@
                    weak_ptr_factory_.GetWeakPtr(), callback));
   } else {
     GetPagesMatchingQueryCallbackForGetDismissedSuggestions(
         callback, std::vector<OfflinePageItem>());
   }
 }
 
 void DownloadSuggestionsProvider::ClearDismissedSuggestionsForDebugging(
     Category category) {
   DCHECK_EQ(provided_category_, category);
-  StoreAssetDismissedIDsToPrefs(std::vector<std::string>());
+  StoreAssetDismissedIDsToPrefs(std::set<std::string>());
   StoreOfflinePageDismissedIDsToPrefs(std::set<std::string>());
   AsynchronouslyFetchAllDownloadsAndSubmitSuggestions();
 }
 
 // static
 void DownloadSuggestionsProvider::RegisterProfilePrefs(
     PrefRegistrySimple* registry) {
   registry->RegisterListPref(kDismissedAssetDownloadSuggestions);
   registry->RegisterListPref(kDismissedOfflinePageDownloadSuggestions);
 }
 
-int DownloadSuggestionsProvider::GetMaxDismissedCountForTesting() {
-  return kMaxDismissedIdCount;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // Private methods
 
 void DownloadSuggestionsProvider::
     GetPagesMatchingQueryCallbackForGetDismissedSuggestions(
         const ntp_snippets::DismissedSuggestionsCallback& callback,
         const std::vector<OfflinePageItem>& offline_pages) const {
-  std::set<std::string> dismissed_offline_ids =
-      ReadOfflinePageDismissedIDsFromPrefs();
+  std::set<std::string> dismissed_ids = ReadOfflinePageDismissedIDsFromPrefs();
   std::vector<ContentSuggestion> suggestions;
   for (const OfflinePageItem& item : offline_pages) {
-    if (dismissed_offline_ids.count(
-            GetOfflinePagePerCategoryID(item.offline_id))) {
+    if (dismissed_ids.count(GetOfflinePagePerCategoryID(item.offline_id))) {
       suggestions.push_back(ConvertOfflinePage(item));
     }
   }
 
   if (download_manager_) {
     std::vector<DownloadItem*> all_downloads;
     download_manager_->GetAllDownloads(&all_downloads);
 
-    std::vector<std::string> dismissed_asset_ids =
-        ReadAssetDismissedIDsFromPrefs();
+    dismissed_ids = ReadAssetDismissedIDsFromPrefs();
 
     for (const DownloadItem* item : all_downloads) {
-      if (ContainsValue(dismissed_asset_ids,
-                        GetAssetDownloadPerCategoryID(item->GetId()))) {
+      if (dismissed_ids.count(GetAssetDownloadPerCategoryID(item->GetId()))) {
         suggestions.push_back(ConvertDownloadItem(*item));
       }
     }
   }
 
   callback.Run(std::move(suggestions));
 }
 
 void DownloadSuggestionsProvider::OfflinePageModelLoaded(
     offline_pages::OfflinePageModel* model) {
@@ skipping 36 matching lines @@
     const offline_pages::ClientId& client_id) {
   DCHECK(offline_page_model_);
   if (IsClientIdForOfflinePageDownload(
           offline_page_model_->GetPolicyController(), client_id)) {
     InvalidateSuggestion(GetOfflinePagePerCategoryID(offline_id));
   }
 }
 
 void DownloadSuggestionsProvider::OnDownloadCreated(DownloadManager* manager,
                                                     DownloadItem* item) {
+  DCHECK(is_asset_downloads_initialization_complete_);
   DCHECK_EQ(download_manager_, manager);
-  // This is called when new downloads are started and on startup for existing
-  // ones. We listen to each item to know when it is destroyed.
+
+  // This is called when new downloads are started. We listen to each item to
+  // know when it is finished or destroyed.
   item->AddObserver(this);
   if (CacheAssetDownloadIfNeeded(item)) {
     SubmitContentSuggestions();
   }
 }
 
 void DownloadSuggestionsProvider::ManagerGoingDown(DownloadManager* manager) {
   DCHECK_EQ(download_manager_, manager);
   UnregisterDownloadItemObservers();
   download_manager_ = nullptr;
+  if (download_history_) {
+    download_history_->RemoveObserver(this);
+    download_history_ = nullptr;
+  }
 }
 
 void DownloadSuggestionsProvider::OnDownloadUpdated(DownloadItem* item) {
-  if (ContainsValue(cached_asset_downloads_, item)) {
+  DCHECK(is_asset_downloads_initialization_complete_);
+  if (base::ContainsValue(cached_asset_downloads_, item)) {
     if (item->GetFileExternallyRemoved()) {
       InvalidateSuggestion(GetAssetDownloadPerCategoryID(item->GetId()));
     } else {
       // The download may have changed.
       SubmitContentSuggestions();
     }
   } else {
     // Unfinished downloads may become completed.
     if (CacheAssetDownloadIfNeeded(item)) {
       SubmitContentSuggestions();
     }
   }
 }
 
 void DownloadSuggestionsProvider::OnDownloadOpened(DownloadItem* item) {
   // Ignored.
 }
 
 void DownloadSuggestionsProvider::OnDownloadRemoved(DownloadItem* item) {
   // Ignored. We listen to |OnDownloadDestroyed| instead. The reason is that
   // we may need to retrieve all downloads, but |OnDownloadRemoved| is called
   // before the download is removed from the list.
 }
 
 void DownloadSuggestionsProvider::OnDownloadDestroyed(
     content::DownloadItem* item) {
+  DCHECK(is_asset_downloads_initialization_complete_);
+
   item->RemoveObserver(this);
 
   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::OnHistoryQueryComplete() {
+  is_asset_downloads_initialization_complete_ = true;
+  if (download_manager_) {
+    download_manager_->AddObserver(this);
+  }
+  AsynchronouslyFetchAllDownloadsAndSubmitSuggestions();
+}
+
+void DownloadSuggestionsProvider::OnDownloadHistoryDestroyed() {
+  DCHECK(download_history_);
+  download_history_->RemoveObserver(this);
+  download_history_ = nullptr;
+}
+
 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_);
@@ skipping 24 matching lines @@
 }
 
 void DownloadSuggestionsProvider::FetchAssetsDownloads() {
   if (!download_manager_) {
     // The manager has gone down or was explicitly turned off.
     return;
   }
 
   std::vector<DownloadItem*> all_downloads;
   download_manager_->GetAllDownloads(&all_downloads);
-  std::vector<std::string> old_dismissed_ids = ReadAssetDismissedIDsFromPrefs();
+  std::set<std::string> old_dismissed_ids = ReadAssetDismissedIDsFromPrefs();
+  std::set<std::string> retained_dismissed_ids;
   cached_asset_downloads_.clear();
-  for (const DownloadItem* item : all_downloads) {
+  for (DownloadItem* item : all_downloads) {
     std::string within_category_id =
         GetAssetDownloadPerCategoryID(item->GetId());
-    if (!ContainsValue(old_dismissed_ids, within_category_id)) {
-      if (IsDownloadCompleted(*item)) {
-        cached_asset_downloads_.push_back(item);
-      }
+    if (old_dismissed_ids.count(within_category_id)) {
+      retained_dismissed_ids.insert(within_category_id);
+    } else if (IsDownloadCompleted(*item)) {
+      cached_asset_downloads_.push_back(item);
+      // We may already observe this item and, therefore, we remove the
+      // observer first.
+      item->RemoveObserver(this);
+      item->AddObserver(this);
     }
   }
 
-  // We do not prune dismissed IDs, because it is not possible to ensure that
-  // the list of downloads is complete (i.e. DownloadManager has finished
-  // reading them).
-  // TODO(vitaliii): Prune dismissed IDs once the |OnLoaded| notification is
-  // provided. See crbug.com/672758.
+  if (old_dismissed_ids.size() != retained_dismissed_ids.size()) {
+    StoreAssetDismissedIDsToPrefs(retained_dismissed_ids);
+  }
+
   const int max_suggestions_count = GetMaxSuggestionsCount();
   if (static_cast<int>(cached_asset_downloads_.size()) >
       max_suggestions_count) {
     // Partially sorts |downloads| such that:
     // 1) The element at the index |max_suggestions_count| is changed to the
     //    element which would occur on this position if |downloads| was sorted;
     // 2) All of the elements before index |max_suggestions_count| are less than
     //    or equal to the elements after it.
     std::nth_element(cached_asset_downloads_.begin(),
                      cached_asset_downloads_.begin() + max_suggestions_count,
@@ skipping 77 matching lines @@
   suggestion.set_download_suggestion_extra(std::move(extra));
   return suggestion;
 }
 
 bool DownloadSuggestionsProvider::CacheAssetDownloadIfNeeded(
     const content::DownloadItem* item) {
   if (!IsDownloadCompleted(*item)) {
     return false;
   }
 
-  if (ContainsValue(cached_asset_downloads_, item)) {
+  if (base::ContainsValue(cached_asset_downloads_, item)) {
     return false;
   }
 
-  std::vector<std::string> dismissed_ids = ReadAssetDismissedIDsFromPrefs();
-  if (ContainsValue(dismissed_ids,
-                    GetAssetDownloadPerCategoryID(item->GetId()))) {
+  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()),
             GetMaxSuggestionsCount());
   if (static_cast<int>(cached_asset_downloads_.size()) ==
       GetMaxSuggestionsCount()) {
     auto oldest = std::max_element(
         cached_asset_downloads_.begin(), cached_asset_downloads_.end(),
         &CompareDownloadsMostRecentlyDownloadedFirst);
@@ skipping 117 matching lines @@
   if (notify) {
     SubmitContentSuggestions();
   }
 }
 
 void DownloadSuggestionsProvider::InvalidateSuggestion(
     const std::string& id_within_category) {
   ContentSuggestion::ID suggestion_id(provided_category_, id_within_category);
   observer()->OnSuggestionInvalidated(this, suggestion_id);
 
-  RemoveFromDismissedStorageIfNeeded(suggestion_id);
+  std::set<std::string> dismissed_ids =
+      ReadDismissedIDsFromPrefs(CorrespondsToOfflinePage(suggestion_id));
+  auto it = dismissed_ids.find(id_within_category);
+  if (it != dismissed_ids.end()) {
+    dismissed_ids.erase(it);
+    StoreDismissedIDsToPrefs(CorrespondsToOfflinePage(suggestion_id),
+                             dismissed_ids);
+  }
+
   RemoveSuggestionFromCacheAndRetrieveMoreIfNeeded(suggestion_id);
 }
 
-// TODO(vitaliii): Do not use std::vector, when we ensure that pruning happens
-// at the right time (crbug.com/672758).
-std::vector<std::string>
+std::set<std::string>
 DownloadSuggestionsProvider::ReadAssetDismissedIDsFromPrefs() const {
-  std::vector<std::string> dismissed_ids;
-  const base::ListValue* list =
-      pref_service_->GetList(kDismissedAssetDownloadSuggestions);
-  for (const std::unique_ptr<base::Value>& value : *list) {
-    std::string dismissed_id;
-    bool success = value->GetAsString(&dismissed_id);
-    DCHECK(success) << "Failed to parse dismissed id from prefs param "
-                    << kDismissedAssetDownloadSuggestions << " into string.";
-    dismissed_ids.push_back(dismissed_id);
-  }
-  return dismissed_ids;
+  return ntp_snippets::prefs::ReadDismissedIDsFromPrefs(
+      *pref_service_, kDismissedAssetDownloadSuggestions);
 }
 
 void DownloadSuggestionsProvider::StoreAssetDismissedIDsToPrefs(
-    const std::vector<std::string>& dismissed_ids) {
+    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; }));
-
-  base::ListValue list;
-  for (const std::string& dismissed_id : dismissed_ids) {
-    list.AppendString(dismissed_id);
-  }
-  pref_service_->Set(kDismissedAssetDownloadSuggestions, list);
+  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);
 }
 
-void DownloadSuggestionsProvider::AddToDismissedStorageIfNeeded(
-    const ContentSuggestion::ID& suggestion_id) {
-  if (CorrespondsToOfflinePage(suggestion_id)) {
-    std::set<std::string> dismissed_ids =
-        ReadOfflinePageDismissedIDsFromPrefs();
-    dismissed_ids.insert(suggestion_id.id_within_category());
+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 {
-    std::vector<std::string> dismissed_ids = ReadAssetDismissedIDsFromPrefs();
-    // The suggestion may be double dismissed from previously opened NTPs.
-    if (!ContainsValue(dismissed_ids, suggestion_id.id_within_category())) {
-      dismissed_ids.push_back(suggestion_id.id_within_category());
-      // TODO(vitaliii): Remove this workaround once the dismissed ids are
-      // pruned. See crbug.com/672758.
-      while (dismissed_ids.size() > kMaxDismissedIdCount) {
-        dismissed_ids.erase(dismissed_ids.begin());
-      }
-      StoreAssetDismissedIDsToPrefs(dismissed_ids);
-    }
+    StoreAssetDismissedIDsToPrefs(dismissed_ids);
   }
 }
 
-void DownloadSuggestionsProvider::RemoveFromDismissedStorageIfNeeded(
-    const ContentSuggestion::ID& suggestion_id) {
-  if (CorrespondsToOfflinePage(suggestion_id)) {
-    std::set<std::string> dismissed_ids =
-        ReadOfflinePageDismissedIDsFromPrefs();
-    if (dismissed_ids.count(suggestion_id.id_within_category())) {
-      dismissed_ids.erase(suggestion_id.id_within_category());
-      StoreOfflinePageDismissedIDsToPrefs(dismissed_ids);
-    }
-  } else {
-    std::vector<std::string> dismissed_ids = ReadAssetDismissedIDsFromPrefs();
-    auto it = std::find(dismissed_ids.begin(), dismissed_ids.end(),
-                        suggestion_id.id_within_category());
-    if (it != dismissed_ids.end()) {
-      dismissed_ids.erase(it);
-      StoreAssetDismissedIDsToPrefs(dismissed_ids);
-    }
-  }
-}
-
 void DownloadSuggestionsProvider::UnregisterDownloadItemObservers() {
   DCHECK_NE(download_manager_, nullptr);
 
   std::vector<DownloadItem*> all_downloads;
   download_manager_->GetAllDownloads(&all_downloads);
 
   for (DownloadItem* item : all_downloads) {
     item->RemoveObserver(this);
   }
 }