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

chrome/browser/engagement/site_engagement_helper.cc

Index: chrome/browser/engagement/site_engagement_helper.cc
diff --git a/chrome/browser/engagement/site_engagement_helper.cc b/chrome/browser/engagement/site_engagement_helper.cc
index b107edf030c458bb1d9803395d91ab0dbc7652c6..a372886051f85aa39304b893a0663fa38e08c431 100644
--- a/chrome/browser/engagement/site_engagement_helper.cc
+++ b/chrome/browser/engagement/site_engagement_helper.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/engagement/site_engagement_helper.h"
 
+#include "base/time/time.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/browser/prerender/prerender_contents.h"
@@ -11,36 +12,161 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
 
+namespace {
+
+double gSecondsBetweenUserInputCheck = 10;

[calamity, 2015/09/22 02:57:20]

The more common style seems to be g_seconds_between_user_input_check.

[dominickn, 2015/09/23 00:06:44]

Done.

+bool gDisableCallbackRegistrationForTesting = false;
+
+}  // anonymous namespace
+
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(SiteEngagementHelper);
 
-SiteEngagementHelper::~SiteEngagementHelper() {
+SiteEngagementHelper::InputTracker::InputTracker(SiteEngagementHelper* helper)
+    : helper_(helper),
+      timer_(new base::Timer(true, false)),
+      callbacks_added_(false) {
+  key_press_event_callback_ =
+      base::Bind(&SiteEngagementHelper::InputTracker::HandleKeyPressEvent,
+                 base::Unretained(this));
+  mouse_event_callback_ =
+      base::Bind(&SiteEngagementHelper::InputTracker::HandleMouseEvent,
+                 base::Unretained(this));
+}
+
+SiteEngagementHelper::InputTracker::~InputTracker() { }
+
+// Record that there was some user input, and defer handling of the input event.
+// web_contents() will return nullptr if the observed contents have been
+// deleted; if the contents exist, record engagement for the site. Once the
+// timer finishes running, the callbacks detecting user input will be registered
+// again.
+bool SiteEngagementHelper::InputTracker::HandleKeyPressEvent(
+    const content::NativeWebKeyboardEvent& event) {
+  PauseTracking(helper_->web_contents()->GetRenderViewHost());
+  helper_->RecordUserInput();

[calamity, 2015/09/22 02:57:21]

Does this mean the first input event can get recorded as soon as tracking is
started? I thought we wanted it to happen at the end of the first timer period.

[dominickn, 2015/09/23 00:06:44]

I can put in a one-shot timer to add that effect back.

[calamity, 2015/09/24 03:50:53]

Hmm, maybe we can save that for a follow-up, to keep this CL slightly less
hairy.

Although, you may to rename timer_ to pause_timer_ or similar to make adding the
one-shot easier.

+  return false;
+}
+
+bool SiteEngagementHelper::InputTracker::HandleMouseEvent(
+    const blink::WebMouseEvent& event) {
+  PauseTracking(helper_->web_contents()->GetRenderViewHost());
+  helper_->RecordUserInput();
+  return false;
+}
+
+void SiteEngagementHelper::InputTracker::StartTracking(
+    content::RenderViewHost* host) {
+  if (!callbacks_added_ && !gDisableCallbackRegistrationForTesting) {
+    host->AddKeyPressEventCallback(key_press_event_callback_);
+    host->AddMouseEventCallback(mouse_event_callback_);
+    callbacks_added_ = true;
+  }
+}
+
+void SiteEngagementHelper::InputTracker::PauseTracking(
+    content::RenderViewHost* host) {
+  StopTracking(host);
+  timer_->Start(FROM_HERE,
+                base::TimeDelta::FromSeconds(gSecondsBetweenUserInputCheck),
+                base::Bind(&SiteEngagementHelper::InputTracker::ResumeTracking,
+                           base::Unretained(this)));
+}
+
+void SiteEngagementHelper::InputTracker::ResumeTracking() {
+  content::WebContents* contents = helper_->web_contents();
+  if (contents)
+    StartTracking(contents->GetRenderViewHost());
+}
+
+void SiteEngagementHelper::InputTracker::StopTracking(
+    content::RenderViewHost* host) {
+  timer_->Stop();
+  if (callbacks_added_) {
+    host->RemoveKeyPressEventCallback(key_press_event_callback_);
+    host->RemoveMouseEventCallback(mouse_event_callback_);
+    callbacks_added_ = false;
+  }
 }
 
-SiteEngagementHelper::SiteEngagementHelper(content::WebContents* web_contents)
-    : content::WebContentsObserver(web_contents) {
+SiteEngagementHelper::~SiteEngagementHelper() {
+  content::WebContents* contents = web_contents();
+  if (contents)
+    tracker_->StopTracking(contents->GetRenderViewHost());
 }
 
-void SiteEngagementHelper::DidStartNavigationToPendingEntry(
-    const GURL& url,
-    content::NavigationController::ReloadType reload_type) {
-  prerender::PrerenderContents* prerender_contents =
-      prerender::PrerenderContents::FromWebContents(web_contents());
+SiteEngagementHelper::SiteEngagementHelper(content::WebContents* contents)
+    : content::WebContentsObserver(contents),
+      tracker_(new InputTracker(this)),
+      record_engagement_(false) { }
+
+void SiteEngagementHelper::RecordUserInput() {
+  content::WebContents* contents = web_contents();
+  if (contents) {
+    Profile* profile =
+        Profile::FromBrowserContext(contents->GetBrowserContext());
+    SiteEngagementService* service =
+        SiteEngagementServiceFactory::GetForProfile(profile);
 
-  // Ignore pre-render loads.
-  if (prerender_contents != NULL)
-    return;
+    // Service is null in incognito.
+    if (service)
+      service->HandleUserInput(contents->GetVisibleURL());
+  }
+}
+
+void SiteEngagementHelper::DidNavigateMainFrame(
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  content::WebContents* contents = web_contents();
+  tracker_->StopTracking(contents->GetRenderViewHost());
 
   // Ignore all schemes except HTTP and HTTPS.
-  if (!url.SchemeIsHTTPOrHTTPS())
-    return;
-
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents()->GetBrowserContext());
-  SiteEngagementService* service =
-      SiteEngagementServiceFactory::GetForProfile(profile);
-  // Service is null in incognito.
-  if (!service)
-    return;
-
-  service->HandleNavigation(url);
+  record_engagement_ = params.url.SchemeIsHTTPOrHTTPS();

[calamity, 2015/09/22 02:57:21]

Needing this bool is a bit unfortunate. Ideally, we'd be able to ask the input
tracker whether or not it's tracking.

Can we use WebContents::FromRenderViewHost() to get the WC and then use
WC::GetLastCommittedURL() to check the origin on the input tracker side?

Then we can have an IsTracking() method on InputTracker.

[dominickn, 2015/09/23 00:06:44]

The issue I have with this is that it's the Helper which definitely knows what
the origin is. The input tracker may be sleeping or disabled during a
navigation, and it's up to the helper to tell the tracker that we've navigated
and it's time to wake up.

[calamity, 2015/09/24 03:50:53]

Acknowledged.

+
+  if (record_engagement_) {

[calamity, 2015/09/22 02:57:21]

Invert and early return.

[dominickn, 2015/09/23 00:06:44]

Done.

+    Profile* profile =
+        Profile::FromBrowserContext(contents->GetBrowserContext());
+    SiteEngagementService* service =
+        SiteEngagementServiceFactory::GetForProfile(profile);
+
+    if (service)
+      service->HandleNavigation(params.url, params.transition);
+
+    tracker_->StartTracking(contents->GetRenderViewHost());
+  }
+}
+
+void SiteEngagementHelper::RenderViewHostChanged(
+    content::RenderViewHost* old_host,
+    content::RenderViewHost* new_host) {
+  // On changing the render view host, we need to re-register the callbacks
+  // listening for user input.
+  if (record_engagement_) {
+    if (old_host)
+      tracker_->StopTracking(old_host);
+    tracker_->StartTracking(new_host);
+  }
+}
+
+void SiteEngagementHelper::WasShown() {
+  // Ensure that the input callbacks are registered when we come into view.
+  if (record_engagement_)
+    tracker_->StartTracking(web_contents()->GetRenderViewHost());
+}
+
+void SiteEngagementHelper::WasHidden() {
+  // Ensure that the input callbacks are not registered when hidden.
+  if (record_engagement_) {
+    content::WebContents* contents = web_contents();
+    if (contents)
+      tracker_->StopTracking(contents->GetRenderViewHost());
+  }
+}
+
+// static
+void SiteEngagementHelper::SetSecondsBetweenUserInputCheck(double seconds) {

[calamity, 2015/09/22 02:57:20]

SetSecondsBetweenUserInputCheckForTestsing

[dominickn, 2015/09/23 00:06:44]

I'm assuming that this may be Finch'ed at some point in the future, so I left
off the ForTesting.

[calamity, 2015/09/24 03:50:53]

Acknowledged.

+  gSecondsBetweenUserInputCheck = seconds;
+}
+

[calamity, 2015/09/22 02:57:20]

// static

[dominickn, 2015/09/23 00:06:44]

Done.

+void SiteEngagementHelper::DisableCallbackRegistrationForTesting() {
+  gDisableCallbackRegistrationForTesting = true;
 }