[Offline Pages] Set up the initial prefetching service.

chrome/browser/android/offline_pages/offline_page_suggestions_observer.cc

Index: chrome/browser/android/offline_pages/offline_page_suggestions_observer.cc
diff --git a/chrome/browser/android/offline_pages/offline_page_suggestions_observer.cc b/chrome/browser/android/offline_pages/offline_page_suggestions_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..20cebe61e50b2ad1618dd92de05458f9e7457716
--- /dev/null
+++ b/chrome/browser/android/offline_pages/offline_page_suggestions_observer.cc
@@ -0,0 +1,168 @@
+// Copyright 2017 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/android/offline_pages/offline_page_suggestions_observer.h"
+
+#include <unordered_set>
+
+#include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/android/offline_pages/prefetch_service_factory.h"
+#include "components/ntp_snippets/category.h"
+#include "components/ntp_snippets/category_status.h"
+#include "components/offline_pages/core/offline_page_model.h"
+#include "components/offline_pages/core/offline_page_model_query.h"
+#include "components/offline_pages/core/prefetch/prefetch_service_impl.h"
+
+namespace offline_pages {
+
+int kOfflinePageSuggestionsObserverUserDataKey;

[carlosk, 2017/04/11 17:31:35]

Where is this initialized?

[dewittj, 2017/04/11 21:25:48]

Nowhere, its address is used for KeyedService purposes. Its value is unused.

+
+namespace {
+
+// The default delegate that contains external dependencies for the Offline Page
+// Suggestions Observer.  This is unused in tests.

[carlosk, 2017/04/11 17:31:36]

nit: s/unused/replaced/

[dewittj, 2017/04/11 21:25:48]

DefaultDelegate is unused :) Reworded.

+class DefaultDelegate : public OfflinePageSuggestionsObserver::Delegate {
+ public:
+  DefaultDelegate(offline_pages::OfflinePageModel* offline_page_model,
+                  ntp_snippets::ContentSuggestionsService* service,
+                  const ntp_snippets::Category& category);
+  ~DefaultDelegate() override = default;
+
+  std::vector<GURL> GetSuggestionURLs() override;
+  void GetPagesWithURLs(
+      const std::vector<GURL>& suggestion_urls,
+      const MultipleOfflinePageItemCallback& callback) override;
+
+ private:
+  offline_pages::OfflinePageModel* offline_page_model_;
+  ntp_snippets::ContentSuggestionsService* service_;
+  ntp_snippets::Category category_;
+};
+
+DefaultDelegate::DefaultDelegate(
+    offline_pages::OfflinePageModel* offline_page_model,
+    ntp_snippets::ContentSuggestionsService* service,
+    const ntp_snippets::Category& category)
+    : offline_page_model_(offline_page_model),
+      service_(service),
+      category_(category) {}
+
+std::vector<GURL> DefaultDelegate::GetSuggestionURLs() {
+  std::vector<GURL> suggestion_urls;
+  for (const auto& suggestion :

[carlosk, 2017/04/11 17:31:35]

nit: explicitly mentioning ContentSuggestion here instead of auto is clearer.

[dewittj, 2017/04/11 21:25:48]

Done.

+       service_->GetSuggestionsForCategory(category_)) {
+    suggestion_urls.emplace_back(suggestion.url());
+  }
+  return suggestion_urls;
+}
+
+void DefaultDelegate::GetPagesWithURLs(
+    const std::vector<GURL>& suggestion_urls,
+    const MultipleOfflinePageItemCallback& callback) {
+  OfflinePageModelQueryBuilder builder;
+  builder.SetUrls(OfflinePageModelQuery::Requirement::INCLUDE_MATCHING,
+                  suggestion_urls);
+  offline_page_model_->GetPagesMatchingQuery(
+      builder.Build(offline_page_model_->GetPolicyController()), callback);
+}
+
+}  // namespace
+
+// static
+void OfflinePageSuggestionsObserver::ObserveContentSuggestionsService(
+    content::BrowserContext* browser_context,
+    offline_pages::OfflinePageModel* offline_page_model,
+    ntp_snippets::ContentSuggestionsService* service) {
+  auto category = ntp_snippets::Category::FromKnownCategory(
+      ntp_snippets::KnownCategories::ARTICLES);
+  auto suggestions_observer = base::MakeUnique<OfflinePageSuggestionsObserver>(
+      browser_context,
+      base::MakeUnique<DefaultDelegate>(offline_page_model, service, category),
+      category);
+  service->AddObserver(suggestions_observer.get());
+  service->SetUserData(&kOfflinePageSuggestionsObserverUserDataKey,
+                       suggestions_observer.release());
+}
+
+OfflinePageSuggestionsObserver::OfflinePageSuggestionsObserver(
+    content::BrowserContext* browser_context,
+    std::unique_ptr<Delegate> delegate,
+    const ntp_snippets::Category& category)
+    : delegate_(std::move(delegate)),
+      browser_context_(browser_context),
+      category_(category),
+      weak_ptr_factory_(this) {}
+
+OfflinePageSuggestionsObserver::~OfflinePageSuggestionsObserver() = default;
+
+void OfflinePageSuggestionsObserver::OnNewSuggestions(
+    ntp_snippets::Category category) {
+  if (category != category_ ||
+      category_status_ != ntp_snippets::CategoryStatus::AVAILABLE) {
+    return;
+  }
+
+  // If we get suggestions while processing other suggestions, just cancel those
+  // operations and restart anew.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
+  std::vector<GURL> suggestion_urls = delegate_->GetSuggestionURLs();
+  delegate_->GetPagesWithURLs(
+      suggestion_urls,
+      base::Bind(&OfflinePageSuggestionsObserver::GotPagesMatchingSuggestions,
+                 weak_ptr_factory_.GetWeakPtr(), suggestion_urls));
+}
+
+void OfflinePageSuggestionsObserver::OnCategoryStatusChanged(
+    ntp_snippets::Category category,
+    ntp_snippets::CategoryStatus new_status) {
+  if (category != category_)
+    return;
+
+  category_status_ = new_status;
+
+  if (category_status_ ==
+          ntp_snippets::CategoryStatus::CATEGORY_EXPLICITLY_DISABLED ||
+      category_status_ ==
+          ntp_snippets::CategoryStatus::ALL_SUGGESTIONS_EXPLICITLY_DISABLED) {
+    PrefetchService* service =
+        PrefetchServiceFactory::GetForBrowserContext(browser_context_);
+    service->RemoveAllUnprocessedURLSuggestions();
+  }
+}
+
+void OfflinePageSuggestionsObserver::OnSuggestionInvalidated(
+    const ntp_snippets::ContentSuggestion::ID& suggestion_id) {
+  // TODO(dewittj): Keep track of ContentSuggestion::IDs so we can invalidate
+  // suggestions as well.
+  NOTIMPLEMENTED();
+}
+
+void OfflinePageSuggestionsObserver::OnFullRefreshRequired() {
+  PrefetchService* service =
+      PrefetchServiceFactory::GetForBrowserContext(browser_context_);
+  service->RemoveAllUnprocessedURLSuggestions();

[carlosk, 2017/04/11 17:31:35]

From reading the doc on this observer method [1] it seems we should also fetch
new URLs following this cleanup.

[1]
https://cs.chromium.org/chromium/src/components/ntp_snippets/content_suggesti...

[dewittj, 2017/04/11 21:25:48]

Done.

+}
+
+void OfflinePageSuggestionsObserver::ContentSuggestionsServiceShutdown() {
+  // No need to do anything here, we will just stop getting events.
+}
+
+void OfflinePageSuggestionsObserver::GotPagesMatchingSuggestions(
+    const std::vector<GURL>& suggestion_urls,
+    const MultipleOfflinePageItemResult& pages) {
+  std::set<GURL> result_gurls;
+  result_gurls.insert(suggestion_urls.begin(), suggestion_urls.end());
+  for (const auto& page : pages) {

[Dmitry Titov, 2017/04/11 01:17:13]

That seems too strict. What if a user manually downloaded a page from X many
months ago - this will prevent us downloading a fresh copy (which has content
caused it to get into the suggestions today).

I think the 'recently downloaded' schema that we discussed today is better in
preventing 'duplicates' while not stopping the feature to download same URLs if
they reappear in suggestions.

[carlosk, 2017/04/11 17:31:35]

I think the main point here is not re-downloading archives that are still
available. Maybe restrict to only prefetched offline pages?

I would not worry too much about the manually downloaded coincidences as they
seem very unlikely.

[dewittj, 2017/04/11 21:25:48]

Maybe it is premature to do the offline page filtering in this patch. I've
removed it and will leave filtering to a future patch (perhaps adding a
CheckForDuplicatesOrZombies function to the PrefetchService).

+    result_gurls.erase(page.url);
+    result_gurls.erase(page.original_url);
+  }
+
+  PrefetchService* service =
+      PrefetchServiceFactory::GetForBrowserContext(browser_context_);
+  service->OnNewURLSuggestions(
+      std::vector<GURL>(result_gurls.begin(), result_gurls.end()));
+}
+
+}  // namespace offline_pages