Plumb site engagement to the renderer process.

chrome/browser/engagement/site_engagement_service.h

 // 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.
 
 #ifndef CHROME_BROWSER_ENGAGEMENT_SITE_ENGAGEMENT_SERVICE_H_
 #define CHROME_BROWSER_ENGAGEMENT_SITE_ENGAGEMENT_SERVICE_H_
 
 #include <map>
 #include <memory>
 #include <set>
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/time/time.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"
 #include "ui/base/page_transition_types.h"
 
 namespace base {
 class Clock;
 }
 
 namespace content {
 class WebContents;
 }
 
@@ skipping 32 matching lines @@
 // threads using SiteEngagementService::GetScoreFromSettings, but use of this
 // method is discouraged unless it is not possible to use the UI thread.
 class SiteEngagementService : public KeyedService,
                               public history::HistoryServiceObserver,
                               public SiteEngagementScoreProvider {
  public:
   // WebContentsObserver that detects engagement triggering events and notifies
   // the service of them.
   class Helper;
 
-  enum EngagementLevel {
-    ENGAGEMENT_LEVEL_NONE,
-    ENGAGEMENT_LEVEL_LOW,
-    ENGAGEMENT_LEVEL_MEDIUM,
-    ENGAGEMENT_LEVEL_HIGH,
-    ENGAGEMENT_LEVEL_MAX,
-  };
-
   // The name of the site engagement variation field trial.
   static const char kEngagementParams[];
 
   // Returns the site engagement service attached to this profile. The service
   // exists in incognito mode; scores will be initialised using the score from
   // the profile that the incognito session was created from, and will increase
   // and decrease as usual. Engagement earned or decayed in incognito will not
   // be persisted or reflected in the original profile.
   //
   // This method must be called on the UI thread.
@@ skipping 10 matching lines @@
   //
   // Should only be used if you cannot create a SiteEngagementService (i.e. you
   // cannot run on the UI thread).
   static double GetScoreFromSettings(HostContentSettingsMap* settings,
                                      const GURL& origin);
 
   explicit SiteEngagementService(Profile* profile);
   ~SiteEngagementService() override;
 
   // Returns the engagement level of |url|.
-  EngagementLevel GetEngagementLevel(const GURL& url) const;
+  blink::mojom::EngagementLevel GetEngagementLevel(const GURL& url) const;
 
   // Returns a map of all stored origins and their engagement scores.
   std::map<GURL, double> GetScoreMap() const;
 
   // Returns whether the engagement service has enough data to make meaningful
   // decisions. Clients should avoid using engagement in their heuristic until
   // this is true.
   bool IsBootstrapped() const;
 
   // Returns whether |url| has at least the given |level| of engagement.
-  bool IsEngagementAtLeast(const GURL& url, EngagementLevel level) const;
+  bool IsEngagementAtLeast(const GURL& url,
+                           blink::mojom::EngagementLevel level) const;
 
   // Resets the engagement score |url| to |score|, clearing daily limits.
   void ResetScoreForURL(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);
 
+  void HelperCreated(SiteEngagementService::Helper* helper);

[benwells, 2016/12/09 06:11:24]

This seems like an observer now. Apart from this, there doesn't appear to be any
reason for the Helper to be a nested class. Can you:
- remove the Helper nested class
- add an Observer nested class
- change SiteEnagementService::Helper to be SiteEngagementHelper
- make the SiteEngagementHelper implement the SiteEngagementService::Observer
interface
- use the normal AddObserver, ObserverList etc. patterns

[dominickn, 2016/12/13 06:27:37]

I've revamped the Helper to make it more of a private implementation detail of
the service. WDYT?

[benwells, 2016/12/14 06:17:53]

Hrmph. I think it is now more obscure. Not only is the observer not called an
observer or managed like other observers, the creation feels more convoluted. 

I won't block you on this. I think the weirdness is all private to this class'
implementation and tests, and you pretty much own all of it, so I'll defer to
you. But I'd prefer the previous patch set as the helper creation was more
straightforward, so will lg if you go back to that.

[dominickn, 2016/12/15 01:57:08]

Done.

I think the main thing here is that we have what really is a private
implementation detail (the helper) in a separate .h/.cc file for clarity
(otherwise the service .h/.cc files would be enormous). As much as possible, we
want to keep the helper -> service interface as a private detail.

+  void HelperDeleted(SiteEngagementService::Helper* helper);
+
   // Overridden from SiteEngagementScoreProvider.
   double GetScore(const GURL& url) const override;
   double GetTotalEngagementPoints() const override;
 
  private:
   friend class SiteEngagementObserver;
   FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, CheckHistograms);
   FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest, CleanupEngagementScores);
   FRIEND_TEST_ALL_PREFIXES(SiteEngagementServiceTest,
                            CleanupMovesScoreBackToNow);
@@ skipping 70 matching lines @@
   // Update the engagement score of the origin loaded in |web_contents| for
   // navigation.
   void HandleNavigation(content::WebContents* web_contents,
                         ui::PageTransition transition);
 
   // 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);
 
+  // Called if |url| changes to |level| engagement, and informs every Helper of
+  // the change.
+  void SendLevelChangeToHelpers(const GURL& url,
+                                blink::mojom::EngagementLevel level);
+
   // Returns true if the last engagement increasing event seen by the site
   // engagement service was sufficiently long ago that we need to reset all
   // scores to be relative to now. This ensures that users who do not use the
   // browser for an extended period of time do not have their engagement decay.
   bool IsLastEngagementStale() const;
 
   // Overridden from history::HistoryServiceObserver:
   void OnURLsDeleted(history::HistoryService* history_service,
                      bool all_history,
                      bool expired,
@@ skipping 22 matching lines @@
   // The clock used to vend times.
   std::unique_ptr<base::Clock> clock_;
 
   // Metrics are recorded at non-incognito browser startup, and then
   // approximately once per hour thereafter. Store the local time at which
   // metrics were previously uploaded: the first event which affects any
   // origin's engagement score after an hour has elapsed triggers the next
   // upload.
   base::Time last_metrics_time_;
 
+  // All helpers currently attached to a WebContents.
+  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.
   base::ObserverList<SiteEngagementObserver> observer_list_;
 
   base::WeakPtrFactory<SiteEngagementService> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SiteEngagementService);
 };
 
 #endif  // CHROME_BROWSER_ENGAGEMENT_SITE_ENGAGEMENT_SERVICE_H_