Fix navigation deletion for urls with multiple visits

If the user deletes urls from history for a specific day we only delete
navigations if there are no additional visits outside this time range because
url are not reported as being deleted to history observers unless all visits
have been removed.
To fix the issue, the restrict_urls parameter is passed through
HistoryServiceObserver to NavigationEntryRemover in order perform a
deletion for specific urls within the time range correctly.

Bug: 838086
Change-Id: I22226924b3ca9a3d0880aa62548c87d33b093d13
Reviewed-on: https://chromium-review.googlesource.com/1041956
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557167}

chrome/browser/browsing_data/browsing_data_history_observer_service.cc

@@ -31,8 +31,7 @@ void BrowsingDataHistoryObserverService::OnURLsDeleted(
     history::HistoryService* history_service,
     const history::DeletionInfo& deletion_info) {
   if (!deletion_info.is_from_expiration())
-    browsing_data::RemoveNavigationEntries(profile_, deletion_info.time_range(),
-                                           deletion_info.deleted_rows());
+    browsing_data::RemoveNavigationEntries(profile_, deletion_info);
 }
 
 // static

chrome/browser/browsing_data/navigation_entry_remover.cc

@@ -33,28 +33,42 @@
 
 namespace {
 
-bool TimeRangeMatcher(base::Time begin,
-                      base::Time end,
-                      const content::NavigationEntry& entry) {
-  return begin <= entry.GetTimestamp() &&
-         (entry.GetTimestamp() < end || end.is_null());
+bool ShouldDeleteUrl(base::Time begin,
+                     base::Time end,
+                     const base::Optional<std::set<GURL>>& restrict_urls,
+                     const GURL& url,
+                     base::Time time_stamp) {
+  return begin <= time_stamp && (time_stamp < end || end.is_null()) &&
+         (!restrict_urls.has_value() ||
+          restrict_urls->find(url) != restrict_urls->end());
 }
 
-bool TimeRangeMatcherForSession(
+bool ShouldDeleteNavigationEntry(
     base::Time begin,
     base::Time end,
+    const base::Optional<std::set<GURL>>& restrict_urls,
+    const content::NavigationEntry& entry) {
+  return ShouldDeleteUrl(begin, end, restrict_urls, entry.GetURL(),
+                         entry.GetTimestamp());
+}
+
+bool ShouldDeleteSerializedNavigationEntry(
+    base::Time begin,
+    base::Time end,
+    const base::Optional<std::set<GURL>>& restrict_urls,
     const sessions::SerializedNavigationEntry& entry) {
-  return begin <= entry.timestamp() &&
-         (entry.timestamp() < end || end.is_null());
+  return ShouldDeleteUrl(begin, end, restrict_urls, entry.virtual_url(),
+                         entry.timestamp());
 }
 
-bool UrlMatcher(const base::flat_set<GURL>& urls,
-                const content::NavigationEntry& entry) {
+bool UrlMatcherForNavigationEntry(const base::flat_set<GURL>& urls,
+                                  const content::NavigationEntry& entry) {
   return urls.find(entry.GetURL()) != urls.end();
 }
 
-bool UrlMatcherForSession(const base::flat_set<GURL>& urls,
-                          const sessions::SerializedNavigationEntry& entry) {
+bool UrlMatcherForSerializedNavigationEntry(
+    const base::flat_set<GURL>& urls,
+    const sessions::SerializedNavigationEntry& entry) {
   return urls.find(entry.virtual_url()) != urls.end();
 }
 
@@ -76,21 +90,26 @@ void DeleteNavigationEntries(
     controller->DeleteNavigationEntries(predicate);
 }
 
-void DeleteTabNavigationEntries(Profile* profile,
-                                const history::DeletionTimeRange& time_range,
-                                const base::flat_set<GURL>& url_set) {
-  auto predicate =
-      time_range.IsValid()
-          ? base::BindRepeating(&TimeRangeMatcher, time_range.begin(),
-                                time_range.end())
-          : base::BindRepeating(&UrlMatcher, base::ConstRef(url_set));
+void DeleteTabNavigationEntries(
+    Profile* profile,
+    const history::DeletionTimeRange& time_range,
+    const base::Optional<std::set<GURL>>& restrict_urls,
+    const base::flat_set<GURL>& url_set) {
+  auto predicate = time_range.IsValid()
+                       ? base::BindRepeating(
+                             &ShouldDeleteNavigationEntry, time_range.begin(),
+                             time_range.end(), base::ConstRef(restrict_urls))
+                       : base::BindRepeating(&UrlMatcherForNavigationEntry,
+                                             base::ConstRef(url_set));
 
 #if defined(OS_ANDROID)
   auto session_predicate =
       time_range.IsValid()
-          ? base::BindRepeating(&TimeRangeMatcherForSession, time_range.begin(),
-                                time_range.end())
-          : base::BindRepeating(&UrlMatcherForSession, base::ConstRef(url_set));
+          ? base::BindRepeating(&ShouldDeleteSerializedNavigationEntry,
+                                time_range.begin(), time_range.end(),
+                                base::ConstRef(restrict_urls))
+          : base::BindRepeating(&UrlMatcherForSerializedNavigationEntry,
+                                base::ConstRef(url_set));
 
   for (auto it = TabModelList::begin(); it != TabModelList::end(); ++it) {
     TabModel* tab_model = *it;
@@ -155,9 +174,11 @@ class TabRestoreDeletionHelper : public sessions::TabRestoreServiceObserver {
   DISALLOW_COPY_AND_ASSIGN(TabRestoreDeletionHelper);
 };
 
-void DeleteTabRestoreEntries(Profile* profile,
-                             const history::DeletionTimeRange& time_range,
-                             const base::flat_set<GURL>& url_set) {
+void DeleteTabRestoreEntries(
+    Profile* profile,
+    const history::DeletionTimeRange& time_range,
+    const base::Optional<std::set<GURL>>& restrict_urls,
+    const base::flat_set<GURL>& url_set) {
   sessions::TabRestoreService* tab_service =
       TabRestoreServiceFactory::GetForProfile(profile);
   if (!tab_service)
@@ -165,9 +186,11 @@ void DeleteTabRestoreEntries(Profile* profile,
 
   auto predicate =
       time_range.IsValid()
-          ? base::BindRepeating(&TimeRangeMatcherForSession, time_range.begin(),
-                                time_range.end())
-          : base::BindRepeating(&UrlMatcherForSession, url_set);
+          ? base::BindRepeating(&ShouldDeleteSerializedNavigationEntry,
+                                time_range.begin(), time_range.end(),
+                                restrict_urls)
+          : base::BindRepeating(&UrlMatcherForSerializedNavigationEntry,
+                                url_set);
   if (tab_service->IsLoaded()) {
     PerformTabRestoreDeletion(tab_service, predicate);
   } else {
@@ -190,20 +213,22 @@ void DeleteLastSessionFromSessionService(Profile* profile) {
 namespace browsing_data {
 
 void RemoveNavigationEntries(Profile* profile,
-                             const history::DeletionTimeRange& time_range,
-                             const history::URLRows& deleted_rows) {
+                             const history::DeletionInfo& deletion_info) {
   DCHECK(profile->GetProfileType() == Profile::ProfileType::REGULAR_PROFILE);
+  DCHECK(!deletion_info.is_from_expiration());
   if (!base::FeatureList::IsEnabled(
           browsing_data::features::kRemoveNavigationHistory)) {
     return;
   }
 
   base::flat_set<GURL> url_set;
-  if (!time_range.IsValid())
-    url_set = CreateUrlSet(deleted_rows);
+  if (!deletion_info.time_range().IsValid())
+    url_set = CreateUrlSet(deletion_info.deleted_rows());
 
-  DeleteTabNavigationEntries(profile, time_range, url_set);
-  DeleteTabRestoreEntries(profile, time_range, url_set);
+  DeleteTabNavigationEntries(profile, deletion_info.time_range(),
+                             deletion_info.restrict_urls(), url_set);
+  DeleteTabRestoreEntries(profile, deletion_info.time_range(),
+                          deletion_info.restrict_urls(), url_set);
   DeleteLastSessionFromSessionService(profile);
 }
 

chrome/browser/browsing_data/navigation_entry_remover.h

@@ -15,12 +15,12 @@ namespace browsing_data {
 // Remove navigation entries from the tabs of all browsers of |profile|.
 // Recent tabs will be cleaned up as well and the session will be rewritten.
 // The last session will be removed as it can't be cleaned up easily.
-// If a valid time_range is supplied, all entries within this time range will be
-// removed and |deleted_rows| is ignored.
-// Otherwise entries matching |deleted_rows| will be deleted.
+// If a valid |deletion_info.time_range()| is supplied,
+// |deletion_info.restrict_urls()| (or all URLs if empty) within this time range
+// will be removed and |deletion_info.deleted_rows()| is ignored. Otherwise
+// entries matching |deletion_info.deleted_rows()| will be deleted.
 void RemoveNavigationEntries(Profile* profile,
-                             const history::DeletionTimeRange& time_range,
-                             const history::URLRows& deleted_rows);
+                             const history::DeletionInfo& deletion_info);
 
 }  // namespace browsing_data
 

chrome/browser/browsing_data/navigation_entry_remover_browsertest.cc

@@ -23,6 +23,8 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "url/gurl.h"
 
+using history::DeletionInfo;
+
 class NavigationEntryRemoverTest : public InProcessBrowserTest {
  protected:
   void SetUpOnMainThread() override {
@@ -85,6 +87,14 @@ class NavigationEntryRemoverTest : public InProcessBrowserTest {
     return urls;
   }
 
+  DeletionInfo CreateDeletionInfo(base::Time from,
+                                  base::Time to,
+                                  std::set<GURL> restrict_urls = {}) {
+    return DeletionInfo(history::DeletionTimeRange(from, to), false, {}, {},
+                        restrict_urls.empty() ? base::Optional<std::set<GURL>>()
+                                              : restrict_urls);
+  }
+
   // Helper to compare vectors. The macro gets confused by EXPECT_EQ(v, {a,b}).
   void ExpectEntries(const std::vector<GURL>& expected,
                      const std::vector<GURL>& actual) {
@@ -146,38 +156,46 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, GoBack) {
 IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteIndividual) {
   AddNavigations(browser(), {url_a_, url_b_, url_c_, url_d_});
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_b_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_b_, base::Time())}, {}));
   ExpectEntries({about_blank_, url_a_, url_c_, url_d_}, GetEntries());
 
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_c_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_c_, base::Time())}, {}));
   ExpectEntries({about_blank_, url_a_, url_d_}, GetEntries());
 }
 
 IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteAfterNavigation) {
   AddNavigations(browser(), {url_a_, url_b_});
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_b_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_b_, base::Time())}, {}));
   // The commited entry can't be removed.
   ExpectEntries({about_blank_, url_a_, url_b_}, GetEntries());
 
   AddNavigations(browser(), {url_c_});
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_b_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_b_, base::Time())}, {}));
   ExpectEntries({about_blank_, url_a_, url_c_}, GetEntries());
 }
 
 IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteAll) {
   AddNavigations(browser(), {url_a_, url_b_, url_c_});
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::AllTime(), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         DeletionInfo::ForAllHistory());
   ExpectEntries({url_c_}, GetEntries());
 }
 
+IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteRestricted) {
+  AddNavigations(browser(), {url_a_, url_b_, url_c_});
+  browsing_data::RemoveNavigationEntries(
+      browser()->profile(),
+      CreateDeletionInfo(base::Time(), base::Time::Now(), {url_b_}));
+  ExpectEntries({about_blank_, url_a_, url_c_}, GetEntries());
+}
+
 IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteRange) {
   base::Time t1 = base::Time::Now();
   AddNavigations(browser(), {url_a_});
@@ -188,36 +206,54 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteRange) {
   ASSERT_NE(t1, t2);
   ASSERT_NE(t2, t3);
 
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange(t2, t3), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         CreateDeletionInfo(t2, t3));
   ExpectEntries({about_blank_, url_a_, url_c_, url_d_}, GetEntries());
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange(base::Time(), t1), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         CreateDeletionInfo(base::Time(), t1));
   ExpectEntries({url_a_, url_c_, url_d_}, GetEntries());
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange(t3, base::Time()), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         CreateDeletionInfo(t3, base::Time()));
   ExpectEntries({url_a_, url_d_}, GetEntries());
 }
 
+IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteRangeRestricted) {
+  base::Time t1 = base::Time::Now();
+  AddNavigations(browser(), {url_a_});
+  base::Time t2 = base::Time::Now();
+  AddNavigations(browser(), {url_b_});
+  base::Time t3 = base::Time::Now();
+  AddNavigations(browser(), {url_c_, url_a_});
+  ASSERT_NE(t1, t2);
+  ASSERT_NE(t2, t3);
+
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         CreateDeletionInfo(t1, t3, {url_b_}));
+  ExpectEntries({about_blank_, url_a_, url_c_, url_a_}, GetEntries());
+  browsing_data::RemoveNavigationEntries(
+      browser()->profile(), CreateDeletionInfo(base::Time(), t2, {url_a_}));
+  ExpectEntries({about_blank_, url_c_, url_a_}, GetEntries());
+}
+
 IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, DeleteAllAfterNavigation) {
   AddNavigations(browser(), {url_a_, url_b_, url_c_});
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::AllTime(), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         DeletionInfo::ForAllHistory());
   ExpectEntries({url_c_}, GetEntries());
 
   AddNavigations(browser(), {url_d_});
   ExpectEntries({url_c_, url_d_}, GetEntries());
 
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::AllTime(), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         DeletionInfo::ForAllHistory());
   ExpectEntries({url_d_}, GetEntries());
 }
 
 IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, TwoTabsDeletion) {
   AddNavigations(browser(), {url_a_, url_b_});
   AddTab(browser(), {url_c_, url_d_});
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::AllTime(), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         DeletionInfo::ForAllHistory());
 
   ExpectEntries({url_b_, url_d_}, GetEntries());
 }
@@ -226,8 +262,8 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, TwoWindowsDeletion) {
   AddNavigations(browser(), {url_a_, url_b_});
   AddBrowser(browser(), {url_c_, url_d_});
 
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::AllTime(), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         DeletionInfo::ForAllHistory());
 
   ExpectEntries({url_b_, url_d_}, GetEntries());
 }
@@ -236,8 +272,8 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, GoBackAndDelete) {
   AddNavigations(browser(), {url_a_, url_b_, url_c_});
 
   GoBack(browser()->tab_strip_model()->GetActiveWebContents());
-  browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::AllTime(), {});
+  browsing_data::RemoveNavigationEntries(browser()->profile(),
+                                         DeletionInfo::ForAllHistory());
 
   ExpectEntries({url_b_}, GetEntries());
 }
@@ -254,8 +290,8 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, RecentTabDeletion) {
   EXPECT_EQ(2U, tab_service->entries().size());
 
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_c_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_c_, base::Time())}, {}));
   content::RunAllTasksUntilIdle();
   EXPECT_EQ(1U, tab_service->entries().size());
   auto* tab = static_cast<sessions::TabRestoreService::Tab*>(
@@ -264,8 +300,8 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, RecentTabDeletion) {
   EXPECT_TRUE(tab_service->IsLoaded());
 
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_d_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_d_, base::Time())}, {}));
   EXPECT_EQ(0U, tab_service->entries().size());
 }
 
@@ -288,9 +324,10 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, RecentTabWindowDeletion) {
 
   // Delete b and d. The last opened tab should be removed.
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_b_, base::Time()),
-       history::URLResult(url_d_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_b_, base::Time()),
+                             history::URLResult(url_d_, base::Time())},
+                            {}));
   content::RunAllTasksUntilIdle();
   EXPECT_EQ(1U, tab_service->entries().size());
   ASSERT_EQ(sessions::TabRestoreService::WINDOW,
@@ -307,8 +344,8 @@ IN_PROC_BROWSER_TEST_F(NavigationEntryRemoverTest, RecentTabWindowDeletion) {
 
   // Delete a. The Window should be converted to a Tab.
   browsing_data::RemoveNavigationEntries(
-      browser()->profile(), history::DeletionTimeRange::Invalid(),
-      {history::URLResult(url_a_, base::Time())});
+      browser()->profile(),
+      DeletionInfo::ForUrls({history::URLResult(url_a_, base::Time())}, {}));
   EXPECT_EQ(1U, tab_service->entries().size());
   ASSERT_EQ(sessions::TabRestoreService::TAB,
             tab_service->entries().front()->type);

chrome/browser/media/media_engagement_service_unittest.cc

@@ -716,9 +716,9 @@ TEST_F(MediaEngagementServiceTest, HistoryExpirationIsNoOp) {
         profile(), ServiceAccessType::IMPLICIT_ACCESS);
 
     service()->OnURLsDeleted(
-        history,
-        history::DeletionInfo(history::DeletionTimeRange::Invalid(), true,
-                              history::URLRows(), std::set<GURL>()));
+        history, history::DeletionInfo(history::DeletionTimeRange::Invalid(),
+                                       true, history::URLRows(),
+                                       std::set<GURL>(), base::nullopt));
 
     // Same as above, nothing should have changed.
     ExpectScores(origin1, 7.0 / 11.0,

components/history/core/browser/expire_history_backend.cc

@@ -218,7 +218,7 @@ void ExpireHistoryBackend::DeleteURLs(const std::vector<GURL>& urls) {
   DeleteFaviconsIfPossible(&effects);
 
   BroadcastNotifications(&effects, DELETION_USER_INITIATED,
-                         DeletionTimeRange::Invalid());
+                         DeletionTimeRange::Invalid(), base::nullopt);
 }
 
 void ExpireHistoryBackend::ExpireHistoryBetween(
@@ -244,10 +244,8 @@ void ExpireHistoryBackend::ExpireHistoryBetween(
         visits.push_back(*visit);
     }
   }
-  DeletionTimeRange time_range = restrict_urls.empty()
-                                     ? DeletionTimeRange(begin_time, end_time)
-                                     : DeletionTimeRange::Invalid();
-  ExpireVisitsInternal(visits, time_range);
+  DeletionTimeRange time_range(begin_time, end_time);
+  ExpireVisitsInternal(visits, time_range, restrict_urls);
 }
 
 void ExpireHistoryBackend::ExpireHistoryForTimes(
@@ -270,12 +268,13 @@ void ExpireHistoryBackend::ExpireHistoryForTimes(
 }
 
 void ExpireHistoryBackend::ExpireVisits(const VisitVector& visits) {
-  ExpireVisitsInternal(visits, DeletionTimeRange::Invalid());
+  ExpireVisitsInternal(visits, DeletionTimeRange::Invalid(), {});
 }
 
 void ExpireHistoryBackend::ExpireVisitsInternal(
     const VisitVector& visits,
-    const DeletionTimeRange& time_range) {
+    const DeletionTimeRange& time_range,
+    const std::set<GURL>& restrict_urls) {
   if (visits.empty())
     return;
 
@@ -292,7 +291,9 @@ void ExpireHistoryBackend::ExpireVisitsInternal(
   // and we don't want to leave any evidence.
   ExpireURLsForVisits(visits_and_redirects, &effects);
   DeleteFaviconsIfPossible(&effects);
-  BroadcastNotifications(&effects, DELETION_USER_INITIATED, time_range);
+  BroadcastNotifications(
+      &effects, DELETION_USER_INITIATED, time_range,
+      restrict_urls.empty() ? base::Optional<std::set<GURL>>() : restrict_urls);
 
   // Pick up any bits possibly left over.
   ParanoidExpireHistory();
@@ -377,14 +378,15 @@ void ExpireHistoryBackend::DeleteFaviconsIfPossible(DeleteEffects* effects) {
 void ExpireHistoryBackend::BroadcastNotifications(
     DeleteEffects* effects,
     DeletionType type,
-    const DeletionTimeRange& time_range) {
+    const DeletionTimeRange& time_range,
+    base::Optional<std::set<GURL>> restrict_urls) {
   if (!effects->modified_urls.empty()) {
     notifier_->NotifyURLsModified(effects->modified_urls);
   }
   if (!effects->deleted_urls.empty() || time_range.IsValid()) {
     notifier_->NotifyURLsDeleted(DeletionInfo(
         time_range, type == DELETION_EXPIRED, std::move(effects->deleted_urls),
-        std::move(effects->deleted_favicons)));
+        std::move(effects->deleted_favicons), std::move(restrict_urls)));
   }
 }
 
@@ -611,7 +613,7 @@ void ExpireHistoryBackend::ClearOldOnDemandFaviconsIfPossible(
   }
 
   BroadcastNotifications(&effects, DELETION_EXPIRED,
-                         DeletionTimeRange::Invalid());
+                         DeletionTimeRange::Invalid(), base::nullopt);
 }
 
 bool ExpireHistoryBackend::ExpireSomeOldHistory(
@@ -636,7 +638,7 @@ bool ExpireHistoryBackend::ExpireSomeOldHistory(
   DeleteFaviconsIfPossible(&deleted_effects);
 
   BroadcastNotifications(&deleted_effects, DELETION_EXPIRED,
-                         DeletionTimeRange::Invalid());
+                         DeletionTimeRange::Invalid(), base::nullopt);
 
   return more_to_expire;
 }

components/history/core/browser/expire_history_backend.h

@@ -202,7 +202,8 @@ class ExpireHistoryBackend {
   void ExpireURLsForVisits(const VisitVector& visits, DeleteEffects* effects);
 
   void ExpireVisitsInternal(const VisitVector& visits,
-                            const DeletionTimeRange& time_range);
+                            const DeletionTimeRange& time_range,
+                            const std::set<GURL>& restrict_urls);
 
   // Deletes the favicons listed in |effects->affected_favicons| if they are
   // unsued. Fails silently (we don't care about favicons so much, so don't want
@@ -223,7 +224,8 @@ class ExpireHistoryBackend {
   // Broadcasts URL modified and deleted notifications.
   void BroadcastNotifications(DeleteEffects* effects,
                               DeletionType type,
-                              const DeletionTimeRange& time_range);
+                              const DeletionTimeRange& time_range,
+                              base::Optional<std::set<GURL>> restrict_urls);
 
   // Schedules a call to DoExpireIteration.
   void ScheduleExpire();

components/history/core/browser/expire_history_backend_unittest.cc

@@ -859,9 +859,12 @@ TEST_F(ExpireHistoryTest, FlushRecentURLsUnstarredRestricted) {
   ASSERT_EQ(1U, visits.size());
 
   // This should delete the last two visits.
-  std::set<GURL> restrict_urls;
-  restrict_urls.insert(url_row1.url());
+  std::set<GURL> restrict_urls = {url_row1.url()};
   expirer_.ExpireHistoryBetween(restrict_urls, visit_times[2], base::Time());
+  EXPECT_EQ(GetLastDeletionInfo()->time_range().begin(), visit_times[2]);
+  EXPECT_EQ(GetLastDeletionInfo()->time_range().end(), base::Time());
+  EXPECT_EQ(GetLastDeletionInfo()->deleted_rows().size(), 0U);
+  EXPECT_EQ(GetLastDeletionInfo()->restrict_urls()->size(), 1U);
 
   // Verify that the middle URL had its last visit deleted only.
   visits.clear();

components/history/core/browser/history_types.cc

@@ -374,26 +374,34 @@ bool DeletionTimeRange::IsAllTime() const {
 
 // static
 DeletionInfo DeletionInfo::ForAllHistory() {
-  return DeletionInfo(DeletionTimeRange::AllTime(), false, {}, {});
+  return DeletionInfo(DeletionTimeRange::AllTime(), false, {}, {},
+                      base::nullopt);
 }
 
 // static
 DeletionInfo DeletionInfo::ForUrls(URLRows deleted_rows,
                                    std::set<GURL> favicon_urls) {
   return DeletionInfo(DeletionTimeRange::Invalid(), false,
-                      std::move(deleted_rows), std::move(favicon_urls));
+                      std::move(deleted_rows), std::move(favicon_urls),
+                      base::nullopt);
 }
 
 DeletionInfo::DeletionInfo(const DeletionTimeRange& time_range,
                            bool is_from_expiration,
                            URLRows deleted_rows,
-                           std::set<GURL> favicon_urls)
+                           std::set<GURL> favicon_urls,
+                           base::Optional<std::set<GURL>> restrict_urls)
     : time_range_(time_range),
       is_from_expiration_(is_from_expiration),
       deleted_rows_(std::move(deleted_rows)),
-      favicon_urls_(std::move(favicon_urls)) {
-  DCHECK(!time_range.IsAllTime() || deleted_rows.empty());
-}
+      favicon_urls_(std::move(favicon_urls)),
+      restrict_urls_(std::move(restrict_urls)) {
+  // If time_range is all time or invalid, restrict_urls should be empty.
+  DCHECK(!time_range_.IsAllTime() || !restrict_urls_.has_value());
+  DCHECK(time_range_.IsValid() || !restrict_urls_.has_value());
+  // If restrict_urls_ is defined, it should be non-empty.
+  DCHECK(!restrict_urls_.has_value() || !restrict_urls_->empty());
+};
 
 DeletionInfo::~DeletionInfo() = default;
 

components/history/core/browser/history_types.h

@@ -17,6 +17,7 @@
 #include "base/containers/stack_container.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "components/favicon_base/favicon_types.h"
@@ -641,7 +642,9 @@ class DeletionInfo {
   DeletionInfo(const DeletionTimeRange& time_range,
                bool is_from_expiration,
                URLRows deleted_rows,
-               std::set<GURL> favicon_urls);
+               std::set<GURL> favicon_urls,
+               base::Optional<std::set<GURL>> restrict_urls);
+
   ~DeletionInfo();
   // Move-only because of potentially large containers.
   DeletionInfo(DeletionInfo&& other) noexcept;
@@ -651,10 +654,15 @@ class DeletionInfo {
   //  and |favicon_urls()| are undefined.
   bool IsAllHistory() const { return time_range_.IsAllTime(); }
 
-  // If time_range.IsValid() is true, all URLs between time_range.begin()
-  // and time_range.end() have been removed.
+  // If time_range.IsValid() is true, |restrict_urls| (or all URLs if empty)
+  // between time_range.begin() and time_range.end() have been removed.
   const DeletionTimeRange& time_range() const { return time_range_; }
 
+  // Restricts deletions within |time_range()|.
+  const base::Optional<std::set<GURL>>& restrict_urls() const {
+    return restrict_urls_;
+  }
+
   // Returns true, if the URL deletion is due to expiration.
   bool is_from_expiration() const { return is_from_expiration_; }
 
@@ -685,6 +693,7 @@ class DeletionInfo {
   bool is_from_expiration_;
   URLRows deleted_rows_;
   std::set<GURL> favicon_urls_;
+  base::Optional<std::set<GURL>> restrict_urls_;
   OriginCountAndLastVisitMap deleted_urls_origin_map_;
 
   DISALLOW_COPY_AND_ASSIGN(DeletionInfo);