SiteEngagementObserver now gets the engagement type (+ refactor).

This is to support https://crrev.com/c/792315, which adds a new observer
for logging engagement-type UMA in PWA app windows.

The major interface changes are:
- Renamed OnEngagementIncreased to OnEngagementEvent to indicate that it
  is now for all events that modify engagement, not just increases.
- OnEngagementEvent is now called on launch (so now every place where an
  engagement UMA is logged also corresponds to an OnEngagementEvent).
- OnEngagementEvent now takes an EngagementType enum, indicating what
  caused the engagement event.

Because the engagement type is now exposed through the observer (and can
therefore be used for non-metrics purposes), it has been moved out of
SiteEngagementMetrics into SiteEngagementService.

Also refactored SiteEngagementService to record UMA from the
OnEngagementEvent method, rather than in many places.

Bug: 785661
Change-Id: I352d0363c9ee3a40293e7b99d5b8f6e75c0e999b
Reviewed-on: https://chromium-review.googlesource.com/802661Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521639}

chrome/browser/android/metrics/launch_metrics.cc

@@ -63,7 +63,7 @@ static void JNI_LaunchMetrics_RecordLaunch(
     // launched from a shortcut receive a boost to their engagement.
     SiteEngagementService* service = SiteEngagementService::Get(
         Profile::FromBrowserContext(web_contents->GetBrowserContext()));
-    service->SetLastShortcutLaunchTime(url);
+    service->SetLastShortcutLaunchTime(web_contents, url);
   }
 
   std::string rappor_metric_source;

chrome/browser/banners/app_banner_manager.cc

@@ -289,8 +289,8 @@ void AppBannerManager::OnDidPerformInstallableCheck(
 
   // If we triggered the installability check on page load, then it's possible
   // we don't have enough engagement yet. If that's the case, return here but
-  // don't call Terminate(). We wait for OnEngagementIncreased to tell us that
-  // we should trigger.
+  // don't call Terminate(). We wait for OnEngagementEvent to tell us that we
+  // should trigger.
   if (!HasSufficientEngagement()) {
     UpdateState(State::PENDING_ENGAGEMENT);
     return;
@@ -457,9 +457,11 @@ void AppBannerManager::WebContentsDestroyed() {
   Terminate();
 }
 
-void AppBannerManager::OnEngagementIncreased(content::WebContents* contents,
-                                             const GURL& url,
-                                             double score) {
+void AppBannerManager::OnEngagementEvent(
+    content::WebContents* contents,
+    const GURL& url,
+    double score,
+    SiteEngagementService::EngagementType /*type*/) {
   // Only trigger a banner using site engagement if:
   //  1. engagement increased for the web contents which we are attached to; and
   //  2. there are no currently active media players; and

chrome/browser/banners/app_banner_manager.h

@@ -233,9 +233,10 @@ class AppBannerManager : public content::WebContentsObserver,
   void WebContentsDestroyed() override;
 
   // SiteEngagementObserver overrides.
-  void OnEngagementIncreased(content::WebContents* web_contents,
-                             const GURL& url,
-                             double score) override;
+  void OnEngagementEvent(content::WebContents* web_contents,
+                         const GURL& url,
+                         double score,
+                         SiteEngagementService::EngagementType type) override;
 
   // Subclass accessors for private fields which should not be changed outside
   // this class.

chrome/browser/banners/app_banner_manager_desktop.cc

@@ -128,14 +128,15 @@ void AppBannerManagerDesktop::DidFinishLoad(
   AppBannerManager::DidFinishLoad(render_frame_host, validated_url);
 }
 
-void AppBannerManagerDesktop::OnEngagementIncreased(
+void AppBannerManagerDesktop::OnEngagementEvent(
     content::WebContents* web_contents,
     const GURL& url,
-    double score) {
+    double score,
+    SiteEngagementService::EngagementType type) {
   if (gDisableTriggeringForTesting)
     return;
 
-  AppBannerManager::OnEngagementIncreased(web_contents, url, score);
+  AppBannerManager::OnEngagementEvent(web_contents, url, score, type);
 }
 
 }  // namespace banners

chrome/browser/banners/app_banner_manager_desktop.h

@@ -50,9 +50,10 @@ class AppBannerManagerDesktop
                      const GURL& validated_url) override;
 
   // SiteEngagementObserver override.
-  void OnEngagementIncreased(content::WebContents* web_contents,
-                             const GURL& url,
-                             double score) override;
+  void OnEngagementEvent(content::WebContents* web_contents,
+                         const GURL& url,
+                         double score,
+                         SiteEngagementService::EngagementType type) override;
 
   std::unique_ptr<extensions::BookmarkAppHelper> bookmark_app_helper_;
 

chrome/browser/engagement/site_engagement_helper.cc

@@ -135,17 +135,17 @@ void SiteEngagementService::Helper::InputTracker::DidGetUserInteraction(
   // compiler verifying that all cases are covered).
   switch (type) {
     case blink::WebInputEvent::kRawKeyDown:
-      helper()->RecordUserInput(SiteEngagementMetrics::ENGAGEMENT_KEYPRESS);
+      helper()->RecordUserInput(SiteEngagementService::ENGAGEMENT_KEYPRESS);
       break;
     case blink::WebInputEvent::kMouseDown:
-      helper()->RecordUserInput(SiteEngagementMetrics::ENGAGEMENT_MOUSE);
+      helper()->RecordUserInput(SiteEngagementService::ENGAGEMENT_MOUSE);
       break;
     case blink::WebInputEvent::kTouchStart:
       helper()->RecordUserInput(
-          SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE);
+          SiteEngagementService::ENGAGEMENT_TOUCH_GESTURE);
       break;
     case blink::WebInputEvent::kGestureScrollBegin:
-      helper()->RecordUserInput(SiteEngagementMetrics::ENGAGEMENT_SCROLL);
+      helper()->RecordUserInput(SiteEngagementService::ENGAGEMENT_SCROLL);
       break;
     case blink::WebInputEvent::kUndefined:
       // Explicitly ignore browser-initiated navigation input.
@@ -208,7 +208,7 @@ SiteEngagementService::Helper::Helper(content::WebContents* web_contents)
 }
 
 void SiteEngagementService::Helper::RecordUserInput(
-    SiteEngagementMetrics::EngagementType type) {
+    SiteEngagementService::EngagementType type) {
   TRACE_EVENT0("SiteEngagement", "RecordUserInput");
   content::WebContents* contents = web_contents();
   if (contents)

chrome/browser/engagement/site_engagement_helper.h

@@ -7,7 +7,6 @@
 
 #include "base/macros.h"
 #include "base/timer/timer.h"
-#include "chrome/browser/engagement/site_engagement_metrics.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
@@ -165,7 +164,7 @@ class SiteEngagementService::Helper
 
   // Ask the SiteEngagementService to record engagement via user input at the
   // current WebContents URL.
-  void RecordUserInput(SiteEngagementMetrics::EngagementType type);
+  void RecordUserInput(SiteEngagementService::EngagementType type);
 
   // Ask the SiteEngagementService to record engagement via media playing at the
   // current WebContents URL.

chrome/browser/engagement/site_engagement_helper_unittest.cc

@@ -8,6 +8,7 @@
 #include "base/test/histogram_tester.h"
 #include "base/timer/mock_timer.h"
 #include "base/values.h"
+#include "chrome/browser/engagement/site_engagement_metrics.h"
 #include "chrome/browser/engagement/site_engagement_score.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/common/chrome_switches.h"
@@ -298,10 +299,10 @@ TEST_F(SiteEngagementHelperTest, MixedInputEngagementAccumulation) {
   histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
                               2);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_NAVIGATION, 1);
+                               SiteEngagementService::ENGAGEMENT_NAVIGATION, 1);
   histograms.ExpectBucketCount(
       SiteEngagementMetrics::kEngagementTypeHistogram,
-      SiteEngagementMetrics::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 1);
+      SiteEngagementService::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 1);
 
   HandleUserInputAndRestartTracking(helper, blink::WebInputEvent::kRawKeyDown);
   HandleUserInputAndRestartTracking(helper, blink::WebInputEvent::kTouchStart);
@@ -314,17 +315,17 @@ TEST_F(SiteEngagementHelperTest, MixedInputEngagementAccumulation) {
   histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
                               7);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_NAVIGATION, 1);
+                               SiteEngagementService::ENGAGEMENT_NAVIGATION, 1);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_KEYPRESS, 2);
+                               SiteEngagementService::ENGAGEMENT_KEYPRESS, 2);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_MOUSE, 1);
+                               SiteEngagementService::ENGAGEMENT_MOUSE, 1);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE,
+                               SiteEngagementService::ENGAGEMENT_TOUCH_GESTURE,
                                2);
   histograms.ExpectBucketCount(
       SiteEngagementMetrics::kEngagementTypeHistogram,
-      SiteEngagementMetrics::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 1);
+      SiteEngagementService::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 1);
 
   HandleUserInputAndRestartTracking(helper,
                                     blink::WebInputEvent::kGestureScrollBegin);
@@ -338,21 +339,21 @@ TEST_F(SiteEngagementHelperTest, MixedInputEngagementAccumulation) {
   histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
                               12);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_MOUSE, 2);
+                               SiteEngagementService::ENGAGEMENT_MOUSE, 2);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_SCROLL, 1);
+                               SiteEngagementService::ENGAGEMENT_SCROLL, 1);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE,
+                               SiteEngagementService::ENGAGEMENT_TOUCH_GESTURE,
                                3);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_MEDIA_VISIBLE,
+                               SiteEngagementService::ENGAGEMENT_MEDIA_VISIBLE,
                                1);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_MEDIA_HIDDEN,
+                               SiteEngagementService::ENGAGEMENT_MEDIA_HIDDEN,
                                1);
   histograms.ExpectBucketCount(
       SiteEngagementMetrics::kEngagementTypeHistogram,
-      SiteEngagementMetrics::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 1);
+      SiteEngagementService::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 1);
 
   NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(), url2);
   TrackingStarted(helper);
@@ -370,15 +371,15 @@ TEST_F(SiteEngagementHelperTest, MixedInputEngagementAccumulation) {
   histograms.ExpectTotalCount(SiteEngagementMetrics::kEngagementTypeHistogram,
                               16);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_NAVIGATION, 2);
+                               SiteEngagementService::ENGAGEMENT_NAVIGATION, 2);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_KEYPRESS, 3);
+                               SiteEngagementService::ENGAGEMENT_KEYPRESS, 3);
   histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram,
-                               SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE,
+                               SiteEngagementService::ENGAGEMENT_TOUCH_GESTURE,
                                4);
   histograms.ExpectBucketCount(
       SiteEngagementMetrics::kEngagementTypeHistogram,
-      SiteEngagementMetrics::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 2);
+      SiteEngagementService::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 2);
 }
 
 TEST_F(SiteEngagementHelperTest, CheckTimerAndCallbacks) {

chrome/browser/engagement/site_engagement_metrics.cc

@@ -127,8 +127,10 @@ void SiteEngagementMetrics::RecordPercentOriginsWithMaxEngagement(
                            percentage);
 }
 
-void SiteEngagementMetrics::RecordEngagement(EngagementType type) {
-  UMA_HISTOGRAM_ENUMERATION(kEngagementTypeHistogram, type, ENGAGEMENT_LAST);
+void SiteEngagementMetrics::RecordEngagement(
+    SiteEngagementService::EngagementType type) {
+  UMA_HISTOGRAM_ENUMERATION(kEngagementTypeHistogram, type,
+                            SiteEngagementService::ENGAGEMENT_LAST);
 }
 
 void SiteEngagementMetrics::RecordDaysSinceLastShortcutLaunch(int days) {

chrome/browser/engagement/site_engagement_metrics.h

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/gtest_prod_util.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
 #include "url/gurl.h"
 
 namespace mojom {
@@ -17,23 +18,6 @@ class SiteEngagementDetails;
 // Helper class managing the UMA histograms for the Site Engagement Service.
 class SiteEngagementMetrics {
  public:
-  // This is used to back a UMA histogram, so it should be treated as
-  // append-only. Any new values should be inserted immediately prior to
-  // ENGAGEMENT_LAST.
-  enum EngagementType {
-    ENGAGEMENT_NAVIGATION,
-    ENGAGEMENT_KEYPRESS,
-    ENGAGEMENT_MOUSE,
-    ENGAGEMENT_TOUCH_GESTURE,
-    ENGAGEMENT_SCROLL,
-    ENGAGEMENT_MEDIA_HIDDEN,
-    ENGAGEMENT_MEDIA_VISIBLE,
-    ENGAGEMENT_WEBAPP_SHORTCUT_LAUNCH,
-    ENGAGEMENT_FIRST_DAILY_ENGAGEMENT,
-    ENGAGEMENT_NOTIFICATION_INTERACTION,
-    ENGAGEMENT_LAST,
-  };
-
   static void RecordTotalSiteEngagement(double total_engagement);
   static void RecordTotalOriginsEngaged(int total_origins);
   static void RecordMeanEngagement(double mean_engagement);
@@ -43,7 +27,7 @@ class SiteEngagementMetrics {
   static void RecordOriginsWithMaxEngagement(int total_origins);
   static void RecordOriginsWithMaxDailyEngagement(int total_origins);
   static void RecordPercentOriginsWithMaxEngagement(double percentage);
-  static void RecordEngagement(EngagementType type);
+  static void RecordEngagement(SiteEngagementService::EngagementType type);
   static void RecordDaysSinceLastShortcutLaunch(int days);
   static void RecordScoreDecayedFrom(double score);
   static void RecordScoreDecayedTo(double score);

chrome/browser/engagement/site_engagement_observer.h

@@ -7,6 +7,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
 
 namespace content {
 class WebContents;
@@ -17,22 +18,22 @@ class SiteEngagementService;
 
 class SiteEngagementObserver {
  public:
-  // Called when the engagement for |url| loaded in |web_contents| increases to
-  // |score|. |is_hidden| is true if the engagement occurred when |web_contents|
-  // was hidden (e.g. in the background). This method may be run on user input,
-  // so observers *must not* perform any expensive tasks here. |web_contents|
-  // may be null if the engagement has increased when |url| is not in a tab,
-  // e.g. from a notification interaction.
-  virtual void OnEngagementIncreased(content::WebContents* web_contents,
-                                     const GURL& url,
-                                     double score) {}
+  // Called when the engagement for |url| loaded in |web_contents| is changed
+  // to |score|, due to an event of type |type|. This method may be run on user
+  // input, so observers *must not* perform any expensive tasks here.
+  // |web_contents| may be null if the engagement has increased when |url| is
+  // not in a tab, e.g. from a notification interaction.
+  virtual void OnEngagementEvent(content::WebContents* web_contents,
+                                 const GURL& url,
+                                 double score,
+                                 SiteEngagementService::EngagementType type) {}
 
  protected:
   explicit SiteEngagementObserver(SiteEngagementService* service);
 
   SiteEngagementObserver();
 
-  ~SiteEngagementObserver();
+  virtual ~SiteEngagementObserver();
 
   // Returns the site engagement service which this object is observing.
   SiteEngagementService* GetSiteEngagementService() const;

chrome/browser/engagement/site_engagement_score.cc

@@ -258,7 +258,7 @@ void SiteEngagementScore::AddPoints(double points) {
     // Award bonus engagement for the first engagement of the day for a site.
     points += GetFirstDailyEngagementPoints();
     SiteEngagementMetrics::RecordEngagement(
-        SiteEngagementMetrics::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT);
+        SiteEngagementService::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT);
   }
 
   double to_add = std::min(kMaxPoints - raw_score_,

chrome/browser/engagement/site_engagement_service.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/engagement/site_engagement_helper.h"
 #include "chrome/browser/engagement/site_engagement_metrics.h"
+#include "chrome/browser/engagement/site_engagement_observer.h"
 #include "chrome/browser/engagement/site_engagement_score.h"
 #include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
@@ -180,12 +181,11 @@ void SiteEngagementService::HandleNotificationInteraction(const GURL& url) {
   if (!ShouldRecordEngagement(url))
     return;
 
-  SiteEngagementMetrics::RecordEngagement(
-      SiteEngagementMetrics::ENGAGEMENT_NOTIFICATION_INTERACTION);
   AddPoints(url, SiteEngagementScore::GetNotificationInteractionPoints());
 
   RecordMetrics();
-  OnEngagementIncreased(nullptr /* web_contents */, url);
+  OnEngagementEvent(nullptr /* web_contents */, url,
+                    ENGAGEMENT_NOTIFICATION_INTERACTION);
 }
 
 bool SiteEngagementService::IsBootstrapped() const {
@@ -232,7 +232,9 @@ void SiteEngagementService::ResetBaseScoreForURL(const GURL& url,
   engagement_score.Commit();
 }
 
-void SiteEngagementService::SetLastShortcutLaunchTime(const GURL& url) {
+void SiteEngagementService::SetLastShortcutLaunchTime(
+    content::WebContents* web_contents,
+    const GURL& url) {
   SiteEngagementScore score = CreateEngagementScore(url);
 
   // Record the number of days since the last launch in UMA. If the user's clock
@@ -243,11 +245,11 @@ void SiteEngagementService::SetLastShortcutLaunchTime(const GURL& url) {
     SiteEngagementMetrics::RecordDaysSinceLastShortcutLaunch(
         std::max(0, (now - last_launch).InDays()));
   }
-  SiteEngagementMetrics::RecordEngagement(
-      SiteEngagementMetrics::ENGAGEMENT_WEBAPP_SHORTCUT_LAUNCH);
 
   score.set_last_shortcut_launch_time(now);
   score.Commit();
+
+  OnEngagementEvent(web_contents, url, ENGAGEMENT_WEBAPP_SHORTCUT_LAUNCH);
 }
 
 void SiteEngagementService::HelperCreated(
@@ -506,14 +508,13 @@ void SiteEngagementService::HandleMediaPlaying(
   if (!ShouldRecordEngagement(url))
     return;
 
-  SiteEngagementMetrics::RecordEngagement(
-      is_hidden ? SiteEngagementMetrics::ENGAGEMENT_MEDIA_HIDDEN
-                : SiteEngagementMetrics::ENGAGEMENT_MEDIA_VISIBLE);
   AddPoints(url, is_hidden ? SiteEngagementScore::GetHiddenMediaPoints()
                            : SiteEngagementScore::GetVisibleMediaPoints());
 
   RecordMetrics();
-  OnEngagementIncreased(web_contents, url);
+  OnEngagementEvent(
+      web_contents, url,
+      is_hidden ? ENGAGEMENT_MEDIA_HIDDEN : ENGAGEMENT_MEDIA_VISIBLE);
 }
 
 void SiteEngagementService::HandleNavigation(content::WebContents* web_contents,
@@ -522,34 +523,33 @@ void SiteEngagementService::HandleNavigation(content::WebContents* web_contents,
   if (!IsEngagementNavigation(transition) || !ShouldRecordEngagement(url))
     return;
 
-  SiteEngagementMetrics::RecordEngagement(
-      SiteEngagementMetrics::ENGAGEMENT_NAVIGATION);
   AddPoints(url, SiteEngagementScore::GetNavigationPoints());
 
   RecordMetrics();
-  OnEngagementIncreased(web_contents, url);
+  OnEngagementEvent(web_contents, url, ENGAGEMENT_NAVIGATION);
 }
 
-void SiteEngagementService::HandleUserInput(
-    content::WebContents* web_contents,
-    SiteEngagementMetrics::EngagementType type) {
+void SiteEngagementService::HandleUserInput(content::WebContents* web_contents,
+                                            EngagementType type) {
   const GURL& url = web_contents->GetLastCommittedURL();
   if (!ShouldRecordEngagement(url))
     return;
 
-  SiteEngagementMetrics::RecordEngagement(type);
   AddPoints(url, SiteEngagementScore::GetUserInputPoints());
 
   RecordMetrics();
-  OnEngagementIncreased(web_contents, url);
+  OnEngagementEvent(web_contents, url, type);
 }
 
-void SiteEngagementService::OnEngagementIncreased(
+void SiteEngagementService::OnEngagementEvent(
     content::WebContents* web_contents,
-    const GURL& url) {
+    const GURL& url,
+    EngagementType type) {
+  SiteEngagementMetrics::RecordEngagement(type);
+
   double score = GetScore(url);
   for (SiteEngagementObserver& observer : observer_list_)
-    observer.OnEngagementIncreased(web_contents, url, score);
+    observer.OnEngagementEvent(web_contents, url, score, type);
 }
 
 void SiteEngagementService::SendLevelChangeToHelpers(

chrome/browser/engagement/site_engagement_service.h

@@ -16,8 +16,6 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/engagement/site_engagement_details.mojom.h"
-#include "chrome/browser/engagement/site_engagement_metrics.h"
-#include "chrome/browser/engagement/site_engagement_observer.h"
 #include "components/history/core/browser/history_service_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "third_party/WebKit/public/platform/site_engagement.mojom.h"
@@ -43,6 +41,7 @@ class HistoryService;
 class GURL;
 class HostContentSettingsMap;
 class Profile;
+class SiteEngagementObserver;
 class SiteEngagementScore;
 
 #if defined(OS_ANDROID)
@@ -78,6 +77,25 @@ class SiteEngagementService : public KeyedService,
                               public history::HistoryServiceObserver,
                               public SiteEngagementScoreProvider {
  public:
+  // This is used to back a UMA histogram, so it should be treated as
+  // append-only. Any new values should be inserted immediately prior to
+  // ENGAGEMENT_LAST and added to SiteEngagementServiceEngagementType in
+  // tools/metrics/histograms/enums.xml.
+  // TODO(calamity): Document each of these engagement types.
+  enum EngagementType {
+    ENGAGEMENT_NAVIGATION,
+    ENGAGEMENT_KEYPRESS,
+    ENGAGEMENT_MOUSE,
+    ENGAGEMENT_TOUCH_GESTURE,
+    ENGAGEMENT_SCROLL,
+    ENGAGEMENT_MEDIA_HIDDEN,
+    ENGAGEMENT_MEDIA_VISIBLE,
+    ENGAGEMENT_WEBAPP_SHORTCUT_LAUNCH,
+    ENGAGEMENT_FIRST_DAILY_ENGAGEMENT,
+    ENGAGEMENT_NOTIFICATION_INTERACTION,
+    ENGAGEMENT_LAST,
+  };
+
   // WebContentsObserver that detects engagement triggering events and notifies
   // the service of them.
   class Helper;
@@ -139,9 +157,10 @@ class SiteEngagementService : public KeyedService,
   // the result of GetScore(|url|) may not be the same as |score|.
   void ResetBaseScoreForURL(const GURL& url, double score);
 
-  // Update the last time |url| was opened from an installed shortcut to be
-  // clock_->Now().
-  void SetLastShortcutLaunchTime(const GURL& url);
+  // Update the last time |url| was opened from an installed shortcut (hosted in
+  // |web_contents|) to be clock_->Now().
+  void SetLastShortcutLaunchTime(content::WebContents* web_contents,
+                                 const GURL& url);
 
   void HelperCreated(SiteEngagementService::Helper* helper);
   void HelperDeleted(SiteEngagementService::Helper* helper);
@@ -251,15 +270,16 @@ class SiteEngagementService : public KeyedService,
 
   // Update the engagement score of the origin loaded in |web_contents| for
   // time-on-site, based on user input.
-  void HandleUserInput(content::WebContents* web_contents,
-                       SiteEngagementMetrics::EngagementType type);
+  void HandleUserInput(content::WebContents* web_contents, EngagementType type);
 
-  // Called when the engagement for |url| loaded in |web_contents| increases.
-  // Calls OnEngagementIncreased in all observers. |web_contents| may be null if
-  // the engagement has increased when |url| is not in a tab, e.g. from a
-  // notification interaction.
-  void OnEngagementIncreased(content::WebContents* web_contents,
-                             const GURL& url);
+  // Called when the engagement for |url| loaded in |web_contents| is changed,
+  // due to an event of type |type|. Calls OnEngagementEvent in all observers.
+  // |web_contents| may be null if the engagement has increased when |url| is
+  // not in a tab, e.g. from a notification interaction. Also records
+  // engagement-type metrics.
+  void OnEngagementEvent(content::WebContents* web_contents,
+                         const GURL& url,
+                         EngagementType type);
 
   // Called if |url| changes to |level| engagement, and informs every Helper of
   // the change.
@@ -317,7 +337,7 @@ class SiteEngagementService : public KeyedService,
   std::set<SiteEngagementService::Helper*> helpers_;
 
   // A list of observers. When any origin registers an engagement-increasing
-  // event, each observer's OnEngagementIncreased method will be called.
+  // event, each observer's OnEngagementEvent method will be called.
   base::ObserverList<SiteEngagementObserver> observer_list_;
 
   base::WeakPtrFactory<SiteEngagementService> weak_factory_;

chrome/browser/ui/extensions/application_launch.cc

@@ -376,7 +376,7 @@ WebContents* OpenEnabledApplication(const AppLaunchParams& params) {
     // Record the launch time in the site engagement service. A recent bookmark
     // app launch will provide an engagement boost to the origin.
     SiteEngagementService* service = SiteEngagementService::Get(params.profile);
-    service->SetLastShortcutLaunchTime(url);
+    service->SetLastShortcutLaunchTime(tab, url);
 
     // Refresh the app banner added to homescreen event. The user may have
     // cleared their browsing data since installing the app, which removes the