Instant App Banner logic.

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 5c41600cf87b7865830d9d77451f9fd045a8bcfe..2f262c2a108a404ef4131bd1b55cab9a005813e7 100644
--- a/chrome/browser/banners/app_banner_settings_helper.cc
+++ b/chrome/browser/banners/app_banner_settings_helper.cc
@@ -15,6 +15,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "chrome/browser/android/instantapps/instant_apps_settings.h"

[Ted C, 2016/09/30 18:01:34]

This class isn't android specific as of yet, so maybe we actually want to define
the instant app key in this file to avoid that?

Or we should ifdef this to android and only handle those actions for android
(also fine)

[Maria, 2016/09/30 20:35:18]

Done.

 #include "chrome/browser/banners/app_banner_manager.h"
 #include "chrome/browser/banners/app_banner_metrics.h"
 #include "chrome/browser/browser_process.h"
@@ -40,7 +41,7 @@ const size_t kMaxAppsPerSite = 3;
 const unsigned int kOldestCouldShowBannerEventInDays = 14;
 
 // Number of days that showing the banner will prevent it being seen again for.
-const unsigned int kMinimumDaysBetweenBannerShows = 60;
+const unsigned int kMinimumDaysBetweenBannerShows = 14;
 
 const unsigned int kNumberOfMinutesInADay = 1440;
 
@@ -394,10 +395,8 @@ InstallableStatusCode AppBannerSettingsHelper::ShouldShowBanner(
   base::Time added_time =
       GetSingleBannerEvent(web_contents, origin_url, package_name_or_start_url,
                            APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN);
-  if (!added_time.is_null()) {
-    banners::TrackDisplayEvent(banners::DISPLAY_EVENT_INSTALLED_PREVIOUSLY);
+  if (!added_time.is_null())
     return ALREADY_INSTALLED;
-  }
 
   base::Time blocked_time =
       GetSingleBannerEvent(web_contents, origin_url, package_name_or_start_url,
@@ -406,19 +405,15 @@ InstallableStatusCode AppBannerSettingsHelper::ShouldShowBanner(
   // Null times are in the distant past, so the delta between real times and
   // null events will always be greater than the limits.
   if (time - blocked_time <
-      base::TimeDelta::FromDays(kMinimumBannerBlockedToBannerShown)) {
-    banners::TrackDisplayEvent(banners::DISPLAY_EVENT_BLOCKED_PREVIOUSLY);
+      base::TimeDelta::FromDays(kMinimumBannerBlockedToBannerShown))
     return PREVIOUSLY_BLOCKED;
-  }
 
   base::Time shown_time =
       GetSingleBannerEvent(web_contents, origin_url, package_name_or_start_url,
                            APP_BANNER_EVENT_DID_SHOW);
   if (time - shown_time <
-      base::TimeDelta::FromDays(kMinimumDaysBetweenBannerShows)) {
-    banners::TrackDisplayEvent(banners::DISPLAY_EVENT_IGNORED_PREVIOUSLY);
+      base::TimeDelta::FromDays(kMinimumDaysBetweenBannerShows))
     return PREVIOUSLY_IGNORED;
-  }
 
   // If we have gotten this far and want to use site engagement, the banner flow
   // was triggered by the site engagement service informing the banner manager
@@ -427,7 +422,9 @@ InstallableStatusCode AppBannerSettingsHelper::ShouldShowBanner(
   // TODO(dominickn): just return true here and remove all of the following code
   // in this method when app banners have fully migrated to using site
   // engagement as a trigger condition. See crbug.com/616322.
-  if (ShouldUseSiteEngagementScore())
+  // Do not do engagement checks for instant app banners.
+  if (ShouldUseSiteEngagementScore() ||
+      package_name_or_start_url == InstantAppsSettings::kInstantAppsKey)

[Ted C, 2016/09/30 18:01:34]

when the condition is more than one line, I like to use {}s

[Maria, 2016/09/30 20:35:18]

Done.

     return NO_ERROR_DETECTED;
 
   double total_engagement = 0;
@@ -437,10 +434,8 @@ InstallableStatusCode AppBannerSettingsHelper::ShouldShowBanner(
   for (const auto& event : could_show_events)
     total_engagement += event.engagement;
 
-  if (!HasSufficientEngagement(total_engagement)) {
-    banners::TrackDisplayEvent(banners::DISPLAY_EVENT_NOT_VISITED_ENOUGH);
+  if (!HasSufficientEngagement(total_engagement))
     return INSUFFICIENT_ENGAGEMENT;
-  }
 
   return NO_ERROR_DETECTED;
 }
@@ -565,7 +560,8 @@ bool AppBannerSettingsHelper::WasLaunchedRecently(Profile* profile,
       std::string event_key(
           kBannerEventKeys[APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN]);
       double internal_time;
-      if (!value->GetDouble(event_key, &internal_time))
+      if (it.key() == InstantAppsSettings::kInstantAppsKey ||
+          !value->GetDouble(event_key, &internal_time))
         continue;
 
       base::Time added_time = base::Time::FromInternalValue(internal_time);