Add SiteEngagementService::GetAllDetails(), to return detailed scores.

chrome/browser/engagement/site_engagement_score.cc

 // Copyright 2016 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/engagement/site_engagement_score.h"
 
 #include <cmath>
 #include <utility>
 
 #include "base/memory/ptr_util.h"
@@ skipping 252 matching lines @@
   double to_add = std::min(kMaxPoints - raw_score_,
                            GetMaxPointsPerDay() - points_added_today_);
   to_add = std::min(to_add, points);
 
   points_added_today_ += to_add;
   raw_score_ += to_add;
 
   last_engagement_time_ = now;
 }
 
-double SiteEngagementScore::GetScore() const {
-  return std::min(DecayedScore() + BonusScore(), kMaxPoints);
+mojom::SiteEngagementDetails SiteEngagementScore::GetDetails() const {
+  mojom::SiteEngagementDetails engagement;
+  engagement.origin = origin_;
+  engagement.base_score = DecayedScore();
+  engagement.installed_bonus = BonusIfShortcutLaunched();
+  engagement.notifications_bonus = BonusIfHasNotifications();
+  engagement.total_score = GetTotal();
+  return engagement;
+}
+
+double SiteEngagementScore::GetTotal() const {

[Wez, 2017/04/08 01:52:53]

Renamed this to GetTotal, for clarity, and rearranged things to avoid the churn
of calling GetDetails() under the hood.

[dominickn, 2017/04/10 00:58:11]

I'm not sure that GetTotal is more clear than GetScore (plus the comment in the
header file can clarify). But I'm not overly fussy on this.

[Wez, 2017/04/10 03:45:28]

The class is SiteEngagementScore, so SiteEngagementScore::GetTotal() seems
reasonably intuitive, and more obvious than Site...::GetScore().  If you prefer
GetTotalScore() though then happy to update to that.

[dominickn, 2017/04/10 04:41:26]

Not fussed, it can stay.

+  return std::min(
+      DecayedScore() + BonusIfShortcutLaunched() + BonusIfHasNotifications(),
+      kMaxPoints);
 }
 
 void SiteEngagementScore::Commit() {
   DCHECK(settings_map_);
   if (!UpdateScoreDict(score_dict_.get()))
     return;
 
   settings_map_->SetWebsiteSettingDefaultScope(
       origin_, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(),
       std::move(score_dict_));
 }
 
 blink::mojom::EngagementLevel SiteEngagementScore::GetEngagementLevel() const {
   DCHECK_LT(GetMediumEngagementBoundary(), GetHighEngagementBoundary());
 
-  double score = GetScore();
+  double score = GetTotal();
   if (score == 0)
     return blink::mojom::EngagementLevel::NONE;
 
   if (score < 1)
     return blink::mojom::EngagementLevel::MINIMAL;
 
   if (score < GetMediumEngagementBoundary())
     return blink::mojom::EngagementLevel::LOW;
 
   if (score < GetHighEngagementBoundary())
@@ skipping 92 matching lines @@
   int hours_since_engagement =
       (clock_->Now() - last_engagement_time_).InHours();
   if (hours_since_engagement < 0)
     return raw_score_;
 
   int periods = hours_since_engagement / GetDecayPeriodInHours();
   return std::max(0.0, raw_score_ * pow(GetDecayProportion(), periods) -
                            periods * GetDecayPoints());
 }
 
-double SiteEngagementScore::BonusScore() const {
-  double bonus = 0;
+double SiteEngagementScore::BonusIfShortcutLaunched() const {
   int days_since_shortcut_launch =
       (clock_->Now() - last_shortcut_launch_time_).InDays();
   if (days_since_shortcut_launch <= kMaxDaysSinceShortcutLaunch)

[Wez, 2017/04/10 03:45:28]

nit: While we'll never hit this in practice, this code would have had unexpected
behaviour in January 1970 (and it does behave unexpectedly in tests with
SimpleTestClock...).

-    bonus += GetWebAppInstalledPoints();
+    return GetWebAppInstalledPoints();
+  return 0;
+}
 
+double SiteEngagementScore::BonusIfHasNotifications() const {

[dominickn, 2017/04/10 00:58:11]

Nit: I'd call this BonusIfCanShowNotifications()

[Wez, 2017/04/10 03:45:28]

Done.

   // TODO(dominickn, raymes): call PermissionManager::GetPermissionStatus when
   // the PermissionManager is thread-safe.
   if (settings_map_ && settings_map_->GetContentSetting(
                            origin_, GURL(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
                            std::string()) == CONTENT_SETTING_ALLOW) {
-    bonus += GetNotificationPermissionPoints();
+    return GetNotificationPermissionPoints();
   }
 
-  return bonus;
+  return 0;
 }