History data counter update and deletion with OLDER_THAN_30_DAYS time-period.

HistoryService::GetHistoryCount() takes period endtime instead of Time::Max().
Added unittest for history deletion.

Bug: 243833
Change-Id: If804a5b17b1f34d985d4a46a6d77a33fd39c0cdc
Reviewed-on: https://chromium-review.googlesource.com/780219Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Sarath Singapati <sarath.singapati@huawei.com>
Cr-Commit-Position: refs/heads/master@{#521677}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -1223,6 +1223,39 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, RemoveHistoryForLastHour) {
   EXPECT_TRUE(tester.HistoryContainsURL(kOrigin2));
 }
 
+TEST_F(ChromeBrowsingDataRemoverDelegateTest, RemoveHistoryForOlderThan30Days) {
+  RemoveHistoryTester tester;
+  ASSERT_TRUE(tester.Init(GetProfile()));
+
+  base::Time older_than_29days =
+      base::Time::Now() - base::TimeDelta::FromDays(29);
+  base::Time older_than_30days =
+      base::Time::Now() - base::TimeDelta::FromDays(30);
+  base::Time older_than_31days =
+      base::Time::Now() - base::TimeDelta::FromDays(31);
+
+  tester.AddHistory(kOrigin1, base::Time::Now());
+  tester.AddHistory(kOrigin2, older_than_29days);
+  tester.AddHistory(kOrigin3, older_than_31days);
+
+  ASSERT_TRUE(tester.HistoryContainsURL(kOrigin1));
+  ASSERT_TRUE(tester.HistoryContainsURL(kOrigin2));
+  ASSERT_TRUE(tester.HistoryContainsURL(kOrigin3));
+
+  BlockUntilBrowsingDataRemoved(
+      base::Time(), older_than_30days,
+      ChromeBrowsingDataRemoverDelegate::DATA_TYPE_HISTORY, false);
+
+  EXPECT_EQ(ChromeBrowsingDataRemoverDelegate::DATA_TYPE_HISTORY,
+            GetRemovalMask());
+  EXPECT_EQ(content::BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB,
+            GetOriginTypeMask());
+
+  EXPECT_TRUE(tester.HistoryContainsURL(kOrigin1));
+  EXPECT_TRUE(tester.HistoryContainsURL(kOrigin2));
+  EXPECT_FALSE(tester.HistoryContainsURL(kOrigin3));
+}
+
 // This should crash (DCHECK) in Debug, but death tests don't work properly
 // here.
 // TODO(msramek): To make this testable, the refusal to delete history should

chrome/browser/browsing_data/history_counter_browsertest.cc

@@ -240,6 +240,11 @@ IN_PROC_BROWSER_TEST_F(HistoryCounterTest, PeriodChanged) {
   AddVisit("https://www.example.com");
   AddVisit("https://www.example.com");
 
+  RevertTimeInDays(100);
+  AddVisit("https://www.google.com");
+  AddVisit("https://www.example.com");
+  AddVisit("https://www.example.com");
+
   Profile* profile = browser()->profile();
 
   HistoryCounter counter(
@@ -270,7 +275,11 @@ IN_PROC_BROWSER_TEST_F(HistoryCounterTest, PeriodChanged) {
 
   SetDeletionPeriodPref(browsing_data::TimePeriod::ALL_TIME);
   WaitForCounting();
-  EXPECT_EQ(9u, GetLocalResult());
+  EXPECT_EQ(11u, GetLocalResult());
+
+  SetDeletionPeriodPref(browsing_data::TimePeriod::OLDER_THAN_30_DAYS);
+  WaitForCounting();
+  EXPECT_EQ(3u, GetLocalResult());
 }
 
 // Test the behavior for a profile that syncs history.

components/browsing_data/core/counters/history_counter.cc

@@ -66,7 +66,7 @@ void HistoryCounter::Count() {
   local_counting_finished_ = false;
 
   history_service_->GetHistoryCount(
-      GetPeriodStart(), base::Time::Max(),
+      GetPeriodStart(), GetPeriodEnd(),
       base::Bind(&HistoryCounter::OnGetLocalHistoryCount,
                  weak_ptr_factory_.GetWeakPtr()),
       &cancelable_task_tracker_);

components/history/core/browser/expire_history_backend_unittest.cc

@@ -630,6 +630,145 @@ TEST_F(ExpireHistoryTest, FlushRecentURLsUnstarred) {
   EXPECT_FALSE(HasFavicon(favicon_id2));
 }
 
+// Expires all URLs visited between two given times, with no starred items.
+TEST_F(ExpireHistoryTest, FlushURLsUnstarredBetweenTwoTimestamps) {
+  URLID url_ids[3];
+  base::Time visit_times[4];
+  AddExampleData(url_ids, visit_times);
+
+  URLRow url_row0, url_row1, url_row2;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[0], &url_row0));
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &url_row1));
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[2], &url_row2));
+
+  VisitVector visits;
+  main_db_->GetVisitsForURL(url_ids[0], &visits);
+  ASSERT_EQ(1U, visits.size());
+  main_db_->GetVisitsForURL(url_ids[1], &visits);
+  ASSERT_EQ(2U, visits.size());
+  main_db_->GetVisitsForURL(url_ids[2], &visits);
+  ASSERT_EQ(1U, visits.size());
+
+  // This should delete the two visits of the url_ids[1].
+  std::set<GURL> restrict_urls;
+  expirer_.ExpireHistoryBetween(restrict_urls, visit_times[1], visit_times[3]);
+
+  main_db_->GetVisitsForURL(url_ids[0], &visits);
+  EXPECT_EQ(1U, visits.size());
+  main_db_->GetVisitsForURL(url_ids[1], &visits);
+  EXPECT_EQ(0U, visits.size());
+  main_db_->GetVisitsForURL(url_ids[2], &visits);
+  EXPECT_EQ(1U, visits.size());
+
+  // Verify that the url_ids[1] was deleted.
+  favicon_base::FaviconID favicon_id1 =
+      GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon);
+  EnsureURLInfoGone(url_row1, false);
+  EXPECT_FALSE(HasFavicon(favicon_id1));
+
+  // Verify that the url_ids[0]'s favicon and thumbnail are still there.
+  favicon_base::FaviconID favicon_id0 =
+      GetFavicon(url_row0.url(), favicon_base::IconType::kFavicon);
+  EXPECT_TRUE(HasFavicon(favicon_id0));
+  // TODO(sky): fix this, see comment in HasThumbnail.
+  // EXPECT_TRUE(HasThumbnail(url_row0.id()));
+
+  // Verify that the url_ids[2]'s favicon and thumbnail are still there.
+  favicon_base::FaviconID favicon_id2 =
+      GetFavicon(url_row2.url(), favicon_base::IconType::kFavicon);
+  EXPECT_TRUE(HasFavicon(favicon_id2));
+  // TODO(sky): fix this, see comment in HasThumbnail.
+  // EXPECT_TRUE(HasThumbnail(url_row2.id()));
+}
+
+// Expires all URLs more recent than a given time, with no starred items.
+// Same as FlushRecentURLsUnstarred test but with base::Time::Max() as end_time.
+TEST_F(ExpireHistoryTest, FlushRecentURLsUnstarredWithMaxTime) {
+  URLID url_ids[3];
+  base::Time visit_times[4];
+  AddExampleData(url_ids, visit_times);
+
+  URLRow url_row1, url_row2;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &url_row1));
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[2], &url_row2));
+
+  VisitVector visits;
+  main_db_->GetVisitsForURL(url_ids[2], &visits);
+  ASSERT_EQ(1U, visits.size());
+
+  // Use base::Time::Max() instead of base::Time().
+  // This should delete the last two visits.
+  std::set<GURL> restrict_urls;
+  expirer_.ExpireHistoryBetween(restrict_urls, visit_times[2],
+                                base::Time::Max());
+
+  // Verify that the middle URL had its last visit deleted only.
+  visits.clear();
+  main_db_->GetVisitsForURL(url_ids[1], &visits);
+  EXPECT_EQ(1U, visits.size());
+
+  // Verify that the middle URL visit time and visit counts were updated.
+  EXPECT_TRUE(ModifiedNotificationSent(url_row1.url()));
+  URLRow temp_row;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &temp_row));
+  EXPECT_TRUE(visit_times[2] == url_row1.last_visit());  // Previous value.
+  EXPECT_TRUE(visit_times[1] == temp_row.last_visit());  // New value.
+  EXPECT_EQ(2, url_row1.visit_count());
+  EXPECT_EQ(1, temp_row.visit_count());
+  EXPECT_EQ(1, url_row1.typed_count());
+  EXPECT_EQ(0, temp_row.typed_count());
+
+  // Verify that the middle URL's favicon and thumbnail is still there.
+  favicon_base::FaviconID favicon_id =
+      GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon);
+  EXPECT_TRUE(HasFavicon(favicon_id));
+  // TODO(sky): fix this, see comment in HasThumbnail.
+  // EXPECT_TRUE(HasThumbnail(url_row1.id()));
+
+  // Verify that the last URL was deleted.
+  favicon_base::FaviconID favicon_id2 =
+      GetFavicon(url_row2.url(), favicon_base::IconType::kFavicon);
+  EnsureURLInfoGone(url_row2, false);
+  EXPECT_FALSE(HasFavicon(favicon_id2));
+}
+
+// Expires all URLs with no starred items.
+TEST_F(ExpireHistoryTest, FlushAllURLsUnstarred) {
+  URLID url_ids[3];
+  base::Time visit_times[4];
+  AddExampleData(url_ids, visit_times);
+
+  URLRow url_row1, url_row2;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &url_row1));
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[2], &url_row2));
+
+  VisitVector visits;
+  main_db_->GetVisitsForURL(url_ids[2], &visits);
+  ASSERT_EQ(1U, visits.size());
+
+  // This should delete all URL visits.
+  std::set<GURL> restrict_urls;
+  expirer_.ExpireHistoryBetween(restrict_urls, base::Time(), base::Time::Max());
+
+  // Verify that all URL visits deleted.
+  visits.clear();
+  main_db_->GetVisitsForURL(url_ids[1], &visits);
+  EXPECT_EQ(0U, visits.size());
+  main_db_->GetVisitsForURL(url_ids[2], &visits);
+  EXPECT_EQ(0U, visits.size());
+
+  // Verify that all URLs were deleted.
+  favicon_base::FaviconID favicon_id1 =
+      GetFavicon(url_row1.url(), favicon_base::IconType::kFavicon);
+  EnsureURLInfoGone(url_row1, false);
+  EXPECT_FALSE(HasFavicon(favicon_id1));
+
+  favicon_base::FaviconID favicon_id2 =
+      GetFavicon(url_row2.url(), favicon_base::IconType::kFavicon);
+  EnsureURLInfoGone(url_row2, false);
+  EXPECT_FALSE(HasFavicon(favicon_id2));
+}
+
 // Expires all URLs with times in a given set.
 TEST_F(ExpireHistoryTest, FlushURLsForTimes) {
   URLID url_ids[3];