Implement first version of OfflinePageSuggestionsProvider

components/ntp_snippets/offline_pages/offline_page_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 "components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h"
+
+#include "base/bind.h"
+#include "base/strings/stringprintf.h"
+
+using offline_pages::MultipleOfflinePageItemResult;
+using offline_pages::OfflinePageModel;
+using offline_pages::OfflinePageItem;
+
+namespace ntp_snippets {
+
+OfflinePageSuggestionsProvider::OfflinePageSuggestionsProvider(
+    OfflinePageModel* offline_page_model)
+    : ContentSuggestionsProvider({ContentSuggestionsCategory::OFFLINE_PAGES}),
+      category_status_(ContentSuggestionsCategoryStatus::AVAILABLE_LOADING),
+      observer_(nullptr),
+      offline_page_model_(offline_page_model) {
+  offline_page_model_->AddObserver(this);
+  FetchOfflinePages();

[Marc Treib, 2016/07/14 14:17:24]

Hm, this should probably happen in SetObserver, to make sure it doesn't return
before the observer exists? Otherwise, we'd lose the update.

[Philipp Keck, 2016/07/14 15:30:06]

Done. The same goes for the NTPSnippetsService as soon as it doesn't talk to the
UI directly anymore -- left a TODO comment there.

+}
+
+OfflinePageSuggestionsProvider::~OfflinePageSuggestionsProvider() {}
+
+// Inherited from KeyedService.
+void OfflinePageSuggestionsProvider::Shutdown() {
+  offline_page_model_->RemoveObserver(this);
+  category_status_ = ContentSuggestionsCategoryStatus::NOT_PROVIDED;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Private methods
+
+void OfflinePageSuggestionsProvider::SetObserver(
+    ContentSuggestionsProvider::Observer* observer) {
+  observer_ = observer;
+}
+
+ContentSuggestionsCategoryStatus
+OfflinePageSuggestionsProvider::GetCategoryStatus(
+    ContentSuggestionsCategory category) {
+  return category_status_;
+}
+
+void OfflinePageSuggestionsProvider::DiscardSuggestion(
+    const std::string& suggestion_id) {
+  // TODO(pke): Implement some "dont show on NTP anymore" behaviour,
+  // then also implement ClearDiscardedSuggestionsForDebugging.
+}
+
+void OfflinePageSuggestionsProvider::FetchSuggestionImage(
+    const std::string& suggestion_id,
+    const ImageFetchedCallback& callback) {
+  // TODO(pke): Implement
+}
+
+void OfflinePageSuggestionsProvider::ClearCachedSuggestionsForDebugging() {
+  // Ignore
+}
+
+void OfflinePageSuggestionsProvider::ClearDiscardedSuggestionsForDebugging() {
+  // Ignore
+}
+
+void OfflinePageSuggestionsProvider::OfflinePageModelLoaded(
+    OfflinePageModel* model) {
+  DCHECK_EQ(model, offline_page_model_);
+  // Ignore (when offline pages have been loaded, OnOfflinePagesLoaded will be
+  // called as a delayed callback, see ctor).
+}
+
+void OfflinePageSuggestionsProvider::OfflinePageModelChanged(
+    OfflinePageModel* model) {
+  FetchOfflinePages();

[Marc Treib, 2016/07/14 14:17:24]

DCHECK also here :)

[Philipp Keck, 2016/07/14 15:30:06]

Done.

+}
+
+void OfflinePageSuggestionsProvider::OfflinePageDeleted(
+    int64_t offline_id,
+    const offline_pages::ClientId& client_id) {
+  // TODO(pke): Implement, suggestion has to be removed from UI immediately.
+}
+
+void OfflinePageSuggestionsProvider::FetchOfflinePages() {
+  offline_page_model_->GetAllPages(
+      base::Bind(&OfflinePageSuggestionsProvider::OnOfflinePagesLoaded,
+                 base::Unretained(this)));
+}
+
+void OfflinePageSuggestionsProvider::OnOfflinePagesLoaded(
+    const MultipleOfflinePageItemResult& result) {
+  if (!observer_) {
+    category_status_ = ContentSuggestionsCategoryStatus::AVAILABLE;
+    return;
+  }
+
+  if (category_status_ != ContentSuggestionsCategoryStatus::AVAILABLE) {
+    category_status_ = ContentSuggestionsCategoryStatus::AVAILABLE;

[Marc Treib, 2016/07/14 14:17:24]

nit: For readability, I'd prefer having this assignment unconditionally.. maybe
make a SetCategoryStatus helper that updates it and notifies if necessary, and
call that right at the top?

[Philipp Keck, 2016/07/14 15:30:06]

Ok. Maybe that helper method should move to ContentSuggestionsProvider?

[Marc Treib, 2016/07/14 16:13:16]

Hm, if so, then all the status handling needs to go there... not sure if that's
a good idea. (I'm not opposed, I'm actually not sure :) )

[Philipp Keck, 2016/07/15 09:08:42]

Having all the status handling there would be difficult because a provider can
have multiple categories. So rather leave it here...

Created the helper method.

+    observer_->OnCategoryStatusChanged(
+        ContentSuggestionsCategory::OFFLINE_PAGES, category_status_);
+  }
+
+  std::vector<ContentSuggestion> suggestions;
+  for (const OfflinePageItem& item : result) {
+    ContentSuggestion suggestion(
+        MakeUniqueID(ContentSuggestionsCategory::OFFLINE_PAGES,
+                     std::to_string(item.offline_id)),

[Marc Treib, 2016/07/14 14:17:24]

This isn't allowed yet, see https://chromium-cpp.appspot.com/
Use base::StringToInt instead.

[Philipp Keck, 2016/07/14 15:30:06]

Right. I'm sorry that things like this (or TODOs without colon) keep reappearing
-- these CLs are older Git branches that I've been working on for a while and I
don't spot everything when I revise them before uploading.

[Marc Treib, 2016/07/14 16:13:16]

No worries!

+        item.GetOfflineURL());
+
+    // TODO(pke): Sort my most recently visited and only keep the top one of
+    // multiple entries for the same URL.
+    // TODO(pke): Get more reasonable data from the OfflinePageModel here.
+    suggestion.set_title(item.url.spec());
+    suggestion.set_snippet_text(std::string());
+    suggestion.set_publish_date(item.creation_time);
+    suggestion.set_publisher_name(item.url.host());
+    suggestions.emplace_back(std::move(suggestion));
+  }
+
+  observer_->OnNewSuggestions(ContentSuggestionsCategory::OFFLINE_PAGES,
+                              std::move(suggestions));
+}
+
+}  // namespace ntp_snippets