Use heuristic to work out when to prompt for app install banners.

chrome/browser/banners/app_banner_settings_helper_unittest.cc

Index: chrome/browser/banners/app_banner_settings_helper_unittest.cc
diff --git a/chrome/browser/banners/app_banner_settings_helper_unittest.cc b/chrome/browser/banners/app_banner_settings_helper_unittest.cc
index 917dbec3d6891b348fda0e587accf85e89c29598..17b849e3a0e3cf635fead54c9f57d2fcd9a2e3cf 100644
--- a/chrome/browser/banners/app_banner_settings_helper_unittest.cc
+++ b/chrome/browser/banners/app_banner_settings_helper_unittest.cc
@@ -12,6 +12,20 @@ namespace {
 const char kTestURL[] = "http://www.google.com";
 const char kTestPackageName[] = "test.package";
 
+base::Time GetReferenceTime() {
+  base::Time::Exploded exploded_reference_time;
+  exploded_reference_time.year = 2015;
+  exploded_reference_time.month = 1;
+  exploded_reference_time.day_of_month = 30;
+  exploded_reference_time.day_of_week = 5;
+  exploded_reference_time.hour = 11;
+  exploded_reference_time.minute = 0;
+  exploded_reference_time.second = 0;
+  exploded_reference_time.millisecond = 0;
+
+  return base::Time::FromLocalExploded(exploded_reference_time);
+}
+
 bool IsWithinDay(base::Time time1, base::Time time2) {
   return time1 - time2 < base::TimeDelta::FromDays(1) ||
          time2 - time1 < base::TimeDelta::FromDays(1);
@@ -45,26 +59,16 @@ TEST_F(AppBannerSettingsHelperTest, CouldShowEvents) {
                                                         kTestPackageName);
   EXPECT_TRUE(events.empty());
 
-  base::Time::Exploded exploded_reference_time;
-  exploded_reference_time.year = 2015;
-  exploded_reference_time.month = 1;
-  exploded_reference_time.day_of_month = 30;
-  exploded_reference_time.day_of_week = 5;
-  exploded_reference_time.hour = 11;
-  exploded_reference_time.minute = 0;
-  exploded_reference_time.second = 0;
-  exploded_reference_time.millisecond = 0;
-
-  base::Time reference_time =
-      base::Time::FromLocalExploded(exploded_reference_time);
+  base::Time reference_time = GetReferenceTime();
   base::Time same_day = reference_time + base::TimeDelta::FromHours(2);
   base::Time three_days_prior = reference_time - base::TimeDelta::FromDays(3);
   base::Time previous_fortnight =
       reference_time - base::TimeDelta::FromDays(14);
 
   // Test adding the first date.
-  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
-      web_contents(), url, kTestPackageName, previous_fortnight);
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, previous_fortnight);
 
   // It should be the only date recorded.
   events = AppBannerSettingsHelper::GetCouldShowBannerEvents(
@@ -73,8 +77,9 @@ TEST_F(AppBannerSettingsHelperTest, CouldShowEvents) {
   EXPECT_TRUE(IsWithinDay(events[0], previous_fortnight));
 
   // Now add the next date.
-  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
-      web_contents(), url, kTestPackageName, three_days_prior);
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, three_days_prior);
 
   // Now there should be two days.
   events = AppBannerSettingsHelper::GetCouldShowBannerEvents(
@@ -84,8 +89,9 @@ TEST_F(AppBannerSettingsHelperTest, CouldShowEvents) {
   EXPECT_TRUE(IsWithinDay(events[1], three_days_prior));
 
   // Now add the reference date.
-  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
-      web_contents(), url, kTestPackageName, reference_time);
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, reference_time);
 
   // Now there should still be two days, but the first date should have been
   // removed.
@@ -96,8 +102,9 @@ TEST_F(AppBannerSettingsHelperTest, CouldShowEvents) {
   EXPECT_TRUE(IsWithinDay(events[1], reference_time));
 
   // Now add the the other day on the reference date.
-  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
-      web_contents(), url, kTestPackageName, same_day);
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, same_day);
 
   // Now there should still be the same two days.
   events = AppBannerSettingsHelper::GetCouldShowBannerEvents(
@@ -106,3 +113,125 @@ TEST_F(AppBannerSettingsHelperTest, CouldShowEvents) {
   EXPECT_TRUE(IsWithinDay(events[0], three_days_prior));
   EXPECT_TRUE(IsWithinDay(events[1], reference_time));
 }
+
+TEST_F(AppBannerSettingsHelperTest, SingleEvents) {
+  GURL url(kTestURL);
+  NavigateAndCommit(url);
+
+  base::Time reference_time = GetReferenceTime();
+  base::Time other_time = reference_time - base::TimeDelta::FromDays(3);
+  for (int event = AppBannerSettingsHelper::APP_BANNER_EVENT_DID_SHOW;
+       event < AppBannerSettingsHelper::APP_BANNER_EVENT_NUM_EVENTS; ++event) {
+    // Check that by default, there is no event.
+    base::Time event_time = AppBannerSettingsHelper::GetSingleBannerEvent(
+        web_contents(), url, kTestPackageName,
+        AppBannerSettingsHelper::AppBannerEvent(event));
+    EXPECT_TRUE(event_time.is_null());
+
+    // Check that a time can be recorded.
+    AppBannerSettingsHelper::RecordBannerEvent(
+        web_contents(), url, kTestPackageName,
+        AppBannerSettingsHelper::AppBannerEvent(event), reference_time);
+
+    event_time = AppBannerSettingsHelper::GetSingleBannerEvent(
+        web_contents(), url, kTestPackageName,
+        AppBannerSettingsHelper::AppBannerEvent(event));
+    EXPECT_EQ(reference_time, event_time);
+
+    // Check that another time can be recorded.
+    AppBannerSettingsHelper::RecordBannerEvent(
+        web_contents(), url, kTestPackageName,
+        AppBannerSettingsHelper::AppBannerEvent(event), other_time);
+
+    event_time = AppBannerSettingsHelper::GetSingleBannerEvent(
+        web_contents(), url, kTestPackageName,
+        AppBannerSettingsHelper::AppBannerEvent(event));
+    EXPECT_EQ(other_time, event_time);
+  }
+}
+
+TEST_F(AppBannerSettingsHelperTest, ShouldShow) {

[gone, 2015/02/05 22:21:10]

Maybe split into multiple tests?

[benwells, 2015/02/06 16:36:28]

Done.

+  GURL url(kTestURL);
+  NavigateAndCommit(url);
+
+  base::Time reference_time = GetReferenceTime();
+  base::Time one_day_ago = reference_time - base::TimeDelta::FromDays(1);
+  base::Time three_weeks_ago = reference_time - base::TimeDelta::FromDays(21);
+  base::Time four_months_ago = reference_time - base::TimeDelta::FromDays(120);
+  base::Time one_year_ago = reference_time - base::TimeDelta::FromDays(366);
+
+  // By default the banner should not be shown.
+  EXPECT_FALSE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Visit the site one, it still should not be shown.

[gone, 2015/02/05 22:21:10]

one year later?

[benwells, 2015/02/06 16:36:28]

Something like that. Fixed.

+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, one_year_ago);
+  EXPECT_FALSE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Visit the site again after a long delay, it still should not be shown.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, one_day_ago);
+  EXPECT_FALSE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Visit the site again; now it should be shown.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, reference_time);
+  EXPECT_TRUE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Block the site a long time ago. It should still be shown.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_DID_BLOCK, one_year_ago);
+  EXPECT_TRUE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Block the site more recently. Now it should not be shown.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_DID_BLOCK, four_months_ago);
+  EXPECT_FALSE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Clear the block event.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_DID_BLOCK, base::Time());
+  EXPECT_TRUE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Show the banner a long time ago. It should still be shown.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_DID_SHOW, one_year_ago);
+  EXPECT_TRUE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Show the site more recently. Now it should not be shown.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_DID_SHOW, three_weeks_ago);
+  EXPECT_FALSE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Clear the show event.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_DID_SHOW, base::Time());
+  EXPECT_TRUE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+
+  // Add the site a long time ago. It should not be shown.
+  AppBannerSettingsHelper::RecordBannerEvent(
+      web_contents(), url, kTestPackageName,
+      AppBannerSettingsHelper::APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN,
+      one_year_ago);
+  EXPECT_FALSE(AppBannerSettingsHelper::ShouldShowBanner(
+      web_contents(), url, kTestPackageName, reference_time));
+}