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

chrome/browser/banners/app_banner_settings_helper.cc

 // Copyright 2014 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/banners/app_banner_settings_helper.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 18 matching lines @@
 // Oldest could show banner event we care about, in days.
 const unsigned int kOldestCouldShowBannerEventInDays = 14;
 
 // Total site engagements where a banner could have been shown before
 // a banner will actually be triggered.
 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;
 
 // Dictionary keys to use for the events.
 const char* kBannerEventKeys[] = {
     "couldShowBannerEvents",
     "didShowBannerEvent",
     "didBlockBannerEvent",
     "didAddToHomescreenEvent",
 };
 
 // Keys to use when storing BannerEvent structs.
 const char kBannerTimeKey[] = "time";
 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;
 
 scoped_ptr<base::DictionaryValue> GetOriginDict(
     HostContentSettingsMap* settings,
     const GURL& origin_url) {
   if (!settings)
     return scoped_ptr<base::DictionaryValue>();
 
   scoped_ptr<base::Value> value = settings->GetWebsiteSetting(
@@ skipping 26 matching lines @@
   if (ui::PageTransitionCoreTypeIs(transition_type,
                                    ui::PAGE_TRANSITION_TYPED) ||
       ui::PageTransitionCoreTypeIs(transition_type,
                                    ui::PAGE_TRANSITION_GENERATED)) {
     return kDirectNavigationEngagement;
   } else {
     return kIndirectNavigationEnagagement;
   }
 }
 
+// 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(
     Profile* profile,
     const std::set<GURL>& origin_urls) {
   HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
   for (const GURL& origin_url : origin_urls) {
     ContentSettingsPattern pattern(ContentSettingsPattern::FromURL(origin_url));
     if (!pattern.IsValid())
       continue;
@@ skipping 119 matching lines @@
   double engagement = GetEventEngagement(transition_type);
 
   base::ListValue* could_show_list = nullptr;
   if (!app_dict->GetList(event_key, &could_show_list)) {
     could_show_list = new base::ListValue();
     app_dict->Set(event_key, make_scoped_ptr(could_show_list));
   }
 
   // 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)) {
       base::DictionaryValue* internal_value;
       double internal_date;
       (*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,
                                         &other_engagement) &&
               other_engagement >= engagement) {
             // This date has already been added, but with an equal or higher
             // engagement. Don't add the date again. If the conditional fails,
             // fall to the end of the loop where the existing entry is deleted.
             return;
           }
@@ skipping 161 matching lines @@
     return base::Time();
 
   return base::Time::FromInternalValue(internal_time);
 }
 
 void AppBannerSettingsHelper::SetEngagementWeights(double direct_engagement,
                                                    double indirect_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.

+}