Small cleanup of SiteEngagementService.

chrome/browser/engagement/site_engagement_score.h

Index: chrome/browser/engagement/site_engagement_score.h
diff --git a/chrome/browser/engagement/site_engagement_score.h b/chrome/browser/engagement/site_engagement_score.h
index 7bebca721288261319567a1b77d8b93ecc64c414..662ca88706d4c7d564477fa49cb5aacde3572da1 100644
--- a/chrome/browser/engagement/site_engagement_score.h
+++ b/chrome/browser/engagement/site_engagement_score.h
@@ -9,11 +9,14 @@
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "url/gurl.h"
 
 namespace base {
 class Clock;
 }
 
+class HostContentSettingsMap;
+
 class SiteEngagementScore {
  public:
   // The parameters which can be varied via field trial. All "points" values
@@ -84,15 +87,22 @@ class SiteEngagementScore {
   // responsibility of the caller to make sure |clock| outlives this
   // SiteEngagementScore.
   SiteEngagementScore(base::Clock* clock,
-                      const base::DictionaryValue& score_dict);
+                      const GURL& origin,
+                      HostContentSettingsMap* settings_map);
+  SiteEngagementScore(SiteEngagementScore&& other);
   ~SiteEngagementScore();
 
+  SiteEngagementScore& operator=(SiteEngagementScore&& other);
+
   // Adds |points| to this score, respecting daily limits and the maximum
   // possible score. Decays the score if it has not been updated recently
   // enough.
   void AddPoints(double points);
   double GetScore() const;
 
+  // Writes the values in this score into |settings_map_|.
+  void Commit();
+
   // Returns true if the maximum number of points today has been added.
   bool MaxPointsPerDayAdded() const;
 
@@ -101,15 +111,7 @@ class SiteEngagementScore {
   // shortcut launch time (if it is non-null) to |updated_time|. Otherwise, last
   // engagement time is set to the current time and last shortcut launch time is
   // left unchanged.
-  // TODO(calamity): Ideally, all SiteEngagementScore methods should take a
-  // base::Time argument like this one does rather than each SiteEngagementScore
-  // hold a pointer to a base::Clock. Then SiteEngagementScore doesn't need to
-  // worry about clock vending. See crbug.com/604305.
-  void Reset(double points, const base::Time* updated_time);
-
-  // Updates the content settings dictionary |score_dict| with the current score
-  // fields. Returns true if |score_dict| changed, otherwise return false.
-  bool UpdateScoreDict(base::DictionaryValue* score_dict);
+  void Reset(double points, const base::Time updated_time);
 
   // Get/set the last time this origin was launched from an installed shortcut.
   base::Time last_shortcut_launch_time() const {
@@ -139,7 +141,9 @@ class SiteEngagementScore {
   static const char* kLastShortcutLaunchTimeKey;
 
   // This version of the constructor is used in unit tests.
-  explicit SiteEngagementScore(base::Clock* clock);
+  explicit SiteEngagementScore(

[dominickn, 2016/05/25 06:52:43]

Nit: remove explicit and the unit tests comment

[calamity, 2016/05/26 08:26:20]

The comment is still true.

[dominickn, 2016/05/26 09:33:20]

Acknowledged.

+      base::Clock* clock,
+      std::unique_ptr<base::DictionaryValue> score_dict);
 
   // Determine the score, accounting for any decay.
   double DecayedScore() const;
@@ -151,6 +155,10 @@ class SiteEngagementScore {
   // newly added parameters receive a fixed value here.
   static void SetParamValuesForTesting();
 
+  // Updates the content settings dictionary |score_dict| with the current score
+  // fields. Returns true if |score_dict| changed, otherwise return false.
+  bool UpdateScoreDict(base::DictionaryValue* score_dict);
+
   // The clock used to vend times. Enables time travelling in tests. Owned by
   // the SiteEngagementService.
   base::Clock* clock_;
@@ -171,6 +179,15 @@ class SiteEngagementScore {
   // shortcut.
   base::Time last_shortcut_launch_time_;
 
+  // The dictionary that represents this engagement score.
+  std::unique_ptr<base::DictionaryValue> score_dict_;
+
+  // The origin this score represents.
+  GURL origin_;
+
+  // The settings map to write this score to when Commit() is called.
+  HostContentSettingsMap* settings_map_;
+
   DISALLOW_COPY_AND_ASSIGN(SiteEngagementScore);
 };