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

chrome/browser/engagement/site_engagement_service.cc

 // Copyright 2015 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_service.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
-#include <vector>
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_util.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
@@ skipping 23 matching lines @@
 namespace {
 
 const int FOUR_WEEKS_IN_DAYS = 28;
 
 // Global bool to ensure we only update the parameters from variations once.
 bool g_updated_from_variations = false;
 
 // Length of time between metrics logging.
 const int kMetricsIntervalInMinutes = 60;
 
-std::unique_ptr<ContentSettingsForOneType> GetEngagementContentSettings(
-    HostContentSettingsMap* settings_map) {
-  std::unique_ptr<ContentSettingsForOneType> engagement_settings(
-      new ContentSettingsForOneType);
+// Returns the combined list of origins which either have site engagement
+// data stored, or have other settings that would provide a score bonus.
+std::set<GURL> GetEngagementOriginsFromContentSettings(Profile* profile) {
+  HostContentSettingsMap* settings_map =
+      HostContentSettingsMapFactory::GetForProfile(profile);
+
+  ContentSettingsForOneType content_settings_list;
+  std::set<GURL> urls;
+
+  // Fetch site engagement content settings.
   settings_map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
-                                      std::string(), engagement_settings.get());
-  return engagement_settings;
+                                      content_settings::ResourceIdentifier(),
+                                      &content_settings_list);
+  for (const auto& site : content_settings_list) {

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

Nit: no braces

[Wez, 2017/04/10 21:18:09]

Done, though note that other areas of Chromium tend to use braces all the time,
to mitigate potential security issues due to typos. :)

+    urls.insert(GURL(site.primary_pattern.ToString()));
+  }
+
+  // Fetch notifications allowed content settings.
+  settings_map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+                                      content_settings::ResourceIdentifier(),
+                                      &content_settings_list);
+  for (const auto& site : content_settings_list) {
+    if (site.setting == CONTENT_SETTING_ALLOW)

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

Should this be != ?

[Wez, 2017/04/10 21:18:09]

Yup; and of course my simple test wouldn't catch that... *facepalm*

+      continue;
+    urls.insert(GURL(site.primary_pattern.ToString()));
+  }
+
+  return urls;
 }
 
 // Only accept a navigation event for engagement if it is one of:
 //  a. direct typed navigation
 //  b. clicking on an omnibox suggestion brought up by typing a keyword
 //  c. clicking on a bookmark or opening a bookmark app
 //  d. a custom search engine keyword search (e.g. Wikipedia search box added as
 //  search engine).
 bool IsEngagementNavigation(ui::PageTransition transition) {
   return ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_TYPED) ||
@@ skipping 25 matching lines @@
       base::FieldTrialList::FindFullName(kEngagementParams);
   return !base::StartsWith(group_name, "Disabled",
                            base::CompareCase::SENSITIVE);
 }
 
 // static
 double SiteEngagementService::GetScoreFromSettings(
     HostContentSettingsMap* settings,
     const GURL& origin) {
   auto clock = base::MakeUnique<base::DefaultClock>();
-  return SiteEngagementScore(clock.get(), origin, settings)
-      .GetScore();
+  return SiteEngagementScore(clock.get(), origin, settings).GetTotalScore();
 }
 
 SiteEngagementService::SiteEngagementService(Profile* profile)
     : SiteEngagementService(profile, base::MakeUnique<base::DefaultClock>()) {
   content::BrowserThread::PostAfterStartupTask(
       FROM_HERE, content::BrowserThread::GetTaskRunnerForThread(
                      content::BrowserThread::UI),
       base::Bind(&SiteEngagementService::AfterStartupTask,
                  weak_factory_.GetWeakPtr()));
 
@@ skipping 13 matching lines @@
 }
 
 blink::mojom::EngagementLevel
 SiteEngagementService::GetEngagementLevel(const GURL& url) const {
   if (IsLastEngagementStale())
     CleanupEngagementScores(true);
 
   return CreateEngagementScore(url).GetEngagementLevel();
 }
 
-std::map<GURL, double> SiteEngagementService::GetScoreMap() const {
-  HostContentSettingsMap* settings_map =
-      HostContentSettingsMapFactory::GetForProfile(profile_);
-  std::unique_ptr<ContentSettingsForOneType> engagement_settings =
-      GetEngagementContentSettings(settings_map);
+std::vector<mojom::SiteEngagementDetails> SiteEngagementService::GetAllDetails()
+    const {
+  std::set<GURL> origins = GetEngagementOriginsFromContentSettings(profile_);
 
-  std::map<GURL, double> score_map;
-  for (const auto& site : *engagement_settings) {
-    GURL origin(site.primary_pattern.ToString());
+  std::vector<mojom::SiteEngagementDetails> details;
+  details.reserve(origins.size());
+  for (const GURL& origin : origins) {
     if (!origin.is_valid())
       continue;
+    details.push_back(GetDetails(origin));
+  }
 
+  return details;
+}
+
+std::map<GURL, double> SiteEngagementService::GetScoreMap() const {
+  std::map<GURL, double> score_map;
+  for (const GURL& origin : GetEngagementOriginsFromContentSettings(profile_)) {
+    if (!origin.is_valid())
+      continue;
     score_map[origin] = GetScore(origin);
   }
-
   return score_map;
 }
 
 void SiteEngagementService::HandleNotificationInteraction(const GURL& url) {
   if (!ShouldRecordEngagement(url))
     return;
 
   SiteEngagementMetrics::RecordEngagement(
       SiteEngagementMetrics::ENGAGEMENT_NOTIFICATION_INTERACTION);
   AddPoints(url, SiteEngagementScore::GetNotificationInteractionPoints());
@@ skipping 70 matching lines @@
     SiteEngagementService::Helper* helper) {
   helpers_.insert(helper);
 }
 
 void SiteEngagementService::HelperDeleted(
     SiteEngagementService::Helper* helper) {
   helpers_.erase(helper);
 }
 
 double SiteEngagementService::GetScore(const GURL& url) const {
+  return GetDetails(url).total_score;
+}
+
+mojom::SiteEngagementDetails SiteEngagementService::GetDetails(
+    const GURL& url) const {
   // Ensure that if engagement is stale, we clean things up before fetching the
   // score.
   if (IsLastEngagementStale())
     CleanupEngagementScores(true);
 
-  return CreateEngagementScore(url).GetScore();
+  return CreateEngagementScore(url).GetDetails();
 }
 
 double SiteEngagementService::GetTotalEngagementPoints() const {
   std::map<GURL, double> score_map = GetScoreMap();
 
   double total_score = 0;
   for (const auto& value : score_map)
     total_score += value.second;
 
   return total_score;
@@ skipping 45 matching lines @@
 
 void SiteEngagementService::AfterStartupTask() {
   // Check if we need to reset last engagement times on startup - we want to
   // avoid doing this in AddPoints() if possible. It is still necessary to check
   // in AddPoints for people who never restart Chrome, but leave it open and
   // their computer on standby.
   CleanupEngagementScores(IsLastEngagementStale());
   RecordMetrics();
 }
 
 void SiteEngagementService::CleanupEngagementScores(

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

This cleanup method doesn't need to iterate over origins with notification
permission. Its only purpose is to clean up the engagement content setting of
sites with 0 raw scores, and rescale things in there if Chrome hasn't been used
in a while (so you don't get a huge loss if you go on holiday). Notifications is
always a flat boost, so it's not necessary to capture origins which only have
notification permission (and no raw engagement) in this method.

[Wez, 2017/04/10 21:18:09]

Done.

     bool update_last_engagement_time) const {
-  HostContentSettingsMap* settings_map =
-      HostContentSettingsMapFactory::GetForProfile(profile_);
-  std::unique_ptr<ContentSettingsForOneType> engagement_settings =
-      GetEngagementContentSettings(settings_map);
-
   // We want to rebase last engagement times relative to MaxDecaysPerScore
   // periods of decay in the past.
   base::Time now = clock_->Now();
   base::Time last_engagement_time = GetLastEngagementTime();
   base::Time rebase_time = now - GetMaxDecayPeriod();
   base::Time new_last_engagement_time;
 
   // If |update_last_engagement_time| is true, we must have either:
   //   a) last_engagement_time is in the future; OR
   //   b) last_engagement_time < rebase_time < now
   DCHECK(!update_last_engagement_time || last_engagement_time >= now ||
          (last_engagement_time < rebase_time && rebase_time < now));
 
   // Cap |last_engagement_time| at |now| if it is in the future. This ensures
   // that we use sane offsets when a user has adjusted their clock backwards and
   // have a mix of scores prior to and after |now|.
   if (last_engagement_time > now)
     last_engagement_time = now;
 
-  for (const auto& site : *engagement_settings) {
-    GURL origin(site.primary_pattern.ToString());
-
+  HostContentSettingsMap* settings_map =
+      HostContentSettingsMapFactory::GetForProfile(profile_);
+  for (const GURL& origin : GetEngagementOriginsFromContentSettings(profile_)) {
     if (origin.is_valid()) {
       SiteEngagementScore score = CreateEngagementScore(origin);
       if (update_last_engagement_time) {
         // Catch cases of users moving their clocks, or a potential race where
         // a score content setting is written out to prefs, but the updated
         // |last_engagement_time| was not written, as both are lossy
         // preferences. |rebase_time| is strictly in the past, so any score with
         // a last updated time in the future is caught by this branch.
         if (score.last_engagement_time() > rebase_time) {
           score.set_last_engagement_time(now);
@@ skipping 12 matching lines @@
               last_engagement_time - score.last_engagement_time();
           base::Time rebase_score_time = rebase_time - offset;
           score.set_last_engagement_time(rebase_score_time);
         }
 
         if (score.last_engagement_time() > new_last_engagement_time)
           new_last_engagement_time = score.last_engagement_time();
         score.Commit();
       }
 
-      if (score.GetScore() > SiteEngagementScore::GetScoreCleanupThreshold())
+      if (score.GetTotalScore() >
+          SiteEngagementScore::GetScoreCleanupThreshold())
         continue;
     }
 
     // This origin has a score of 0. Wipe it from content settings.
     settings_map->SetWebsiteSettingDefaultScope(
         origin, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(),
         nullptr);
   }
 
   // Set the last engagement time to be consistent with the scores. This will
@@ skipping 180 matching lines @@
     const GURL& origin) const {
   // If we are in incognito, |settings| will automatically have the data from
   // the original profile migrated in, so all engagement scores in incognito
   // will be initialised to the values from the original profile.
   return SiteEngagementScore(
       clock_.get(), origin,
       HostContentSettingsMapFactory::GetForProfile(profile_));
 }
 
 int SiteEngagementService::OriginsWithMaxDailyEngagement() const {
-  HostContentSettingsMap* settings_map =
-      HostContentSettingsMapFactory::GetForProfile(profile_);
-  std::unique_ptr<ContentSettingsForOneType> engagement_settings =
-      GetEngagementContentSettings(settings_map);
-
   int total_origins = 0;
 
   // We cannot call GetScoreMap as we need the score objects, not raw scores.
-  for (const auto& site : *engagement_settings) {
-    GURL origin(site.primary_pattern.ToString());
+  for (const GURL& origin : GetEngagementOriginsFromContentSettings(profile_)) {

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

An origin can't possibly get maximum daily engagement unless it has an entry in
the engagement content setting (max daily = 15), so it's probably not necessary
to do this here.

[Wez, 2017/04/10 21:18:09]

Done.

     if (!origin.is_valid())
       continue;
 
     if (CreateEngagementScore(origin).MaxPointsPerDayAdded())
       ++total_origins;
   }
 
   return total_origins;
 }
 
@@ skipping 48 matching lines @@
     // under the origin in history. If this new last visit date is long enough
     // in the past, the next time the origin's engagement is accessed the
     // automatic decay will kick in - i.e. a double decay will have occurred.
     // To prevent this, compute the decay that would have taken place since the
     // new last visit and add it to the engagement at this point. When the
     // engagement is next accessed, it will decay back to the proportionally
     // reduced value rather than being decayed once here, and then once again
     // when it is next accessed.
     SiteEngagementScore engagement_score = CreateEngagementScore(origin);
 
-    double new_score = proportion_remaining * engagement_score.GetScore();
+    double new_score = proportion_remaining * engagement_score.raw_score();
     int hours_since_engagement = (now - last_visit).InHours();
     int periods =
         hours_since_engagement / SiteEngagementScore::GetDecayPeriodInHours();
     new_score += periods * SiteEngagementScore::GetDecayPoints();
     new_score *= pow(1.0 / SiteEngagementScore::GetDecayProportion(), periods);
 
     double score = std::min(SiteEngagementScore::kMaxPoints, new_score);
     engagement_score.Reset(score, last_visit);
     if (!engagement_score.last_shortcut_launch_time().is_null() &&
         engagement_score.last_shortcut_launch_time() > last_visit) {
       engagement_score.set_last_shortcut_launch_time(last_visit);
     }
 
     engagement_score.Commit();
   }
 
   SetLastEngagementTime(now);
 }