Implement an observer interface for the site engagement service.

chrome/browser/engagement/site_engagement_service.cc

Index: chrome/browser/engagement/site_engagement_service.cc
diff --git a/chrome/browser/engagement/site_engagement_service.cc b/chrome/browser/engagement/site_engagement_service.cc
index fa8f699fa73d44699dceb25ff8fd40be2f399503..80671f721c91d8679450f2583f87d8744738e2fd 100644
--- a/chrome/browser/engagement/site_engagement_service.cc
+++ b/chrome/browser/engagement/site_engagement_service.cc
@@ -139,6 +139,10 @@ SiteEngagementService::~SiteEngagementService() {
       profile_, ServiceAccessType::IMPLICIT_ACCESS);
   if (history)
     history->RemoveObserver(this);
+
+  // Clean up any callbacks which haven't been called.
+  for (const auto& wrapper : engagement_callbacks_)
+    delete wrapper.callback;
 }
 
 SiteEngagementService::EngagementLevel
@@ -209,6 +213,33 @@ bool SiteEngagementService::IsEngagementAtLeast(
   return false;
 }
 
+void SiteEngagementService::RegisterCallback(
+    const EngagementCallback& callback,
+    const GURL& url,
+    EngagementLevel level) {
+  RegisterCallback(callback, url, GetScoreForEngagementLevel(level));
+}
+
+void SiteEngagementService::RegisterCallback(
+    const EngagementCallback& callback,
+    const GURL& url,
+    double score) {
+  // We copy the callback so that when it is invoked, we can immediately delete
+  // the wrapper item in engagement_callbacks_ and ensure it cannot be called
+  // twice. If the callback is run, the copy is deleted as soon as its execution
+  // finishes in RunAndDeleteCallback. Otherwise, the copy is cleaned up in the
+  // destructor.
+  EngagementCallback* callback_ptr = new EngagementCallback(callback);
+  double current_score = GetScore(url);
+  if (current_score >= score) {
+    // Dispatch the callback immediately if its score is already at or above the
+    // requested level.
+    PostCallback(callback_ptr, url.GetOrigin(), current_score);
+  } else {

[calamity, 2016/05/19 03:28:22]

nit: replace with early return.

[dominickn, 2016/05/19 04:36:14]

Done.

+    engagement_callbacks_.push_back({callback_ptr, url.GetOrigin(), score});
+  }
+}
+
 void SiteEngagementService::ResetScoreForURL(const GURL& url, double score) {
   ResetScoreAndAccessTimesForURL(url, score, nullptr);
 }
@@ -261,7 +292,10 @@ double SiteEngagementService::GetTotalEngagementPoints() const {
 
 SiteEngagementService::SiteEngagementService(Profile* profile,
                                              std::unique_ptr<base::Clock> clock)
-    : profile_(profile), clock_(std::move(clock)), weak_factory_(this) {
+    : profile_(profile),
+      clock_(std::move(clock)),
+      engagement_callbacks_(),

[calamity, 2016/05/19 03:28:22]

vectors should initialize fine without being in the list.

[dominickn, 2016/05/19 04:36:14]

Done.

+      weak_factory_(this) {
   // May be null in tests.
   history::HistoryService* history = HistoryServiceFactory::GetForProfile(
       profile, ServiceAccessType::IMPLICIT_ACCESS);
@@ -282,6 +316,7 @@ void SiteEngagementService::AddPoints(const GURL& url, double points) {
         url, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(),
         score_dict.release());
   }
+  ProcessCallbacks(url, score.GetScore());

[calamity, 2016/05/19 03:28:22]

Does this also need to be run when scores are reset?

[dominickn, 2016/05/19 04:36:14]

I explicitly decided to not do that: this runs only when engagement is
explicitly
(not implicitly) earned/changed.

 }
 
 void SiteEngagementService::AfterStartupTask() {
@@ -365,6 +400,25 @@ double SiteEngagementService::GetMedianEngagement(
     return (scores[mid - 1] + scores[mid]) / 2;
 }
 
+double SiteEngagementService::GetScoreForEngagementLevel(
+    EngagementLevel level) const {
+  switch (level) {
+    case ENGAGEMENT_LEVEL_NONE:
+      return 0;
+    case ENGAGEMENT_LEVEL_LOW:
+      return SiteEngagementScore::GetMinimumEngagementIncrement();
+    case ENGAGEMENT_LEVEL_MEDIUM:
+      return SiteEngagementScore::GetMediumEngagementBoundary();
+    case ENGAGEMENT_LEVEL_HIGH:
+      return SiteEngagementScore::GetHighEngagementBoundary();
+    case ENGAGEMENT_LEVEL_MAX:
+      return SiteEngagementScore::kMaxPoints;
+    default:
+      NOTREACHED();
+      return 0;
+  }
+}
+
 void SiteEngagementService::HandleMediaPlaying(const GURL& url,
                                                bool is_hidden) {
   SiteEngagementMetrics::RecordEngagement(
@@ -503,6 +557,32 @@ void SiteEngagementService::GetCountsAndLastVisitForOriginsComplete(
   }
 }
 
+void SiteEngagementService::ProcessCallbacks(const GURL& url, double score) {
+  GURL origin = url.GetOrigin();
+  for (auto it = engagement_callbacks_.begin();
+       it != engagement_callbacks_.end();) {
+    if (origin == it->origin && score >= it->score) {
+      // Found a match. Post the callback to be run on the UI thread and
+      // immediately erase the wrapper object so we don't call it twice if the
+      // score increments again. The callback object will be deleted in
+      // RunAndDeleteCallback.
+      PostCallback(it->callback, url, score);

[calamity, 2016/05/19 03:28:22]

Why do we post this rather than running immediately?

[dominickn, 2016/05/19 04:36:14]

This is potentially run on user input (like scrolling), so I decided to post
the task to avoid lag. This also lets us avoid having to stipulate to callers
that their callback has to be fast enough in case it's run on user input.

+      it = engagement_callbacks_.erase(it);
+    } else {
+      ++it;
+    }
+  }
+}
+
+void SiteEngagementService::PostCallback(const EngagementCallback* callback,
+                                        const GURL& url,
+                                        double score) {

[calamity, 2016/05/19 03:28:22]

nit: alignment

[dominickn, 2016/05/19 04:36:15]

Done.

+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&SiteEngagementService::RunAndDeleteCallback,
+                  weak_factory_.GetWeakPtr(), callback, url, score));
+}
+
 void SiteEngagementService::ResetScoreAndAccessTimesForURL(
     const GURL& url, double score, const base::Time* updated_time) {
   DCHECK(url.is_valid());
@@ -529,3 +609,12 @@ void SiteEngagementService::ResetScoreAndAccessTimesForURL(
         score_dict.release());
   }
 }
+
+void SiteEngagementService::RunAndDeleteCallback(

[calamity, 2016/05/19 03:28:22]

This can be in the anonymous namespace. Doesn't need to be on the class.

[dominickn, 2016/05/19 04:36:15]

Done.

+    const EngagementCallback* callback,
+    const GURL& origin,
+    double score) const {
+  callback->Run(origin, score);
+  delete callback;
+
+}