Allow the minimum time between banner trigger visits to be varied via field trials.

chrome/browser/banners/app_banner_settings_helper.cc

Index: chrome/browser/banners/app_banner_settings_helper.cc
diff --git a/chrome/browser/banners/app_banner_settings_helper.cc b/chrome/browser/banners/app_banner_settings_helper.cc
index 3e821591163b0c21a050836abc2382cde5509079..36a26a3eabf88f9fdb0567058b2736d3f5726e40 100644
--- a/chrome/browser/banners/app_banner_settings_helper.cc
+++ b/chrome/browser/banners/app_banner_settings_helper.cc
@@ -36,6 +36,12 @@ const double kTotalEngagementToTrigger = 2;
 // Number of days that showing the banner will prevent it being seen again for.
 const unsigned int kMinimumDaysBetweenBannerShows = 60;
 
+const unsigned int kNumberOfMinutesInADay = 1440;
+
+// Number of minutes between visits that will trigger a could show banner event.
+// Defaults to the number of minutes in a day.
+unsigned int gMinimumMinutesBetweenVisits = kNumberOfMinutesInADay;
+
 // Number of days that the banner being blocked will prevent it being seen again
 // for.
 const unsigned int kMinimumBannerBlockedToBannerShown = 90;
@@ -54,8 +60,7 @@ const char kBannerEngagementKey[] = "engagement";
 
 // Engagement weight assigned to direct and indirect navigations.
 // By default, a direct navigation is a page visit via ui::PAGE_TRANSITION_TYPED
-// or ui::PAGE_TRANSITION_GENERATED. These are weighted twice the engagement of
-// all other navigations.
+// or ui::PAGE_TRANSITION_GENERATED.
 double kDirectNavigationEngagement = 1;
 double kIndirectNavigationEnagagement = 1;
 
@@ -102,6 +107,32 @@ double GetEventEngagement(ui::PageTransition transition_type) {
   }
 }
 
+// Given a time, returns that time scoped to the nearest resolution locally.
+// For example, if the resolution is one hour, then this function will return
+// the time to the closest (previous) hour in the local time zone.
+base::Time BucketTime(base::Time time, base::TimeDelta resolution) {

[benwells, 2015/08/24 06:55:52]

Nit: I think it would be simpler if this just took 'unsigned int minutes'
instead of a base::TimeDelta, as you would save a couple of conversions.

[dominickn, 2015/08/24 07:43:16]

Done.

+  // Only support resolutions smaller than or equal to one day. Enforce
+  // that resolutions divide evenly into one day. Otherwise, default to a
+  // day resolution (each time converted to midnight local time).
+  unsigned int resolution_minutes = resolution.InMinutes();
+  if (resolution_minutes == 0 || resolution_minutes > kNumberOfMinutesInADay ||
+      kNumberOfMinutesInADay % resolution_minutes != 0) {
+    return time.LocalMidnight();
+  }
+
+  // Extract the number of minutes past midnight in local time. Divide that
+  // number by the resolution size, and return the time converted to local
+  // midnight with the resulting truncated number added.
+  base::Time::Exploded exploded;
+  time.LocalExplode(&exploded);
+  int total_minutes = exploded.hour * 60 + exploded.minute;
+
+  // Use truncating integer division here.
+  return time.LocalMidnight() +
+         base::TimeDelta::FromMinutes((total_minutes / resolution_minutes) *
+                                      resolution_minutes);
+}
+
 }  // namespace
 
 void AppBannerSettingsHelper::ClearHistoryForURLs(
@@ -241,7 +272,9 @@ void AppBannerSettingsHelper::RecordBannerCouldShowEvent(
 
   // Trim any items that are older than we should care about. For comparisons
   // the times are converted to local dates.
-  base::Time date = time.LocalMidnight();
+  base::TimeDelta resolution =
+      base::TimeDelta::FromMinutes(gMinimumMinutesBetweenVisits);
+  base::Time date = BucketTime(time, resolution);
   base::ValueVector::iterator it = could_show_list->begin();
   while (it != could_show_list->end()) {
     if ((*it)->IsType(base::Value::TYPE_DICTIONARY)) {
@@ -250,8 +283,8 @@ void AppBannerSettingsHelper::RecordBannerCouldShowEvent(
       (*it)->GetAsDictionary(&internal_value);
 
       if (internal_value->GetDouble(kBannerTimeKey, &internal_date)) {
-        base::Time other_date =
-            base::Time::FromInternalValue(internal_date).LocalMidnight();
+        base::Time other_date = BucketTime(
+            base::Time::FromInternalValue(internal_date), resolution);
         if (other_date == date) {
           double other_engagement = 0;
           if (internal_value->GetDouble(kBannerEngagementKey,
@@ -433,3 +466,14 @@ void AppBannerSettingsHelper::SetEngagementWeights(double direct_engagement,
   kDirectNavigationEngagement = direct_engagement;
   kIndirectNavigationEnagagement = indirect_engagement;
 }
+
+void AppBannerSettingsHelper::SetMinimumMinutesBetweenVisits(
+    unsigned int minutes) {
+  gMinimumMinutesBetweenVisits = minutes;
+}
+
+base::Time AppBannerSettingsHelper::BucketTimeToResolution(
+    base::Time time,
+    base::TimeDelta resolution) {
+  return BucketTime(time, resolution);

[benwells, 2015/08/24 06:55:52]

Do you need a wrapper function for this? You could just move the anon version
into here and use it...

[dominickn, 2015/08/24 07:43:16]

Done.

+}