Respect time range in browsing data removal for last-visited data.

components/ntp_snippets/bookmarks/bookmark_last_visit_utils_unittest.cc

Index: components/ntp_snippets/bookmarks/bookmark_last_visit_utils_unittest.cc
diff --git a/components/ntp_snippets/bookmarks/bookmark_last_visit_utils_unittest.cc b/components/ntp_snippets/bookmarks/bookmark_last_visit_utils_unittest.cc
index c99c8695a5f72b072f04afff2fa9f51bf1afc629..6f0652717f983b253da12b97c4699f772846d313 100644
--- a/components/ntp_snippets/bookmarks/bookmark_last_visit_utils_unittest.cc
+++ b/components/ntp_snippets/bookmarks/bookmark_last_visit_utils_unittest.cc
@@ -4,8 +4,10 @@
 
 #include "components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h"
 
+#include <memory>
 #include <string>
 
+#include "base/callback.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -19,6 +21,7 @@
 using bookmarks::BookmarkModel;
 using bookmarks::BookmarkNode;
 
+using testing::Eq;
 using testing::IsEmpty;
 using testing::SizeIs;
 
@@ -48,32 +51,35 @@ void AddBookmarks(BookmarkModel* model,
 
 void AddBookmarksRecentOnMobile(BookmarkModel* model,
                                 int num,
-                                const base::Time& threshold_time) {
-  base::TimeDelta week = base::TimeDelta::FromDays(7);
-  base::Time recent_time = threshold_time + week;
-  std::string recent_time_string =
-      base::Int64ToString(recent_time.ToInternalValue());
-
+                                const base::Time& visit_time) {
   AddBookmarks(model, num, kBookmarkLastVisitDateOnMobileKey,
-               recent_time_string);
+               base::Int64ToString(visit_time.ToInternalValue()));
 }
 
 void AddBookmarksRecentOnDesktop(BookmarkModel* model,
                                  int num,
-                                 const base::Time& threshold_time) {
-  base::TimeDelta week = base::TimeDelta::FromDays(7);
-  base::Time recent_time = threshold_time + week;
-  std::string recent_time_string =
-      base::Int64ToString(recent_time.ToInternalValue());
-
+                                 const base::Time& visit_time) {
   AddBookmarks(model, num, kBookmarkLastVisitDateOnDesktopKey,
-               recent_time_string);
+               base::Int64ToString(visit_time.ToInternalValue()));
 }
 
 void AddBookmarksNonVisited(BookmarkModel* model, int num) {
   AddBookmarks(model, num, std::string(), std::string());
 }
 
+const BookmarkNode* AddSingleBookmark(BookmarkModel* model,
+                                      const std::string& url,
+                                      const std::string& last_visit_key,
+                                      const base::Time& visit_time) {
+  base::string16 title =
+      base::ASCIIToUTF16(base::StringPrintf("title-%s", url.c_str()));
+  const BookmarkNode* node =
+      model->AddURL(model->bookmark_bar_node(), 0, title, GURL(url));
+  model->SetNodeMetaInfo(node, last_visit_key,
+                         base::Int64ToString(visit_time.ToInternalValue()));
+  return node;
+}
+
 }  // namespace
 
 class GetRecentlyVisitedBookmarksTest : public testing::Test {
@@ -85,6 +91,10 @@ class GetRecentlyVisitedBookmarksTest : public testing::Test {
 
   const base::Time& threshold_time() const { return threshold_time_; }
 
+  base::Time GetRecentTime() const {
+    return threshold_time_ + base::TimeDelta::FromDays(7);
+  }
+
  private:
   base::Time threshold_time_;
 
@@ -109,7 +119,7 @@ TEST_F(GetRecentlyVisitedBookmarksTest, ShouldNotConsiderDesktopVisits) {
   std::unique_ptr<BookmarkModel> model =
       bookmarks::TestBookmarkClient::CreateModel();
   AddBookmarksRecentOnDesktop(model.get(), number_of_bookmarks,
-                              threshold_time());
+                              GetRecentTime());
 
   std::vector<const bookmarks::BookmarkNode*> result =
       GetRecentlyVisitedBookmarks(model.get(), number_of_bookmarks,
@@ -124,7 +134,7 @@ TEST_F(GetRecentlyVisitedBookmarksTest, ShouldConsiderDesktopVisits) {
   std::unique_ptr<BookmarkModel> model =
       bookmarks::TestBookmarkClient::CreateModel();
   AddBookmarksRecentOnDesktop(model.get(), number_of_recent_desktop,
-                              threshold_time());
+                              GetRecentTime());
   AddBookmarksNonVisited(model.get(),
                          number_of_bookmarks - number_of_recent_desktop);
 
@@ -139,8 +149,7 @@ TEST_F(GetRecentlyVisitedBookmarksTest, ShouldReturnNotMoreThanMaxCount) {
   const int number_of_bookmarks = 3;
   std::unique_ptr<BookmarkModel> model =
       bookmarks::TestBookmarkClient::CreateModel();
-  AddBookmarksRecentOnMobile(model.get(), number_of_bookmarks,
-                             threshold_time());
+  AddBookmarksRecentOnMobile(model.get(), number_of_bookmarks, GetRecentTime());
 
   const int max_count = number_of_bookmarks - 1;
   std::vector<const bookmarks::BookmarkNode*> result =
@@ -149,4 +158,142 @@ TEST_F(GetRecentlyVisitedBookmarksTest, ShouldReturnNotMoreThanMaxCount) {
   EXPECT_THAT(result, SizeIs(max_count));
 }
 
+namespace {
+
+base::Callback<bool(const GURL& url)> DeleteAllFilter() {
+  return base::Bind([] (const GURL& url) { return true; });
+}
+
+base::Callback<bool(const GURL& url)> DeleteOneURLFilter(
+    const GURL& to_delete) {
+  return base::Bind(
+      [](const GURL& to_delete, const GURL& url) { return url == to_delete; },
+      to_delete);
+}
+
+}  // namespace
+
+TEST(RemoveLastVisitedDatesBetween, ShouldRemoveTimestampsWithinTimeRange) {
+  const base::Time delete_begin =
+      base::Time::Now() - base::TimeDelta::FromDays(2);
+  const base::Time delete_end = base::Time::Max();
+
+  std::unique_ptr<BookmarkModel> model =
+      bookmarks::TestBookmarkClient::CreateModel();
+  AddSingleBookmark(model.get(), "http://url-1.com",
+                    kBookmarkLastVisitDateOnMobileKey,
+                    delete_begin + base::TimeDelta::FromSeconds(1));
+  AddSingleBookmark(model.get(), "http://url-1.com",
+                    kBookmarkLastVisitDateOnDesktopKey,
+                    delete_begin + base::TimeDelta::FromSeconds(1));
+  ASSERT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/true),
+      SizeIs(1));
+
+  RemoveLastVisitedDatesBetween(delete_begin, delete_end, DeleteAllFilter(),
+                                model.get());
+
+  EXPECT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/true),
+      IsEmpty());
+  // Verify that the bookmark model nodes themselve still exist.
+  std::vector<const BookmarkNode*> remaining_nodes;
+  model->GetNodesByURL(GURL("http://url-1.com"), &remaining_nodes);
+  EXPECT_THAT(remaining_nodes, SizeIs(2));
+}
+
+TEST(RemoveLastVisitedDatesBetween,
+     ShouldHandleMetadataFromOtherDeviceTypesSeparately) {
+  const base::Time delete_begin =
+      base::Time::Now() - base::TimeDelta::FromDays(2);
+  const base::Time delete_end = base::Time::Max();
+
+  std::unique_ptr<BookmarkModel> model =
+      bookmarks::TestBookmarkClient::CreateModel();
+  // Create a bookmark with last visited times from both, mobile and desktop.
+  // The mobile one is within the deletion interval, the desktop one outside.
+  // Only the mobile one should get deleted.
+  const BookmarkNode* node = AddSingleBookmark(
+      model.get(), "http://url-1.com", kBookmarkLastVisitDateOnMobileKey,
+      delete_begin + base::TimeDelta::FromSeconds(1));
+  model->SetNodeMetaInfo(
+      node, kBookmarkLastVisitDateOnDesktopKey,
+      base::Int64ToString(
+          (delete_begin - base::TimeDelta::FromSeconds(1)).ToInternalValue()));
+  ASSERT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/true),
+      SizeIs(1));
+
+  RemoveLastVisitedDatesBetween(delete_begin, delete_end, DeleteAllFilter(),
+                                model.get());
+
+  EXPECT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/false),
+      IsEmpty());
+  EXPECT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/true),
+      SizeIs(1));
+}
+
+TEST(RemoveLastVisitedDatesBetween, ShouldNotRemoveTimestampsOutsideTimeRange) {
+  const base::Time delete_begin =
+      base::Time::Now() - base::TimeDelta::FromDays(2);
+  const base::Time delete_end = delete_begin + base::TimeDelta::FromDays(5);
+
+  std::unique_ptr<BookmarkModel> model =
+      bookmarks::TestBookmarkClient::CreateModel();
+  AddSingleBookmark(model.get(), "http://url-1.com",
+                    kBookmarkLastVisitDateOnMobileKey,
+                    delete_begin - base::TimeDelta::FromSeconds(1));
+  AddSingleBookmark(model.get(), "http://url-2.com",
+                    kBookmarkLastVisitDateOnDesktopKey,
+                    delete_end + base::TimeDelta::FromSeconds(1));
+  ASSERT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/true),
+      SizeIs(2));
+
+  RemoveLastVisitedDatesBetween(delete_begin, delete_end, DeleteAllFilter(),
+                                model.get());
+
+  EXPECT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/true),
+      SizeIs(2));
+}
+
+TEST(RemoveLastVisitedDatesBetween, ShouldOnlyRemoveURLsWithinFilter) {
+  const base::Time delete_begin =
+      base::Time::Now() - base::TimeDelta::FromDays(2);
+  const base::Time delete_end = base::Time::Max();
+
+  std::unique_ptr<BookmarkModel> model =
+      bookmarks::TestBookmarkClient::CreateModel();
+  AddSingleBookmark(model.get(), "http://url-1.com",
+                    kBookmarkLastVisitDateOnMobileKey,
+                    delete_begin + base::TimeDelta::FromSeconds(1));
+  AddSingleBookmark(model.get(), "http://url-2.com",
+                    kBookmarkLastVisitDateOnMobileKey,
+                    delete_begin + base::TimeDelta::FromSeconds(1));
+  ASSERT_THAT(
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/false),
+      SizeIs(2));
+
+  RemoveLastVisitedDatesBetween(delete_begin, delete_end,
+                                DeleteOneURLFilter(GURL("http://url-2.com")),
+                                model.get());
+
+  std::vector<const bookmarks::BookmarkNode*> remaining_nodes =
+      GetRecentlyVisitedBookmarks(model.get(), 20, base::Time(),
+                                  /*consider_visits_from_desktop=*/false);
+  EXPECT_THAT(remaining_nodes, SizeIs(1));
+  EXPECT_THAT(remaining_nodes[0]->url(), Eq(GURL("http://url-1.com")));
+}
+
 }  // namespace ntp_snippets