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

chrome/browser/browsing_data/browsing_data_remover_unittest.cc

 // Copyright (c) 2012 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 "chrome/browser/browsing_data/browsing_data_remover.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <list>
 #include <memory>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/guid.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_filter_builder.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_test_util.h"
@@ skipping 88 matching lines @@
 using content::BrowserThread;
 using content::StoragePartition;
 using domain_reliability::CLEAR_BEACONS;
 using domain_reliability::CLEAR_CONTEXTS;
 using domain_reliability::DomainReliabilityClearMode;
 using domain_reliability::DomainReliabilityMonitor;
 using domain_reliability::DomainReliabilityService;
 using domain_reliability::DomainReliabilityServiceFactory;
 using testing::_;
 using testing::ByRef;
+using testing::Eq;
 using testing::Invoke;
 using testing::IsEmpty;
 using testing::Matcher;
 using testing::MakeMatcher;
 using testing::MatcherInterface;
 using testing::MatchResultListener;
 using testing::Not;
 using testing::Return;
+using testing::SizeIs;
 using testing::WithArgs;
 
 namespace {
 
 const char kTestOrigin1[] = "http://host1.com:1/";
 const char kTestRegisterableDomain1[] = "host1.com";
 const char kTestOrigin2[] = "http://host2.com:1/";
 const char kTestOrigin3[] = "http://host3.com:1/";
 const char kTestRegisterableDomain3[] = "host3.com";
 const char kTestOrigin4[] = "https://host3.com:1/";
@@ skipping 2915 matching lines @@
 // Test that the remover clears bookmark meta data (normally added in a tab
 // helper).
 TEST_F(BrowsingDataRemoverTest, BookmarkLastVisitDatesGetCleared) {
   TestingProfile profile;
   profile.CreateBookmarkModel(true);
 
   bookmarks::BookmarkModel* bookmark_model =
       BookmarkModelFactory::GetForBrowserContext(&profile);
   bookmarks::test::WaitForBookmarkModelToLoad(bookmark_model);
 
+  const base::Time delete_begin =
+      base::Time::Now() - base::TimeDelta::FromDays(1);
+
   // Create a couple of bookmarks.
   bookmark_model->AddURL(bookmark_model->bookmark_bar_node(), 0,
                          base::string16(),
                          GURL("http://foo.org/desktop"));
   bookmark_model->AddURL(bookmark_model->mobile_node(), 0,
                          base::string16(),
                          GURL("http://foo.org/mobile"));
 
-  // Simulate their visits.
+  // Simulate their visits (this is using Time::Now() as timestamps).
   ntp_snippets::UpdateBookmarkOnURLVisitedInMainFrame(
       bookmark_model, GURL("http://foo.org/desktop"),
       /*is_mobile_platform=*/false);
   ntp_snippets::UpdateBookmarkOnURLVisitedInMainFrame(
       bookmark_model, GURL("http://foo.org/mobile"),
       /*is_mobile_platform=*/true);
 
+  // Add a bookmark with a visited timestamp before the deletion interval.
+  bookmarks::BookmarkNode::MetaInfoMap meta_info = {
+      {"last_visited",
+       base::Int64ToString((delete_begin - base::TimeDelta::FromSeconds(1))
+                               .ToInternalValue())}};
+  bookmark_model->AddURLWithCreationTimeAndMetaInfo(
+      bookmark_model->mobile_node(), 0, base::ASCIIToUTF16("my title"),
+      GURL("http://foo-2.org/"), delete_begin - base::TimeDelta::FromDays(1),
+      &meta_info);
+
   // There should be some recently visited bookmarks.
   EXPECT_THAT(ntp_snippets::GetRecentlyVisitedBookmarks(
                   bookmark_model, 2, base::Time::UnixEpoch(),
                   /*consider_visits_from_desktop=*/false),
               Not(IsEmpty()));
 
   // Inject the bookmark model into the remover.
   BrowsingDataRemover* remover =
       BrowsingDataRemoverFactory::GetForBrowserContext(&profile);
 
   BrowsingDataRemoverCompletionObserver completion_observer(remover);
-  remover->RemoveAndReply(BrowsingDataRemover::Unbounded(),
-                          BrowsingDataRemover::REMOVE_HISTORY,
-                          BrowsingDataHelper::ALL, &completion_observer);
+  remover->RemoveAndReply(
+      BrowsingDataRemover::TimeRange(delete_begin, base::Time::Max()),

[msramek, 2017/01/04 20:27:20]

FYI, I landed https://codereview.chromium.org/2578723002 which deprecated
BrowsingDataRemover::TimeRange and changed the interface here to take two
base::Time parameters.

Please rebase and replace this line with "delete_begin, base::Time::Max()".

+      BrowsingDataRemover::REMOVE_HISTORY, BrowsingDataHelper::ALL,
+      &completion_observer);
   completion_observer.BlockUntilCompletion();
 
-  // There should be no recently visited bookmarks.
-  EXPECT_THAT(ntp_snippets::GetRecentlyVisitedBookmarks(
-                  bookmark_model, 2, base::Time::UnixEpoch(),
-                  /*consider_visits_from_desktop=*/false),
-              IsEmpty());
-  EXPECT_THAT(ntp_snippets::GetRecentlyVisitedBookmarks(
-                  bookmark_model, 2, base::Time::UnixEpoch(),
-                  /*consider_visits_from_desktop=*/true),
-              IsEmpty());
+  // There should be only 1 recently visited bookmarks.
+  std::vector<const bookmarks::BookmarkNode*> remaining_nodes =
+      ntp_snippets::GetRecentlyVisitedBookmarks(
+          bookmark_model, 3, base::Time::UnixEpoch(),
+          /*consider_visits_from_desktop=*/true);
+  EXPECT_THAT(remaining_nodes, SizeIs(1));
+  EXPECT_THAT(remaining_nodes[0]->url().spec(), Eq("http://foo-2.org/"));
 }