Implement a site engagement score based on time-on-site.

chrome/browser/engagement/site_engagement_helper.h

Index: chrome/browser/engagement/site_engagement_helper.h
diff --git a/chrome/browser/engagement/site_engagement_helper.h b/chrome/browser/engagement/site_engagement_helper.h
index c3883d5cf41a1363e67469f1c0e0bcf2d7635844..fb5baf90796b6a08ed4d3d46729af4290003d7a0 100644
--- a/chrome/browser/engagement/site_engagement_helper.h
+++ b/chrome/browser/engagement/site_engagement_helper.h
@@ -6,6 +6,9 @@
 #define CHROME_BROWSER_ENGAGEMENT_SITE_ENGAGEMENT_HELPER_H_
 
 #include "base/macros.h"
+#include "base/observer_list.h"
+#include "base/timer/timer.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
@@ -16,21 +19,104 @@ class WebContents;
 class GURL;
 
 // Per-WebContents class to handle updating the site engagement scores for
-// origins as the user navigates.
+// origins based on the time spent on site.

[benwells, 2015/09/29 00:24:12]

Nit: this comment is too restrictive.

[dominickn, 2015/09/29 01:40:36]

Done.

+//
+// Time on site is recorded by detecting any user input (mouse or keypress) per
+// some discrete time unit. If there is user input, then record a positive site

[benwells, 2015/09/29 00:24:12]

Nit: move this into the InputTracker class level comment.

[dominickn, 2015/09/29 01:40:36]

Done.

+// engagement.
+//
+// TODO(dominickn): account for use cases where user input is not a good proxy
+// for time on site: e.g. watching videos.

[benwells, 2015/09/29 00:24:12]

Nit: I'd remove this TODO as we don't have clear ideas about what to do.

[dominickn, 2015/09/29 01:40:36]

Done.

 class SiteEngagementHelper
     : public content::WebContentsObserver,
       public content::WebContentsUserData<SiteEngagementHelper> {
  public:
+  class Observer {
+   public:
+    // Called once user input is recorded.
+    virtual void OnInputRecorded(SiteEngagementHelper* helper) = 0;
+  };
+
   ~SiteEngagementHelper() override;
 
+  void AddObserverForTesting(Observer* observer);
+  void RemoveObserverForTesting(Observer* observer);
+
+  static void SetSecondsBetweenUserInputCheck(double seconds);
+
  private:
+  // Class to encapsulate the user input listening.
+  //
+  // User input is used to record time-on-site. When input is detected,
+  // SiteEngagementHelper::RecordUserInput is called, and the input detection
+  // callbacks are paused for a duration of g_seconds_between_user_input_check.
+  // This ensures that there is minimal overhead in input listening, and that
+  // input over an extended length of time is required to continually increase
+  // the engagement score.
+  class InputTracker {
+   public:
+    explicit InputTracker(SiteEngagementHelper* helper);
+    ~InputTracker();
+
+    // Callback to handle key press events from the RenderViewHost.
+    bool HandleKeyPressEvent(const content::NativeWebKeyboardEvent& event);
+
+    // Callback to handle mouse events from the RenderViewHost.
+    bool HandleMouseEvent(const blink::WebMouseEvent& event);
+
+    // Register callbacks to listen for user input.
+    void StartTracking(content::RenderViewHost* host);
+
+    // Pause listening for user input, restarting listening after
+    // g_seconds_between_user_input_check seconds.
+    void PauseTracking(content::RenderViewHost* host);
+
+    // Restart listening for user input.
+    void ResumeTracking();
+
+    // Stop listening for user input.
+    void StopTracking(content::RenderViewHost* host);
+
+    // Set the timer object for testing purposes.
+    void SetTimerForTesting(scoped_ptr<base::Timer> timer);
+
+    bool callbacks_added() { return callbacks_added_; }
+
+   private:
+    SiteEngagementHelper* helper_;
+    scoped_ptr<base::Timer> pause_timer_;
+    content::RenderWidgetHost::KeyPressEventCallback key_press_event_callback_;
+    content::RenderWidgetHost::MouseEventCallback mouse_event_callback_;
+    bool callbacks_added_;
+  };
+
   explicit SiteEngagementHelper(content::WebContents* web_contents);
   friend class content::WebContentsUserData<SiteEngagementHelper>;
+  friend class SiteEngagementServiceBrowserTest;
+
+  // Ask the SiteEngagementService to record engagement via user input at the
+  // current contents location.
+  void RecordUserInput();
+
+  void SetTimerForTesting(scoped_ptr<base::Timer> timer);
+
+  static void EnableCallbackRegistrationForTesting();
+  static void DisableCallbackRegistrationForTesting();
 
   // content::WebContentsObserver overrides.
-  void DidStartNavigationToPendingEntry(
-      const GURL& url,
-      content::NavigationController::ReloadType reload_type) override;
+  void DidNavigateMainFrame(
+      const content::LoadCommittedDetails& details,
+      const content::FrameNavigateParams& params) override;
+
+  void RenderViewHostChanged(content::RenderViewHost* old_host,
+                             content::RenderViewHost* new_host) override;
+
+  void WasShown() override;
+  void WasHidden() override;
+
+  scoped_ptr<InputTracker> input_tracker_;

[benwells, 2015/09/29 00:24:12]

Is there any need for this to be a scoped_ptr, versus a plain old member?

[dominickn, 2015/09/29 01:40:36]

Done.

+  base::ObserverList<Observer> observer_list_;
+  bool record_engagement_;
 
   DISALLOW_COPY_AND_ASSIGN(SiteEngagementHelper);
 };