Index: components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc |
diff --git a/components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc b/components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc |
index f8eeb57b9ff11b7ecf4bb0fd670b587442daad84..0cafdb352d7a83bbcc835a0ad6e3032f433c6a0c 100644 |
--- a/components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc |
+++ b/components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc |
@@ -7,13 +7,19 @@ |
#include <vector> |
#include "base/bind.h" |
+#include "base/strings/utf_string_conversions.h" |
#include "base/threading/thread_task_runner_handle.h" |
#include "components/ntp_snippets/category.h" |
#include "components/reading_list/core/reading_list_entry.h" |
#include "components/reading_list/core/reading_list_model.h" |
#include "components/strings/grit/components_strings.h" |
+#include "components/url_formatter/url_formatter.h" |
#include "ui/base/l10n/l10n_util.h" |
+namespace { |
Marc Treib
2017/03/28 13:25:15
nit: Move into the ntp_snippets namespace?
gambard
2017/03/28 15:10:06
Done.
|
+const int kMaxEntries = 3; |
+} |
+ |
namespace ntp_snippets { |
ReadingListSuggestionsProvider::ReadingListSuggestionsProvider( |
@@ -26,13 +32,11 @@ ReadingListSuggestionsProvider::ReadingListSuggestionsProvider( |
reading_list_model_(reading_list_model) { |
observer->OnCategoryStatusChanged(this, provided_category_, category_status_); |
reading_list_model->AddObserver(this); |
- if (reading_list_model_->loaded()) { |
Marc Treib
2017/03/28 13:25:15
Is this not needed anymore?
gambard
2017/03/28 15:10:06
No, if the model is loaded when an observer is add
|
- FetchReadingListInternal(); |
- } |
} |
ReadingListSuggestionsProvider::~ReadingListSuggestionsProvider() { |
- reading_list_model_->RemoveObserver(this); |
+ if (reading_list_model_) |
Marc Treib
2017/03/28 13:25:15
nit: braces please
optional: ScopedObserver might
gambard
2017/03/28 15:10:06
Thanks, I did not know about ScopedObserver!
|
+ reading_list_model_->RemoveObserver(this); |
} |
CategoryStatus ReadingListSuggestionsProvider::GetCategoryStatus( |
@@ -107,8 +111,66 @@ void ReadingListSuggestionsProvider::ReadingListModelLoaded( |
FetchReadingListInternal(); |
} |
+void ReadingListSuggestionsProvider::ReadingListModelBeingDeleted( |
+ const ReadingListModel* model) { |
+ DCHECK(model == reading_list_model_); |
+ reading_list_model_->RemoveObserver(this); |
+ reading_list_model_ = nullptr; |
+} |
+ |
void ReadingListSuggestionsProvider::FetchReadingListInternal() { |
- // TODO(crbug.com/702241): Implement this method. |
+ if (!reading_list_model_) |
+ return; |
+ |
+ DCHECK(reading_list_model_->loaded()); |
+ std::vector<const ReadingListEntry*> entries; |
+ for (const auto& url : reading_list_model_->Keys()) { |
Marc Treib
2017/03/28 13:25:15
s/auto/actual type/ ? IMO it's not clear what the
gambard
2017/03/28 15:10:06
Done.
|
+ const ReadingListEntry* entry = reading_list_model_->GetEntryByURL(url); |
+ if (!entry->IsRead()) { |
Marc Treib
2017/03/28 13:25:16
optional: "if (entry->IsRead()) { continue; }" ?
gambard
2017/03/28 15:10:06
Done.
|
+ bool added = false; |
+ for (auto it = entries.begin(); it != entries.end(); ++it) { |
+ if ((*it)->UpdateTime() < entry->UpdateTime()) { |
+ entries.insert(it, entry); |
+ added = true; |
+ break; |
+ } |
+ } |
+ if (!added && entries.size() < kMaxEntries) { |
+ entries.push_back(entry); |
+ } |
+ if (entries.size() > kMaxEntries) { |
+ entries.pop_back(); |
+ } |
Marc Treib
2017/03/28 13:25:16
Hm, IMO it's not really obvious what this is suppo
gambard
2017/03/28 15:10:07
Done.
|
+ } |
+ } |
+ |
+ std::vector<ContentSuggestion> suggestions; |
+ for (const ReadingListEntry* entry : entries) { |
+ ContentSuggestion suggestion(provided_category_, entry->URL().spec(), |
+ entry->URL()); |
+ |
+ if (entry->Title().size() > 0) { |
Marc Treib
2017/03/28 13:25:16
nit: !entry->Title().empty()
gambard
2017/03/28 15:10:07
Done.
|
+ suggestion.set_title(base::UTF8ToUTF16(entry->Title())); |
+ } else { |
+ suggestion.set_title(url_formatter::FormatUrl(entry->URL())); |
+ } |
+ suggestion.set_snippet_text( |
+ url_formatter::FormatUrl(entry->URL().GetOrigin())); |
Marc Treib
2017/03/28 13:25:16
If the URL is already used as the title, then this
gambard
2017/03/28 15:10:07
The question was to do this test in the provider o
Marc Treib
2017/03/28 15:31:29
How would you check it there? After formatting, I
gambard
2017/03/28 15:42:49
Sorry, I missread your comment. The URL is used in
Marc Treib
2017/03/28 15:51:45
So if the page doesn't have a title, you'd see:
ww
gambard
2017/03/29 07:23:24
Yes, you would see both. I looks better in the UI
|
+ suggestions.emplace_back(std::move(suggestion)); |
+ } |
+ |
+ NotifyStatusChanged(CategoryStatus::AVAILABLE); |
+ observer()->OnNewSuggestions(this, provided_category_, |
+ std::move(suggestions)); |
+} |
+ |
+void ReadingListSuggestionsProvider::NotifyStatusChanged( |
+ CategoryStatus new_status) { |
+ if (category_status_ == new_status) { |
+ return; |
+ } |
+ category_status_ = new_status; |
+ observer()->OnCategoryStatusChanged(this, provided_category_, new_status); |
} |
} // namespace ntp_snippets |