Add a tab helper to record the last visit date for each bookmark.

components/ntp_snippets/bookmarks/bookmark_last_visit_utils.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/bookmarks/bookmark_last_visit_utils.h"
+
+#include <algorithm>
+#include <string>
+#include <utility>
+
+#include "base/strings/string_number_conversions.h"
+
+using bookmarks::BookmarkModel;
+using bookmarks::BookmarkNode;
+
+namespace ntp_snippets {
+
+namespace {
+
+const char kBookmarkLastVisitDateKey[] = "last_visited";
+
+base::Time ParseLastVisitDate(const std::string& date_string) {
+  int64_t date = 0;
+  if (!base::StringToInt64(date_string, &date))
+    return base::Time::UnixEpoch();
+  return base::Time::FromInternalValue(date);
+}
+
+std::string FormatLastVisitDate(const base::Time& date) {
+  return base::Int64ToString(date.ToInternalValue());
+}
+
+bool CompareBookmarksByLastVisitDate(const BookmarkNode* a,
+                                     const BookmarkNode* b) {
+  return GetLastVisitDateForBookmark(a) > GetLastVisitDateForBookmark(b);
+}
+
+}  // namespace
+
+void UpdateBookmarkOnURLVisitedInMainFrame(BookmarkModel* bookmark_model,
+                                           const GURL& url) {
+  std::vector<const BookmarkNode*> bookmarks_for_url;
+  bookmark_model->GetNodesByURL(url, &bookmarks_for_url);
+  if (bookmarks_for_url.empty())
+    return;
+
+  // If there are bookmark for |url|, set their last visit date to now.
+  for (auto* node : bookmarks_for_url) {
+    bookmark_model->SetNodeMetaInfo(node, kBookmarkLastVisitDateKey,
+                                    FormatLastVisitDate(base::Time::Now()));

[Marc Treib, 2016/07/30 14:38:09]

nit: You could pull the base::Time::Now() out of the loop, to make sure they all
get the exact same time.

[Philipp Keck, 2016/08/02 13:05:33]

Done.

+  }
+}
+
+base::Time GetLastVisitDateForBookmark(const BookmarkNode* node) {
+  if (!node)
+    return base::Time::UnixEpoch();
+
+  std::string last_visit_date_string;
+  node->GetMetaInfo(kBookmarkLastVisitDateKey, &last_visit_date_string);
+
+  if (last_visit_date_string.empty()) {
+    // Use creation date if no last visit info present.
+    return node->date_added();
+  }
+
+  return ParseLastVisitDate(last_visit_date_string);
+}
+
+std::vector<const BookmarkNode*> GetRecentlyVisitedBookmarks(
+    BookmarkModel* bookmark_model,
+    int max_count,
+    const base::Time& min_visit_time) {
+  // Get all the bookmarks.
+  std::vector<BookmarkModel::URLAndTitle> bookmarks;
+  bookmark_model->GetBookmarks(&bookmarks);
+  // Remove bookmarks that have not been visited after |min_time|.
+  std::vector<const BookmarkNode*> bookmarks_for_url;

[Marc Treib, 2016/07/30 14:38:09]

Can you declare this variable inside the lambda?

[Philipp Keck, 2016/08/02 13:05:33]

Done.

+  bookmarks.erase(
+      std::remove_if(
+          bookmarks.begin(), bookmarks.end(),
+          [&bookmark_model, &min_visit_time,
+           &bookmarks_for_url](const BookmarkModel::URLAndTitle& bookmark) {

[Marc Treib, 2016/07/30 14:38:09]

Is this line break correct? Looks super weird

[Philipp Keck, 2016/08/02 13:05:33]

Looks fine to me, though now it looks different because bookmarks_for_url is now
in the lambda.

+            // Get all bookmarks for the given URL.
+            bookmarks_for_url.clear();
+            bookmark_model->GetNodesByURL(bookmark.url, &bookmarks_for_url);
+            // Find the most recent visit time in the set.
+            base::Time max_visit_time = min_visit_time;
+            for (auto* node : bookmarks_for_url) {

[Marc Treib, 2016/07/30 14:38:09]

const auto* ?

[Philipp Keck, 2016/08/02 13:05:33]

Done.

+              base::Time node_visit_time = GetLastVisitDateForBookmark(node);
+              if (node_visit_time > max_visit_time)
+                max_visit_time = node_visit_time;

[Marc Treib, 2016/07/30 14:38:09]

optional nit:
max_visit_time = std::max(max_visit_time, GetLastVisitDateForBookmark(node));

[Philipp Keck, 2016/08/02 13:05:33]

Done.

+            }
+            return (max_visit_time <= min_visit_time);

[Marc Treib, 2016/07/30 14:38:09]

nit: no parens required

[Philipp Keck, 2016/08/02 13:05:33]

Done.

+          }),
+      bookmarks.end());
+  // Sort the remaining bookmarks by date.
+  std::sort(bookmarks.begin(), bookmarks.end(),
+            [&bookmark_model](const BookmarkModel::URLAndTitle& a,
+                const BookmarkModel::URLAndTitle& b) {

[Marc Treib, 2016/07/30 14:38:09]

Is this the correct indentation? Looks super weird to me...

[Philipp Keck, 2016/08/02 13:05:33]

Done.

+              return CompareBookmarksByLastVisitDate(
+                  bookmark_model->GetMostRecentlyAddedUserNodeForURL(a.url),
+                  bookmark_model->GetMostRecentlyAddedUserNodeForURL(b.url));
+            });
+
+  std::vector<const BookmarkNode*> result;
+  // Insert the first |max_count| items from |bookmarks| into |result|.
+  for (const BookmarkModel::URLAndTitle& bookmark : bookmarks) {
+    result.push_back(
+        bookmark_model->GetMostRecentlyAddedUserNodeForURL(bookmark.url));
+    if (result.size() >= static_cast<size_t>(max_count))
+      break;
+  }
+  return result;
+}
+
+}  // namespace ntp_snippets