Fix Acid3 Test 48: LINKTEST, Chromium side....

R=brettw
BUG=http://crbug.com/231
BUG=http://crubg.com/5160
TEST=ExpireHistoryTest.ArchiveSomeOldHistory
TEST=ExpireHistoryTest.ExpiringVisitsReader
TEST=VisitedLinkTest.Listener
TEST=VisitedLinkTest.Resizing
TEST=VisitedLinkRelayTest.*
TEST=VisitedLinkEventsTest.*


Review URL: http://codereview.chromium.org/113591

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19910 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 288 insertions, 39 deletions

diff --git a/chrome/browser/history/expire_history_backend.cc b/chrome/browser/history/expire_history_backend.cc
index 084fa61..7243aa3 100644
--- a/chrome/browser/history/expire_history_backend.cc
+++ b/chrome/browser/history/expire_history_backend.cc
@@ -24,6 +24,62 @@ namespace history {
 
 namespace {
 
+// The number of days by which the expiration threshold is advanced for items
+// that we want to expire early, such as those of AUTO_SUBFRAME transition type.
+const int kEarlyExpirationAdvanceDays = 30;
+
+// Reads all types of visits starting from beginning of time to the given end
+// time. This is the most general reader.
+class AllVisitsReader : public ExpiringVisitsReader {
+ public:
+  virtual bool Read(Time end_time, HistoryDatabase* db,
+                    VisitVector* visits, int max_visits) const {
+    DCHECK(db) << "must have a database to operate upon";
+    DCHECK(visits) << "visit vector has to exist in order to populate it";
+
+    db->GetAllVisitsInRange(Time(), end_time, max_visits, visits);
+    // When we got the maximum number of visits we asked for, we say there could
+    // be additional things to expire now.
+    return static_cast<int>(visits->size()) == max_visits;
+  }
+};
+
+// Reads only AUTO_SUBFRAME visits, within a computed range. The range is
+// computed as follows:
+// * |begin_time| is read from the meta table. This value is updated whenever
+//   there are no more additional visits to expire by this reader.
+// * |end_time| is advanced forward by a constant (kEarlyExpirationAdvanceDay),
+//   but not past the current time.
+class AutoSubframeVisitsReader : public ExpiringVisitsReader {
+ public:
+  virtual bool Read(Time end_time, HistoryDatabase* db,
+                    VisitVector* visits, int max_visits) const {
+    DCHECK(db) << "must have a database to operate upon";
+    DCHECK(visits) << "visit vector has to exist in order to populate it";
+
+    Time begin_time = db->GetEarlyExpirationThreshold();
+    // Advance |end_time| to expire early.
+    Time early_end_time = end_time +
+        TimeDelta::FromDays(kEarlyExpirationAdvanceDays);
+
+    // We don't want to set the early expiration threshold to a time in the
+    // future.
+    Time now = Time::Now();
+    if (early_end_time > now)
+      early_end_time = now;
+
+    db->GetVisitsInRangeForTransition(begin_time, early_end_time,
+                                      max_visits,
+                                      PageTransition::AUTO_SUBFRAME,
+                                      visits);
+    bool more = static_cast<int>(visits->size()) == max_visits;
+    if (!more)
+      db->UpdateEarlyExpirationThreshold(early_end_time);
+
+    return more;
+  }
+};
+
 // Returns true if this visit is worth archiving. Otherwise, this visit is not
 // worth saving (for example, subframe navigations and redirects) and we can
 // just delete it when it gets old.
@@ -58,7 +114,7 @@ const int kNumExpirePerIteration = 10;
 // we think there might be more items to expire. This timeout is used when the
 // last expiration found at least kNumExpirePerIteration and we want to check
 // again "soon."
-const int kExpirationDelaySec = 60;
+const int kExpirationDelaySec = 30;
 
 // The number of minutes between checking, as with kExpirationDelaySec, but
 // when we didn't find enough things to expire last time. If there was no
@@ -166,14 +222,47 @@ void ExpireHistoryBackend::ArchiveHistoryBefore(Time end_time) {
     return;
 
   // Archive as much history as possible before the given date.
-  ArchiveSomeOldHistory(end_time, std::numeric_limits<size_t>::max());
+  ArchiveSomeOldHistory(end_time, GetAllVisitsReader(),
+                        std::numeric_limits<size_t>::max());
   ParanoidExpireHistory();
 }
 
+void ExpireHistoryBackend::InitWorkQueue() {
+  DCHECK(work_queue_.empty()) << "queue has to be empty prior to init";
+
+  for (size_t i = 0; i < readers_.size(); i++)
+    work_queue_.push(readers_[i]);
+}
+
+const ExpiringVisitsReader* ExpireHistoryBackend::GetAllVisitsReader() {
+  if (!all_visits_reader_.get())
+    all_visits_reader_.reset(new AllVisitsReader());
+  return all_visits_reader_.get();
+}
+
+const ExpiringVisitsReader*
+    ExpireHistoryBackend::GetAutoSubframeVisitsReader() {
+  if (!auto_subframe_visits_reader_.get())
+    auto_subframe_visits_reader_.reset(new AutoSubframeVisitsReader());
+  return auto_subframe_visits_reader_.get();
+}
+
 void ExpireHistoryBackend::StartArchivingOldStuff(
     TimeDelta expiration_threshold) {
   expiration_threshold_ = expiration_threshold;
-  ScheduleArchive(TimeDelta::FromSeconds(kExpirationDelaySec));
+
+  // Remove all readers, just in case this was method was called before.
+  readers_.clear();
+  // For now, we explicitly add all known readers. If we come up with more
+  // reader types (in case we want to expire different types of visits in
+  // different ways), we can make it be populated by creator/owner of
+  // ExpireHistoryBackend.
+  readers_.push_back(GetAllVisitsReader());
+  readers_.push_back(GetAutoSubframeVisitsReader());
+
+  // Initialize the queue with all tasks for the first set of iterations.
+  InitWorkQueue();
+  ScheduleArchive();
 }
 
 void ExpireHistoryBackend::DeleteFaviconsIfPossible(
@@ -413,40 +502,53 @@ void ExpireHistoryBackend::ArchiveURLsAndVisits(
   }
 }
 
-void ExpireHistoryBackend::ScheduleArchive(TimeDelta delay) {
+void ExpireHistoryBackend::ScheduleArchive() {
+  TimeDelta delay;
+  if (work_queue_.empty()) {
+    // If work queue is empty, reset the work queue to contain all tasks and
+    // schedule next iteration after a longer delay.
+    InitWorkQueue();
+    delay = TimeDelta::FromMinutes(kExpirationEmptyDelayMin);
+  } else {
+    delay = TimeDelta::FromSeconds(kExpirationDelaySec);
+  }
+
   factory_.RevokeAll();
   MessageLoop::current()->PostDelayedTask(FROM_HERE, factory_.NewRunnableMethod(
           &ExpireHistoryBackend::DoArchiveIteration), delay.InMilliseconds());
 }
 
 void ExpireHistoryBackend::DoArchiveIteration() {
-  DCHECK(expiration_threshold_ != TimeDelta()) << "threshold should be set";
-  Time threshold = Time::Now() - expiration_threshold_;
+  DCHECK(!work_queue_.empty()) << "queue has to be non-empty";
 
-  if (ArchiveSomeOldHistory(threshold, kNumExpirePerIteration)) {
-    // Possibly more items to delete now, schedule it sooner to happen again.
-    ScheduleArchive(TimeDelta::FromSeconds(kExpirationDelaySec));
-  } else {
-    // If we didn't find the maximum number of items to delete, wait longer
-    // before trying to delete more later.
-    ScheduleArchive(TimeDelta::FromMinutes(kExpirationEmptyDelayMin));
-  }
+  const ExpiringVisitsReader* reader = work_queue_.front();
+  bool more_to_expire = ArchiveSomeOldHistory(GetCurrentArchiveTime(), reader,
+                                              kNumExpirePerIteration);
+
+  work_queue_.pop();
+  // If there are more items to expire, add the reader back to the queue, thus
+  // creating a new task for future iterations.
+  if (more_to_expire)
+    work_queue_.push(reader);
+
+  ScheduleArchive();
 }
 
-bool ExpireHistoryBackend::ArchiveSomeOldHistory(Time time_threshold,
-                                                 int max_visits) {
+bool ExpireHistoryBackend::ArchiveSomeOldHistory(
+    base::Time end_time,
+    const ExpiringVisitsReader* reader,
+    int max_visits) {
   if (!main_db_)
     return false;
 
-  // Get all visits up to and including the threshold. This is a little tricky
-  // because GetAllVisitsInRange's end value is non-inclusive, so we have to
-  // increment the time by one unit to get the input value to be inclusive.
-  DCHECK(!time_threshold.is_null());
-  Time effective_threshold =
-      Time::FromInternalValue(time_threshold.ToInternalValue() + 1);
+  // Add an extra time unit to given end time, because
+  // GetAllVisitsInRange, et al. queries' end value is non-inclusive.
+  Time effective_end_time =
+      Time::FromInternalValue(end_time.ToInternalValue() + 1);
+
   VisitVector affected_visits;
-  main_db_->GetAllVisitsInRange(Time(), effective_threshold, max_visits,
-                                &affected_visits);
+  bool more_to_expire = reader->Read(effective_end_time, main_db_,
+                                     &affected_visits, max_visits);
 
   // Some visits we'll delete while others we'll archive.
   VisitVector deleted_visits, archived_visits;
@@ -489,9 +591,7 @@ bool ExpireHistoryBackend::ArchiveSomeOldHistory(Time time_threshold,
   // to not do anything if nothing was deleted.
   BroadcastDeleteNotifications(&deleted_dependencies);
 
-  // When we got the maximum number of visits we asked for, we say there could
-  // be additional things to expire now.
-  return static_cast<int>(affected_visits.size()) == max_visits;
+  return more_to_expire;
 }
 
 void ExpireHistoryBackend::ParanoidExpireHistory() {
diff --git a/chrome/browser/history/expire_history_backend.h b/chrome/browser/history/expire_history_backend.h
index ed44096..0b87b28 100644
--- a/chrome/browser/history/expire_history_backend.h
+++ b/chrome/browser/history/expire_history_backend.h
@@ -5,12 +5,14 @@
 #ifndef CHROME_BROWSER_HISTORY_EXPIRE_HISTORY_BACKEND_H__
 #define CHROME_BROWSER_HISTORY_EXPIRE_HISTORY_BACKEND_H__
 
+#include <queue>
 #include <set>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/task.h"
 #include "base/time.h"
+#include "base/scoped_ptr.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/history/text_database_manager.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
@@ -36,6 +38,18 @@ class BroadcastNotificationDelegate {
                                       HistoryDetails* details_deleted) = 0;
 };
 
+// Encapsulates visit expiration criteria and type of visits to expire.
+class ExpiringVisitsReader {
+ public:
+  virtual ~ExpiringVisitsReader() {}
+  // Populates |visits| from |db|, using provided |end_time| and |max_visits|
+  // cap.
+  virtual bool Read(base::Time end_time, HistoryDatabase* db,
+                    VisitVector* visits, int max_visits) const = 0;
+};
+
+typedef std::vector<const ExpiringVisitsReader*> ExpiringVisitsReaders;
+
 // Helper component to HistoryBackend that manages expiration and deleting of
 // history, as well as moving data from the main database to the archived
 // database as it gets old.
@@ -86,6 +100,7 @@ class ExpireHistoryBackend {
   FRIEND_TEST(ExpireHistoryTest, DeleteTextIndexForURL);
   FRIEND_TEST(ExpireHistoryTest, DeleteFaviconsIfPossible);
   FRIEND_TEST(ExpireHistoryTest, ArchiveSomeOldHistory);
+  FRIEND_TEST(ExpireHistoryTest, ExpiringVisitsReader);
   friend class ::TestingProfile;
 
   struct DeleteDependencies {
@@ -207,19 +222,21 @@ class ExpireHistoryBackend {
   // Broadcast the URL deleted notification.
   void BroadcastDeleteNotifications(DeleteDependencies* dependencies);
 
-  // Schedules a call to DoArchiveIteration at the given time in the
-  // future.
-  void ScheduleArchive(base::TimeDelta delay);
+  // Schedules a call to DoArchiveIteration.
+  void ScheduleArchive();
 
-  // Calls ArchiveSomeOldHistory to expire some amount of old history, and
-  // schedules another call to happen in the future.
+  // Calls ArchiveSomeOldHistory to expire some amount of old history, according
+  // to the items in work queue, and schedules another call to happen in the
+  // future.
   void DoArchiveIteration();
 
   // Tries to expire the oldest |max_visits| visits from history that are older
   // than |time_threshold|. The return value indicates if we think there might
   // be more history to expire with the current time threshold (it does not
   // indicate success or failure).
-  bool ArchiveSomeOldHistory(base::Time time_threshold, int max_visits);
+  bool ArchiveSomeOldHistory(base::Time end_time,
+                             const ExpiringVisitsReader* reader,
+                             int max_visits);
 
   // Tries to detect possible bad history or inconsistencies in the database
   // and deletes items. For example, URLs with no visits.
@@ -229,6 +246,18 @@ class ExpireHistoryBackend {
   // NULL.
   BookmarkService* GetBookmarkService();
 
+  // Initializes periodic expiration work queue by populating it with with tasks
+  // for all known readers.
+  void InitWorkQueue();
+
+  // Returns the reader for all visits. This method is only used by the unit
+  // tests.
+  const ExpiringVisitsReader* GetAllVisitsReader();
+
+  // Returns the reader for AUTO_SUBFRAME visits. This method is only used by
+  // the unit tests.
+  const ExpiringVisitsReader* GetAutoSubframeVisitsReader();
+
   // Non-owning pointer to the notification delegate (guaranteed non-NULL).
   BroadcastNotificationDelegate* delegate_;
 
@@ -246,6 +275,21 @@ class ExpireHistoryBackend {
   // the archived database.
   base::TimeDelta expiration_threshold_;
 
+  // List of all distinct types of readers. This list is used to populate the
+  // work queue.
+  ExpiringVisitsReaders readers_;
+
+  // Work queue for periodic expiration tasks, used by DoArchiveIteration() to
+  // determine what to do at an iteration, as well as populate it for future
+  // iterations.
+  std::queue<const ExpiringVisitsReader*> work_queue_;
+
+  // Readers for various types of visits.
+  // TODO(dglazkov): If you are adding another one, please consider reorganizing
+  // into a map.
+  scoped_ptr<ExpiringVisitsReader> all_visits_reader_;
+  scoped_ptr<ExpiringVisitsReader> auto_subframe_visits_reader_;
+
   // The BookmarkService; may be null. This is owned by the Profile.
   //
   // Use GetBookmarkService to access this, which makes sure the service is
diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc
index 1734499..08cdf38 100644
--- a/chrome/browser/history/expire_history_backend_unittest.cc
+++ b/chrome/browser/history/expire_history_backend_unittest.cc
@@ -695,18 +695,49 @@ TEST_F(ExpireHistoryTest, ArchiveSomeOldHistory) {
   URLID url_ids[3];
   Time visit_times[4];
   AddExampleData(url_ids, visit_times);
+  const ExpiringVisitsReader* reader = expirer_.GetAllVisitsReader();
 
   // Deleting a time range with no URLs should return false (nothing found).
   EXPECT_FALSE(expirer_.ArchiveSomeOldHistory(
-      visit_times[0] - TimeDelta::FromDays(100), 1));
+      visit_times[0] - TimeDelta::FromDays(100), reader, 1));
 
   // Deleting a time range with not up the the max results should also return
   // false (there will only be one visit deleted in this range).
-  EXPECT_FALSE(expirer_.ArchiveSomeOldHistory(visit_times[0], 2));
+  EXPECT_FALSE(expirer_.ArchiveSomeOldHistory(visit_times[0], reader, 2));
 
   // Deleting a time range with the max number of results should return true
   // (max deleted).
-  EXPECT_TRUE(expirer_.ArchiveSomeOldHistory(visit_times[2], 1));
+  EXPECT_TRUE(expirer_.ArchiveSomeOldHistory(visit_times[2], reader, 1));
+}
+
+TEST_F(ExpireHistoryTest, ExpiringVisitsReader) {
+  URLID url_ids[3];
+  Time visit_times[4];
+  AddExampleData(url_ids, visit_times);
+
+  const ExpiringVisitsReader* all = expirer_.GetAllVisitsReader();
+  const ExpiringVisitsReader* auto_subframes =
+      expirer_.GetAutoSubframeVisitsReader();
+
+  VisitVector visits;
+  Time now = Time::Now();
+
+  // Verify that the early expiration threshold, stored in the meta table is
+  // initialized.
+  EXPECT_TRUE(main_db_->GetEarlyExpirationThreshold() ==
+      Time::FromInternalValue(1L));
+
+  // First, attempt reading AUTO_SUBFRAME visits. We should get none.
+  EXPECT_FALSE(auto_subframes->Read(now, main_db_.get(), &visits, 1));
+  EXPECT_EQ(0U, visits.size());
+
+  // Verify that the early expiration threshold was updated, since there are no
+  // AUTO_SUBFRAME visits in the given time range.
+  EXPECT_TRUE(now <= main_db_->GetEarlyExpirationThreshold());
+
+  // Now, read all visits and verify that there's at least one.
+  EXPECT_TRUE(all->Read(now, main_db_.get(), &visits, 1));
+  EXPECT_EQ(1U, visits.size());
 }
 
 // TODO(brettw) add some visits with no URL to make sure everything is updated
diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index aa004b4..cbd6754 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -273,8 +273,7 @@ void HistoryService::AddPage(const GURL& url,
   // large part of history (think iframes for ads) and we never display them in
   // history UI. We will still add manual subframes, which are ones the user
   // has clicked on to get.
-  if (!CanAddURL(url) || PageTransition::StripQualifier(transition) ==
-                         PageTransition::AUTO_SUBFRAME)
+  if (!CanAddURL(url))
     return;
 
   // Add link & all redirects to visited link list.
diff --git a/chrome/browser/history/history_database.cc b/chrome/browser/history/history_database.cc
index 7b198fc..5a929b1 100644
--- a/chrome/browser/history/history_database.cc
+++ b/chrome/browser/history/history_database.cc
@@ -10,6 +10,8 @@
 #include "base/string_util.h"
 #include "chrome/common/sqlite_utils.h"
 
+using base::Time;
+
 namespace history {
 
 namespace {
@@ -17,6 +19,7 @@ namespace {
 // Current version number.
 static const int kCurrentVersionNumber = 16;
 static const int kCompatibleVersionNumber = 16;
+static const char kEarlyExpirationThresholdKey[] = "early_expiration_threshold";
 
 }  // namespace
 
@@ -171,6 +174,27 @@ SegmentID HistoryDatabase::GetSegmentID(VisitID visit_id) {
   return 0;
 }
 
+Time HistoryDatabase::GetEarlyExpirationThreshold() {
+  if (!cached_early_expiration_threshold_.is_null())
+    return cached_early_expiration_threshold_;
+
+  int64 threshold;
+  if (!meta_table_.GetValue(kEarlyExpirationThresholdKey, &threshold)) {
+    // Set to a very early non-zero time, so it's before all history, but not
+    // zero to avoid re-retrieval.
+    threshold = 1L;
+  }
+
+  cached_early_expiration_threshold_ = Time::FromInternalValue(threshold);
+  return cached_early_expiration_threshold_;
+}
+
+void HistoryDatabase::UpdateEarlyExpirationThreshold(Time threshold) {
+  meta_table_.SetValue(kEarlyExpirationThresholdKey,
+                       threshold.ToInternalValue());
+  cached_early_expiration_threshold_ = threshold;
+}
+
 sqlite3* HistoryDatabase::GetDB() {
   return db_;
 }
diff --git a/chrome/browser/history/history_database.h b/chrome/browser/history/history_database.h
index 61a27d7..55ca0f8 100644
--- a/chrome/browser/history/history_database.h
+++ b/chrome/browser/history/history_database.h
@@ -121,6 +121,12 @@ class HistoryDatabase : public DownloadDatabase,
   // visit id wasn't found.
   SegmentID GetSegmentID(VisitID visit_id);
 
+  // Retrieves/Updates early expiration threshold, which specifies the earliest
+  // known point in history that may possibly to contain visits suitable for
+  // early expiration (AUTO_SUBFRAMES).
+  virtual base::Time GetEarlyExpirationThreshold();
+  virtual void UpdateEarlyExpirationThreshold(base::Time threshold);
+
   // Drops the starred table and star_id from urls.
   bool MigrateFromVersion15ToVersion16();
 
@@ -156,6 +162,7 @@ class HistoryDatabase : public DownloadDatabase,
   SqliteStatementCache* statement_cache_;
 
   MetaTableHelper meta_table_;
+  base::Time cached_early_expiration_threshold_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryDatabase);
 };
diff --git a/chrome/browser/history/redirect_uitest.cc b/chrome/browser/history/redirect_uitest.cc
index 7f3e7be..30b63b2 100644
--- a/chrome/browser/history/redirect_uitest.cc
+++ b/chrome/browser/history/redirect_uitest.cc
@@ -111,7 +111,7 @@ TEST_F(RedirectTest, ClientEmptyReferer) {
       break;
   }
 
-  EXPECT_EQ(1U, redirects.size());
+  ASSERT_EQ(1U, redirects.size());
   EXPECT_EQ(final_url.spec(), redirects[0].spec());
 }
 
diff --git a/chrome/browser/history/visit_database.cc b/chrome/browser/history/visit_database.cc
index 7ce3e62..60d4c5b 100644
--- a/chrome/browser/history/visit_database.cc
+++ b/chrome/browser/history/visit_database.cc
@@ -213,6 +213,35 @@ void VisitDatabase::GetAllVisitsInRange(Time begin_time, Time end_time,
   FillVisitVector(*statement, visits);
 }
 
+void VisitDatabase::GetVisitsInRangeForTransition(
+    Time begin_time,
+    Time end_time,
+    int max_results,
+    PageTransition::Type transition,
+    VisitVector* visits) {
+  DCHECK(visits);
+  visits->clear();
+
+  SQLITE_UNIQUE_STATEMENT(statement, GetStatementCache(),
+      "SELECT" HISTORY_VISIT_ROW_FIELDS "FROM visits "
+      "WHERE visit_time >= ? AND visit_time < ? "
+      "AND (transition & ?) == ?"
+      "ORDER BY visit_time LIMIT ?");
+  if (!statement.is_valid())
+    return;
+
+  // See GetVisibleVisitsInRange for more info on how these times are bound.
+  int64 end = end_time.ToInternalValue();
+  statement->bind_int64(0, begin_time.ToInternalValue());
+  statement->bind_int64(1, end ? end : std::numeric_limits<int64>::max());
+  statement->bind_int(2, PageTransition::CORE_MASK);
+  statement->bind_int(3, transition);
+  statement->bind_int64(4,
+      max_results ? max_results : std::numeric_limits<int64>::max());
+
+  FillVisitVector(*statement, visits);
+}
+
 void VisitDatabase::GetVisibleVisitsInRange(Time begin_time, Time end_time,
                                             bool most_recent_visit_only,
                                             int max_count,
diff --git a/chrome/browser/history/visit_database.h b/chrome/browser/history/visit_database.h
index 2f73f90..9fa4601 100644
--- a/chrome/browser/history/visit_database.h
+++ b/chrome/browser/history/visit_database.h
@@ -64,6 +64,21 @@ class VisitDatabase {
   void GetAllVisitsInRange(base::Time begin_time, base::Time end_time,
                            int max_results, VisitVector* visits);
 
+  // Fills all visits with specified transition in the time range [begin, end)
+  // to the given vector. Either time can be is_null(), in which case the times
+  // in that direction are unbounded.
+  //
+  // If |max_results| is non-zero, up to that many results will be returned. If
+  // there are more results than that, the oldest ones will be returned. (This
+  // is used for history expiration.)
+  //
+  // The results will be in increasing order of date.
+  void GetVisitsInRangeForTransition(base::Time begin_time,
+                                     base::Time end_time,
+                                     int max_results,
+                                     PageTransition::Type transition,
+                                     VisitVector* visits);
+
   // Fills all visits in the given time range into the given vector that should
   // be user-visible, which excludes things like redirects and subframes. The
   // begin time is inclusive, the end time is exclusive. Either time can be