Fix grouped history query range for months.

chrome/browser/ui/webui/browsing_history_handler.cc

 // Copyright 2015 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/ui/webui/browsing_history_handler.h"
 
 #include <stddef.h>
 
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/i18n/rtl.h"
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/favicon/fallback_icon_service_factory.h"
 #include "chrome/browser/favicon/large_icon_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/history_utils.h"
 #include "chrome/browser/history/web_history_service_factory.h"
@@ skipping 67 matching lines @@
   NUM_WEB_HISTORY_QUERY_BUCKETS
 };
 
 // Identifiers for the type of device from which a history entry originated.
 static const char kDeviceTypeLaptop[] = "laptop";
 static const char kDeviceTypePhone[] = "phone";
 static const char kDeviceTypeTablet[] = "tablet";
 
 // Returns a localized version of |visit_time| including a relative
 // indicator (e.g. today, yesterday).
-base::string16 GetRelativeDateLocalized(const base::Time& visit_time) {
-  base::Time midnight = base::Time::Now().LocalMidnight();
+base::string16 GetRelativeDateLocalized(base::Clock* clock,
+                                        const base::Time& visit_time) {
+  base::Time midnight = clock->Now().LocalMidnight();
   base::string16 date_str = ui::TimeFormat::RelativeDate(visit_time, &midnight);
   if (date_str.empty()) {
     date_str = base::TimeFormatFriendlyDate(visit_time);
   } else {
     date_str = l10n_util::GetStringFUTF16(
         IDS_HISTORY_DATE_WITH_RELATIVE_TIME,
         date_str,
         base::TimeFormatFriendlyDate(visit_time));
   }
   return date_str;
@@ skipping 50 matching lines @@
 void RecordMetricsForNoticeAboutOtherFormsOfBrowsingHistory(bool shown) {
   UMA_HISTOGRAM_BOOLEAN(
       "History.ShownHeaderAboutOtherFormsOfBrowsingHistory",
       shown);
 }
 
 }  // namespace
 
 BrowsingHistoryHandler::HistoryEntry::HistoryEntry(
     BrowsingHistoryHandler::HistoryEntry::EntryType entry_type,
-    const GURL& url, const base::string16& title, base::Time time,
-    const std::string& client_id, bool is_search_result,
-    const base::string16& snippet, bool blocked_visit) {
+    const GURL& url,
+    const base::string16& title,
+    base::Time time,
+    const std::string& client_id,
+    bool is_search_result,
+    const base::string16& snippet,
+    bool blocked_visit,
+    base::Clock* clock) {
   this->entry_type = entry_type;
   this->url = url;
   this->title = title;
   this->time = time;
   this->client_id = client_id;
   all_timestamps.insert(time.ToInternalValue());
   this->is_search_result = is_search_result;
   this->snippet = snippet;
   this->blocked_visit = blocked_visit;
+  this->clock = clock;
 }
 
 BrowsingHistoryHandler::HistoryEntry::HistoryEntry()
     : entry_type(EMPTY_ENTRY), is_search_result(false), blocked_visit(false) {
 }
 
 BrowsingHistoryHandler::HistoryEntry::HistoryEntry(const HistoryEntry& other) =
     default;
 
 BrowsingHistoryHandler::HistoryEntry::~HistoryEntry() {
@@ skipping 70 matching lines @@
   base::string16 date_time_of_day;
   bool is_blocked_visit = false;
   int host_filtering_behavior = -1;
 
   // Only pass in the strings we need (search results need a shortdate
   // and snippet, browse results need day and time information). Makes sure that
   // values of result are never undefined
   if (is_search_result) {
     snippet_string = snippet;
   } else {
-    base::Time midnight = base::Time::Now().LocalMidnight();
+    base::Time midnight = clock->Now().LocalMidnight();
     base::string16 date_str = ui::TimeFormat::RelativeDate(time, &midnight);
     if (date_str.empty()) {
       date_str = base::TimeFormatFriendlyDate(time);
     } else {
       date_str = l10n_util::GetStringFUTF16(
           IDS_HISTORY_DATE_WITH_RELATIVE_TIME,
           date_str,
           base::TimeFormatFriendlyDate(time));
     }
     date_relative_day = date_str;
@@ skipping 34 matching lines @@
   return entry1.time > entry2.time;
 }
 
 BrowsingHistoryHandler::BrowsingHistoryHandler()
     : has_pending_delete_request_(false),
       history_service_observer_(this),
       web_history_service_observer_(this),
       sync_service_observer_(this),
       has_synced_results_(false),
       has_other_forms_of_browsing_history_(false),
+      clock_(new base::DefaultClock()),
       weak_factory_(this) {}
 
 BrowsingHistoryHandler::~BrowsingHistoryHandler() {
   query_task_tracker_.TryCancelAll();
   web_history_request_.reset();
 }
 
 void BrowsingHistoryHandler::RegisterMessages() {
   // Create our favicon data source.
   Profile* profile = Profile::FromWebUI(web_ui());
@@ skipping 195 matching lines @@
     web_ui()->CallJavascriptFunctionUnsafe("deleteFailed");
     return;
   }
 
   history::HistoryService* history_service =
       HistoryServiceFactory::GetForProfile(profile,
                                            ServiceAccessType::EXPLICIT_ACCESS);
   history::WebHistoryService* web_history =
       WebHistoryServiceFactory::GetForProfile(profile);
 
-  base::Time now = base::Time::Now();
+  base::Time now = clock_->Now();
   std::vector<history::ExpireHistoryArgs> expire_list;
   expire_list.reserve(args->GetSize());
 
   DCHECK(urls_to_be_deleted_.empty());
   for (base::ListValue::const_iterator it = args->begin();
        it != args->end(); ++it) {
     base::DictionaryValue* deletion = NULL;
     base::string16 url;
     base::ListValue* timestamps = NULL;
 
@@ skipping 152 matching lines @@
 void BrowsingHistoryHandler::QueryComplete(
     const base::string16& search_text,
     const history::QueryOptions& options,
     history::QueryResults* results) {
   DCHECK_EQ(0U, query_results_.size());
   query_results_.reserve(results->size());
 
   for (size_t i = 0; i < results->size(); ++i) {
     history::URLResult const &page = (*results)[i];
     // TODO(dubroy): Use sane time (crbug.com/146090) here when it's ready.
-    query_results_.push_back(
-        HistoryEntry(
-            HistoryEntry::LOCAL_ENTRY,
-            page.url(),
-            page.title(),
-            page.visit_time(),
-            std::string(),
-            !search_text.empty(),
-            page.snippet().text(),
-            page.blocked_visit()));
+    query_results_.push_back(HistoryEntry(
+        HistoryEntry::LOCAL_ENTRY, page.url(), page.title(), page.visit_time(),
+        std::string(), !search_text.empty(), page.snippet().text(),
+        page.blocked_visit(), clock_.get()));
   }
 
   // The items which are to be written into results_info_value_ are also
   // described in chrome/browser/resources/history/history.js in @typedef for
   // HistoryQuery. Please update it whenever you add or remove any keys in
   // results_info_value_.
   results_info_value_.SetString("term", search_text);
   results_info_value_.SetBoolean("finished", results->reached_beginning());
 
   // Add the specific dates that were searched to display them.
   // TODO(sergiu): Put today if the start is in the future.
-  results_info_value_.SetString("queryStartTime",
-                                GetRelativeDateLocalized(options.begin_time));
+  results_info_value_.SetString(
+      "queryStartTime",
+      GetRelativeDateLocalized(clock_.get(), options.begin_time));
   if (!options.end_time.is_null()) {
-    results_info_value_.SetString("queryEndTime",
-        GetRelativeDateLocalized(options.end_time -
-                                 base::TimeDelta::FromDays(1)));
+    results_info_value_.SetString(
+        "queryEndTime",
+        GetRelativeDateLocalized(
+            clock_.get(), options.end_time - base::TimeDelta::FromDays(1)));
   } else {
-    results_info_value_.SetString("queryEndTime",
-        GetRelativeDateLocalized(base::Time::Now()));
+    results_info_value_.SetString(
+        "queryEndTime", GetRelativeDateLocalized(clock_.get(), clock_->Now()));
   }
   if (!web_history_timer_.IsRunning())
     ReturnResultsToFrontEnd();
 }
 
 void BrowsingHistoryHandler::ReturnResultsToFrontEnd() {
   Profile* profile = Profile::FromWebUI(web_ui());
   BookmarkModel* bookmark_model =
       BookmarkModelFactory::GetForBrowserContext(profile);
   SupervisedUserService* supervised_user_service = NULL;
@@ skipping 114 matching lines @@
         }
         // The timestamp on the server is a Unix time.
         base::Time time = base::Time::UnixEpoch() +
             base::TimeDelta::FromMicroseconds(timestamp_usec);
 
         // Get the ID of the client that this visit came from.
         std::string client_id;
         id->GetString("client_id", &client_id);
 
         web_history_query_results_.push_back(
-            HistoryEntry(
-                HistoryEntry::REMOTE_ENTRY,
-                gurl,
-                title,
-                time,
-                client_id,
-                !search_text.empty(),
-                base::string16(),
-                /* blocked_visit */ false));
+            HistoryEntry(HistoryEntry::REMOTE_ENTRY, gurl, title, time,
+                         client_id, !search_text.empty(), base::string16(),
+                         /* blocked_visit */ false, clock_.get()));
       }
     }
   }
   has_synced_results_ = results_value != nullptr;
   results_info_value_.SetBoolean("hasSyncedResults", has_synced_results_);
   if (!query_task_tracker_.HasTrackedTasks())
     ReturnResultsToFrontEnd();
 }
 
 void BrowsingHistoryHandler::OtherFormsOfBrowsingHistoryQueryComplete(
@@ skipping 22 matching lines @@
   // TODO(dubroy): Should we handle failure somehow? Delete directives will
   // ensure that the visits are eventually deleted, so maybe it's not necessary.
   if (!delete_task_tracker_.HasTrackedTasks())
     RemoveComplete();
 }
 
 void BrowsingHistoryHandler::SetQueryTimeInWeeks(
     int offset, history::QueryOptions* options) {
   // LocalMidnight returns the beginning of the current day so get the
   // beginning of the next one.
-  base::Time midnight = base::Time::Now().LocalMidnight() +
-                              base::TimeDelta::FromDays(1);
+  base::Time midnight =
+      clock_->Now().LocalMidnight() + base::TimeDelta::FromDays(1);
   options->end_time = midnight -
       base::TimeDelta::FromDays(7 * offset);
   options->begin_time = midnight -
       base::TimeDelta::FromDays(7 * (offset + 1));
 }
 
 void BrowsingHistoryHandler::SetQueryTimeInMonths(
     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);
+  clock_->Now().LocalMidnight().LocalExplode(&exploded);
   exploded.day_of_month = 1;
 
   if (offset == 0) {
     if (!base::Time::FromLocalExploded(exploded, &options->begin_time)) {
       // TODO(maksims): implement errors handling here.
       NOTIMPLEMENTED();
     }
 
     // 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();
   } else {
     // Go back |offset| months in the past. The end time is not inclusive, so
     // use the first day of the |offset| - 1 and |offset| months (e.g. for
     // the last month, |offset| = 1, use the first days of the last month and
     // the current month.
     exploded.month -= offset - 1;
     // Set the correct year.
     NormalizeMonths(&exploded);
-    if (!base::Time::FromLocalExploded(exploded, &options->begin_time)) {
+    if (!base::Time::FromLocalExploded(exploded, &options->end_time)) {

[Dan Beam, 2016/11/10 03:31:34]

this is the functional fix, right?

[calamity, 2016/11/10 07:16:27]

Yep, nestled deep amongst all the clock nonsense.

       // TODO(maksims): implement errors handling here.
       NOTIMPLEMENTED();
     }
 
     exploded.month -= 1;
     // Set the correct year
     NormalizeMonths(&exploded);
     if (!base::Time::FromLocalExploded(exploded, &options->begin_time)) {
       // TODO(maksims): implement errors handling here.
       NOTIMPLEMENTED();
@@ skipping 20 matching lines @@
     bool expired,
     const history::URLRows& deleted_rows,
     const std::set<GURL>& favicon_urls) {
   if (all_history || DeletionsDiffer(deleted_rows, urls_to_be_deleted_))
     web_ui()->CallJavascriptFunctionUnsafe("historyDeleted");
 }
 
 void BrowsingHistoryHandler::OnWebHistoryDeleted() {
   web_ui()->CallJavascriptFunctionUnsafe("historyDeleted");
 }