History: Add range navigation control for grouped visits

chrome/browser/ui/webui/history_ui.cc

Index: chrome/browser/ui/webui/history_ui.cc
diff --git a/chrome/browser/ui/webui/history_ui.cc b/chrome/browser/ui/webui/history_ui.cc
index 251580733ff1536ae8a73dc0d4f2046fe445cdca..37bb17c385a705932cec2a604459e44276a29651 100644
--- a/chrome/browser/ui/webui/history_ui.cc
+++ b/chrome/browser/ui/webui/history_ui.cc
@@ -144,7 +144,7 @@ content::WebUIDataSource* CreateHistoryUIHTMLSource() {
   source->AddLocalizedString("displayfiltersites", IDS_GROUP_BY_DOMAIN_LABEL);
   source->AddLocalizedString("rangeweek", IDS_HISTORY_RANGE_WEEK);
   source->AddLocalizedString("rangemonth", IDS_HISTORY_RANGE_MONTH);
-  source->AddLocalizedString("displayfiltersites", IDS_GROUP_BY_DOMAIN_LABEL);
+  source->AddLocalizedString("rangetoday", IDS_HISTORY_RANGE_TODAY);
   source->AddLocalizedString("numbervisits", IDS_HISTORY_NUMBER_VISITS);
   source->AddBoolean("groupByDomain",
       CommandLine::ForCurrentProcess()->HasSwitch(
@@ -262,6 +262,7 @@ void BrowsingHistoryHandler::HandleQueryHistory(const ListValue* args) {
 
   // Parse the arguments from JavaScript. There are six required arguments:
   // - the text to search for (may be empty)
+  // - the offset from which the search should start in multiple of the range.
   // - the range (number) of days to search for, used only for grouped
   //   searches.
   // - the end time of the range to search (see QueryOptions.end_time)
@@ -271,43 +272,48 @@ void BrowsingHistoryHandler::HandleQueryHistory(const ListValue* args) {
   // - the maximum number of results to return (may be 0, meaning that there
   //   is no maximum).
   string16 search_text = ExtractStringValue(args);
-  int range;
-  if (!args->GetInteger(1, &range)) {
+  int offset;
+  if (!args->GetInteger(1, &offset)) {
     NOTREACHED() << "Failed to convert argument 1. ";
     return;
   }
+  int range;
+  if (!args->GetInteger(2, &range)) {
+    NOTREACHED() << "Failed to convert argument 2. ";
+    return;
+  }
+
+  double end_time;
+  if (!args->GetDouble(3, &end_time)) {
+    NOTREACHED() << "Failed to convert argument 3. ";
+    return;
+  }
+  if (end_time)
+    options.end_time = base::Time::FromJsTime(end_time);
 
   switch (range) {
     case BrowsingHistoryHandler::MONTH:
-      SetQueryTimeInMonths(&options);
+      SetQueryTimeInMonths(offset, &options);
       break;
     case BrowsingHistoryHandler::WEEK:
-      SetQueryTimeInWeeks(&options);
+      SetQueryTimeInWeeks(offset, &options);
       break;
     case BrowsingHistoryHandler::NOT_GROUPED:
       break;
   }
 
-  double end_time;
-  if (!args->GetDouble(2, &end_time)) {
-    NOTREACHED() << "Failed to convert argument 2. ";
-    return;
-  }
-  if (end_time)
-    options.end_time = base::Time::FromJsTime(end_time);
-
   const Value* cursor_value;
 
   // Get the cursor. It must be either null, or a list.
-  if (!args->Get(3, &cursor_value) ||
+  if (!args->Get(4, &cursor_value) ||
       (!cursor_value->IsType(Value::TYPE_NULL) &&
       !history::QueryCursor::FromValue(cursor_value, &options.cursor))) {
-    NOTREACHED() << "Failed to convert argument 3. ";
+    NOTREACHED() << "Failed to convert argument 4. ";
     return;
   }
 
-  if (!ExtractIntegerValueAtIndex(args, 4, &options.max_count)) {
-    NOTREACHED() << "Failed to convert argument 4.";
+  if (!ExtractIntegerValueAtIndex(args, 5, &options.max_count)) {
+    NOTREACHED() << "Failed to convert argument 5.";
     return;
   }
 
@@ -530,28 +536,54 @@ void BrowsingHistoryHandler::RemoveComplete() {
 }
 
 void BrowsingHistoryHandler::SetQueryTimeInWeeks(
-    history::QueryOptions* options) {
+    int offset, history::QueryOptions* options) {
   // LocalMidnight returns the beginning of the current day so get the
   // beginning of the next one.
   base::Time midnight_today = base::Time::Now().LocalMidnight() +
                               base::TimeDelta::FromDays(1);
-  options->end_time = midnight_today;
-  options->begin_time = midnight_today - base::TimeDelta::FromDays(7);
+  options->end_time = midnight_today -
+      base::TimeDelta::FromDays(7 * offset);
+  options->begin_time = midnight_today -
+      base::TimeDelta::FromDays(7 * (offset + 1));
 }
 
 void BrowsingHistoryHandler::SetQueryTimeInMonths(
-    history::QueryOptions* options) {
+    int offset, history::QueryOptions* options) {
   // Configure the begin point of the search to the start of the
   // current month.
   base::Time::Exploded exploded;
   base::Time::Now().LocalMidnight().LocalExplode(&exploded);
   exploded.day_of_month = 1;
-  options->begin_time = base::Time::FromLocalExploded(exploded);
 
-  // Set the end time of this first search to null (which will
-  // show results from the future, should the user's clock have
-  // been set incorrectly).
-  options->end_time = base::Time();
+  if (offset == 0) {
+    options->begin_time = base::Time::FromLocalExploded(exploded);
+
+    // Set the end time of this first search to null (which will
+    // show results from the future, should the user's clock have
+    // been set incorrectly).
+    options->end_time = base::Time();

[James Hawkins, 2013/01/28 17:48:15]

This block of code (all the green) is somewhat hairy wrt calculations and
whatnot.  I suggest refactoring to pull out into one more more well-named
functions.

[Sergiu, 2013/01/30 16:09:39]

I've refactored this making it simple, hopefully it is clearer now.

+  } else {
+    // Set the end-time of this search to the end of the month that is
+    // |offset| months before the search end point. The end time is not
+    // inclusive, so we should set it to midnight on the first day of the
+    // following month.
+    exploded.month -= offset - 1;
+    while (exploded.month < 1) {
+      exploded.month += 12;
+      exploded.year--;
+    }
+    options->end_time = base::Time::FromLocalExploded(exploded);
+
+    // Set the begin-time of the search to the start of the month
+    // that is |offset| months prior to search_start_.
+    if (exploded.month > 1) {
+      exploded.month--;
+    } else {
+      exploded.month = 12;
+      exploded.year--;
+    }
+    options->begin_time = base::Time::FromLocalExploded(exploded);
+  }
 }
 
 // Helper function for Observe that determines if there are any differences