Revert "Expire redirect parents when expiring any history visit."

This reverts commit e4d9e6a3.

Reason for revert: This CL causes an infinite loop for some users. https://bugs.chromium.org/p/chromium/issues/detail?id=798234#c9

Original change's description:
> Expire redirect parents when expiring any history visit.
> 
> This CL fixes an issue with history deletion where Visits that were
> the end of redirect chains were not deleting their redirect parents.
> In effect, removing a single history entry from chrome://history was
> not removing all Visits associated with it, causing similar URLs to
> continue to show up in the omnibox autocomplete.
> 
> Bug: 786878
> Change-Id: Id207dd7810629458d19a847d3ff52c0e562f2ce6
> Reviewed-on: https://chromium-review.googlesource.com/795527
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Commit-Queue: calamity <calamity@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525589}

TBR=calamity@chromium.org,brettw@chromium.org,sdefresne@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 786878
Change-Id: I5b10eef437b09b8e61c8ca5d025e9b15e1226935
Reviewed-on: https://chromium-review.googlesource.com/848052Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526588}

components/history/core/browser/expire_history_backend.cc

@@ -262,15 +262,13 @@ void ExpireHistoryBackend::ExpireVisits(const VisitVector& visits) {
   if (visits.empty())
     return;
 
-  const VisitVector visits_and_redirects = GetVisitsAndRedirectParents(visits);
-
   DeleteEffects effects;
-  DeleteVisitRelatedInfo(visits_and_redirects, &effects);
+  DeleteVisitRelatedInfo(visits, &effects);
 
   // Delete or update the URLs affected. We want to update the visit counts
   // since this is called by the user who wants to delete their recent history,
   // and we don't want to leave any evidence.
-  ExpireURLsForVisits(visits_and_redirects, &effects);
+  ExpireURLsForVisits(visits, &effects);
   DeleteFaviconsIfPossible(&effects);
   BroadcastNotifications(&effects, DELETION_USER_INITIATED);
 
@@ -359,20 +357,6 @@ void ExpireHistoryBackend::BroadcastNotifications(DeleteEffects* effects,
   }
 }
 
-VisitVector ExpireHistoryBackend::GetVisitsAndRedirectParents(
-    const VisitVector& visits) {
-  VisitVector visits_and_redirects;
-  for (const auto v : visits) {
-    VisitRow current_visit = v;
-    do {
-      visits_and_redirects.push_back(current_visit);
-    } while (current_visit.referring_visit &&
-             main_db_->GetRowForVisit(current_visit.referring_visit,
-                                      &current_visit));
-  }
-  return visits_and_redirects;
-}
-
 void ExpireHistoryBackend::DeleteVisitRelatedInfo(const VisitVector& visits,
                                                   DeleteEffects* effects) {
   for (size_t i = 0; i < visits.size(); i++) {

components/history/core/browser/expire_history_backend.h

@@ -149,9 +149,6 @@ class ExpireHistoryBackend {
     std::set<GURL> deleted_favicons;
   };
 
-  // Returns a vector with all visits that eventually redirect to |visits|.
-  VisitVector GetVisitsAndRedirectParents(const VisitVector& visits);
-
   // Deletes the visit-related stuff for all the visits in the given list, and
   // adds the rows for unique URLs affected to the affected_urls list in
   // the dependencies structure.

components/history/core/browser/expire_history_backend_unittest.cc

@@ -46,31 +46,13 @@
 namespace history {
 
 namespace {
-
-base::Time PretendNow() {
-  base::Time::Exploded exploded_reference_time;
-  exploded_reference_time.year = 2015;
-  exploded_reference_time.month = 1;
-  exploded_reference_time.day_of_month = 2;
-  exploded_reference_time.day_of_week = 5;
-  exploded_reference_time.hour = 11;
-  exploded_reference_time.minute = 0;
-  exploded_reference_time.second = 0;
-  exploded_reference_time.millisecond = 0;
-
-  base::Time out_time;
-  EXPECT_TRUE(
-      base::Time::FromLocalExploded(exploded_reference_time, &out_time));
-  return out_time;
-}
-
 // Returns whether |url| can be added to history.
 bool MockCanAddURLToHistory(const GURL& url) {
   return url.is_valid();
 }
 
 base::Time GetOldFaviconThreshold() {
-  return PretendNow() -
+  return base::Time::Now() -
          base::TimeDelta::FromDays(internal::kOnDemandFaviconIsOldAfterDays);
 }
 
@@ -85,7 +67,7 @@ class ExpireHistoryTest : public testing::Test, public HistoryBackendNotifier {
         expirer_(this,
                  backend_client_.get(),
                  scoped_task_environment_.GetMainThreadTaskRunner()),
-        now_(PretendNow()) {}
+        now_(base::Time::Now()) {}
 
  protected:
   // Called by individual tests when they want data populated.
@@ -227,7 +209,7 @@ void ExpireHistoryTest::AddExampleData(URLID url_ids[3],
     return;
 
   // Four times for each visit.
-  visit_times[3] = PretendNow();
+  visit_times[3] = base::Time::Now();
   visit_times[2] = visit_times[3] - base::TimeDelta::FromDays(1);
   visit_times[1] = visit_times[3] - base::TimeDelta::FromDays(2);
   visit_times[0] = visit_times[3] - base::TimeDelta::FromDays(3);
@@ -261,7 +243,7 @@ void ExpireHistoryTest::AddExampleData(URLID url_ids[3],
 
   // Thumbnails for each URL.
   gfx::Image thumbnail = CreateGoogleThumbnailForTest();
-  ThumbnailScore score(0.25, true, true, PretendNow());
+  ThumbnailScore score(0.25, true, true, base::Time::Now());
 
   base::Time time;
   GURL gurl;
@@ -296,7 +278,7 @@ void ExpireHistoryTest::AddExampleSourceData(const GURL& url, URLID* id) {
   if (!main_db_)
     return;
 
-  base::Time last_visit_time = PretendNow();
+  base::Time last_visit_time = base::Time::Now();
   // Add one URL.
   URLRow url_row1(url);
   url_row1.set_last_visit(last_visit_time);
@@ -1043,7 +1025,7 @@ TEST_F(ExpireHistoryTest, ExpiringVisitsReader) {
       expirer_.GetAutoSubframeVisitsReader();
 
   VisitVector visits;
-  base::Time now = PretendNow();
+  base::Time now = base::Time::Now();
 
   // Verify that the early expiration threshold, stored in the meta table is
   // initialized.
@@ -1207,46 +1189,6 @@ TEST_F(ExpireHistoryTest,
   EXPECT_EQ(icon_id, icon_mapping[1].icon_id);
 }
 
-// Test that all visits that are redirect parents of specified visits are also
-// removed. See crbug.com/786878.
-TEST_F(ExpireHistoryTest, DeleteVisitAndRedirects) {
-  // Set up the example data.
-  base::Time now = PretendNow();
-  URLRow url_row1(GURL("http://google.com/1"));
-  url_row1.set_last_visit(now - base::TimeDelta::FromDays(1));
-  url_row1.set_visit_count(1);
-  URLID url1 = main_db_->AddURL(url_row1);
-
-  URLRow url_row2(GURL("http://www.google.com/1"));
-  url_row2.set_last_visit(now);
-  url_row2.set_visit_count(1);
-  URLID url2 = main_db_->AddURL(url_row2);
-
-  // Add a visit to "http://google.com/1" that is redirected to
-  // "http://www.google.com/1".
-  VisitRow visit_row1;
-  visit_row1.url_id = url1;
-  visit_row1.visit_time = now - base::TimeDelta::FromDays(1);
-  main_db_->AddVisit(&visit_row1, SOURCE_BROWSED);
-
-  VisitRow visit_row2;
-  visit_row2.url_id = url2;
-  visit_row2.visit_time = now;
-  visit_row2.referring_visit = visit_row1.visit_id;
-  main_db_->AddVisit(&visit_row2, SOURCE_BROWSED);
-
-  // Expiring visit_row2 should also expire visit_row1 which is its redirect
-  // parent.
-  expirer_.ExpireVisits({visit_row2});
-
-  VisitRow v;
-  EXPECT_FALSE(main_db_->GetRowForVisit(visit_row1.visit_id, &v));
-  EXPECT_FALSE(main_db_->GetRowForVisit(visit_row2.visit_id, &v));
-  URLRow u;
-  EXPECT_FALSE(main_db_->GetURLRow(url1, &u));
-  EXPECT_FALSE(main_db_->GetURLRow(url2, &u));
-}
-
 // TODO(brettw) add some visits with no URL to make sure everything is updated
 // properly. Have the visits also refer to nonexistent FTS rows.
 //