Remove the app banner navigation heuristic.

chrome/browser/banners/app_banner_settings_helper.h

 // 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.
 
 #ifndef CHROME_BROWSER_BANNERS_APP_BANNER_SETTINGS_HELPER_H_
 #define CHROME_BROWSER_BANNERS_APP_BANNER_SETTINGS_HELPER_H_
 
 #include <set>
 #include <string>
-#include <vector>
 
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "chrome/browser/installable/installable_logging.h"
-#include "ui/base/page_transition_types.h"
 
 namespace content {
 class WebContents;
 }  // namespace content
 
 class GURL;
 class Profile;
 
 // Utility class to record banner events for the given package or start url.
 //
@@ skipping 22 matching lines @@
     LANGUAGE_OPTION_DEFAULT = 0,
     LANGUAGE_OPTION_MIN = LANGUAGE_OPTION_DEFAULT,
     LANGUAGE_OPTION_ADD = 1,
     LANGUAGE_OPTION_INSTALL = 2,
     LANGUAGE_OPTION_MAX = LANGUAGE_OPTION_INSTALL,
   };
 
   // TODO(mariakhomenko): Rename events to reflect that they are used in more
   // contexts now.
   enum AppBannerEvent {
     APP_BANNER_EVENT_COULD_SHOW,

[benwells, 2016/12/09 04:55:05]

Should this be renamed to something like "FIRST_COULD_SHOW"?

[dominickn, 2016/12/09 05:27:28]

I thought about that. The upside of not changing the name is that the old
couldShowEvent vector in the content setting should be overridden by the single
date, potentially saving us a bunch of data. Otherwise, it'll stay there
permanently until the user clears browsing data. I'm happy to go with your
preference here.

[benwells, 2016/12/09 05:44:55]

Right now it's really not clear what this data is. I think you should either:
- add comments so it is clear what this actually means
- change the name but leave the key as is, with a comment explaining why
- add a new event, and add code to remove the old events.

I'll leave it up to you :)

[dominickn, 2016/12/12 02:23:14]

Done.

     APP_BANNER_EVENT_DID_SHOW,
     APP_BANNER_EVENT_DID_BLOCK,
     APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN,
     APP_BANNER_EVENT_NUM_EVENTS,
   };
 
   enum AppBannerRapporMetric {
     WEB,
     NATIVE,
   };
 
   static const char kInstantAppsKey[];
 
-  // BannerEvents record the time that a site was accessed, along with an
-  // engagement weight representing the importance of the access.
-  struct BannerEvent {
-    base::Time time;
-    double engagement;
-  };
-
   // The content setting basically records a simplified subset of history.
   // For privacy reasons this needs to be cleared. The ClearHistoryForURLs
   // function removes any information from the banner content settings for the
   // given URls.
   static void ClearHistoryForURLs(Profile* profile,
                                   const std::set<GURL>& origin_urls);
 
   // Record a banner installation event, for either a WEB or NATIVE app.
   static void RecordBannerInstallEvent(
       content::WebContents* web_contents,
       const std::string& package_name_or_start_url,
       AppBannerRapporMetric rappor_metric);
 
   // Record a banner dismissal event, for either a WEB or NATIVE app.
   static void RecordBannerDismissEvent(
       content::WebContents* web_contents,
       const std::string& package_name_or_start_url,
       AppBannerRapporMetric rappor_metric);
 
-  // Record a banner event. Should not be used for could show events, as they
-  // require a transition type.
+  // Record a banner event specified by |event|.
   static void RecordBannerEvent(content::WebContents* web_contents,
                                 const GURL& origin_url,
                                 const std::string& package_name_or_start_url,
                                 AppBannerEvent event,
                                 base::Time time);
 
-  // Record a banner could show event, with a specified transition type.
-  static void RecordBannerCouldShowEvent(
-      content::WebContents* web_contents,
-      const GURL& origin_url,
-      const std::string& package_name_or_start_url,
-      base::Time time,
-      ui::PageTransition transition_type);
-
   // Determine if the banner should be shown, given the recorded events for the
   // supplied app. Returns an InstallableStatusCode indicated the reason why the
   // banner shouldn't be shown, or NO_ERROR_DETECTED if it should be shown.
   static InstallableStatusCode ShouldShowBanner(
       content::WebContents* web_contents,
       const GURL& origin_url,
       const std::string& package_name_or_start_url,
       base::Time time);
 
-  // Gets the could have been shown events that are stored for the given package
-  // or start url. This is only exposed for testing.
-  static std::vector<BannerEvent> GetCouldShowBannerEvents(
-      content::WebContents* web_contents,
-      const GURL& origin_url,
-      const std::string& package_name_or_start_url);
-
-  // Get the recorded event for an event type that only records the last event.
-  // Should not be used with APP_BANNER_EVENT_COULD_SHOW. This is only exposed
-  // for testing.
+  // Get the time that |event| was recorded, or a null time if it has not yet
+  // been recorded. Exposed for testing.
   static base::Time GetSingleBannerEvent(
       content::WebContents* web_contents,
       const GURL& origin_url,
       const std::string& package_name_or_start_url,
       AppBannerEvent event);
 
   // Returns true if |total_engagement| is sufficiently high to warrant
   // triggering a banner, or if the command-line flag to bypass engagement
   // checking is true.
   static bool HasSufficientEngagement(double total_engagement);
@@ skipping 12 matching lines @@
   // already been added to homescreen.
   static bool WasLaunchedRecently(Profile* profile,
                                   const GURL& origin_url,
                                   base::Time now);
 
   // Set the number of days which dismissing/ignoring the banner should prevent
   // a banner from showing.
   static void SetDaysAfterDismissAndIgnoreToTrigger(unsigned int dismiss_days,
                                                     unsigned int ignore_days);
 
-  // Set the engagement weights assigned to direct and indirect navigations.
-  static void SetEngagementWeights(double direct_engagement,
-                                   double indirect_engagement);
-
-  // Set the minimum number of minutes between banner visits that will
-  // trigger a could show banner event. This must be less than the
-  // number of minutes in a day, and evenly divide the number of minutes
-  // in a day.
-  static void SetMinimumMinutesBetweenVisits(unsigned int minutes);
-
   // Set the total engagement weight required to trigger a banner.
   static void SetTotalEngagementToTrigger(double total_engagement);
 
   // Resets the engagement weights, minimum minutes, and total engagement to
   // trigger to their default values.
   static void SetDefaultParameters();
 
-  // Bucket a given time to the given resolution in local time.
-  static base::Time BucketTimeToResolution(base::Time time,
-                                           unsigned int minutes);
-
   // Updates all values from field trial.
   static void UpdateFromFieldTrial();
 
   // Queries variations to determine which language option should be used for
   // app banners and add to homescreen.
   static LanguageOption GetHomescreenLanguageOption();
 
-  // Returns true if the app banner trigger condition should use the site
-  // engagement score instead of the navigation-based heuristic.
-  static bool ShouldUseSiteEngagementScore();
-
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(AppBannerSettingsHelper);
 };
 
 #endif  // CHROME_BROWSER_BANNERS_APP_BANNER_SETTINGS_HELPER_H_