Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1009)

Unified Diff: chrome/browser/engagement/site_engagement_helper.cc

Issue 1373363005: Add a delay to Site Engagement tracking after navigation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: address comment Created 5 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 7f1bfe5961390c7145db1c591798936834f36878..96c6db05bffdf06ebd249247ac303f66c0a8cf37 100644
--- a/chrome/browser/engagement/site_engagement_helper.cc
+++ b/chrome/browser/engagement/site_engagement_helper.cc
@@ -15,7 +15,9 @@
namespace {
-double g_seconds_between_user_input_check = 10;
+int g_seconds_between_user_input_check = 10;
+int g_seconds_tracking_delay_after_navigation = 10;
+int g_seconds_tracking_delay_after_show = 5;
} // anonymous namespace
@@ -24,7 +26,9 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(SiteEngagementHelper);
SiteEngagementHelper::InputTracker::InputTracker(SiteEngagementHelper* helper)
: helper_(helper),
pause_timer_(new base::Timer(true, false)),
- callbacks_added_(false) {
+ host_(nullptr),
+ is_active_(false),
+ is_tracking_(false) {
key_press_event_callback_ =
base::Bind(&SiteEngagementHelper::InputTracker::HandleKeyPressEvent,
base::Unretained(this));
@@ -33,7 +37,7 @@ SiteEngagementHelper::InputTracker::InputTracker(SiteEngagementHelper* helper)
base::Unretained(this));
}
-SiteEngagementHelper::InputTracker::~InputTracker() { }
+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
@@ -45,7 +49,7 @@ bool SiteEngagementHelper::InputTracker::HandleKeyPressEvent(
// Only respond to raw key down to avoid multiple triggering on a single input
// (e.g. keypress is a key down then key up).
if (event.type == blink::WebInputEvent::RawKeyDown) {
- PauseTracking(helper_->web_contents()->GetRenderViewHost());
+ Pause();
helper_->RecordUserInput(SiteEngagementMetrics::ENGAGEMENT_KEYPRESS);
}
return false;
@@ -59,62 +63,91 @@ bool SiteEngagementHelper::InputTracker::HandleMouseEvent(
if ((event.button != blink::WebMouseEvent::ButtonNone &&
event.type == blink::WebInputEvent::MouseDown) ||
event.type == blink::WebInputEvent::MouseWheel) {
- PauseTracking(helper_->web_contents()->GetRenderViewHost());
+ Pause();
helper_->RecordUserInput(SiteEngagementMetrics::ENGAGEMENT_MOUSE);
}
return false;
}
-void SiteEngagementHelper::InputTracker::StartTracking(
- content::RenderViewHost* host) {
- if (!callbacks_added_) {
- host->AddKeyPressEventCallback(key_press_event_callback_);
- host->AddMouseEventCallback(mouse_event_callback_);
- callbacks_added_ = true;
- }
+void SiteEngagementHelper::InputTracker::Start(content::RenderViewHost* host,
+ base::TimeDelta initial_delay) {
+ DCHECK(!is_active_);
+ DCHECK(host);
+ host_ = host;
+ StartTimer(initial_delay);
+ is_active_ = true;
}
-void SiteEngagementHelper::InputTracker::PauseTracking(
- content::RenderViewHost* host) {
- StopTracking(host);
- pause_timer_->Start(
- FROM_HERE,
- base::TimeDelta::FromSeconds(g_seconds_between_user_input_check),
- base::Bind(&SiteEngagementHelper::InputTracker::ResumeTracking,
- base::Unretained(this)));
+void SiteEngagementHelper::InputTracker::Pause() {
+ RemoveCallbacks();
+ StartTimer(base::TimeDelta::FromSeconds(g_seconds_between_user_input_check));
}
-void SiteEngagementHelper::InputTracker::ResumeTracking() {
- content::WebContents* contents = helper_->web_contents();
- if (contents)
- StartTracking(contents->GetRenderViewHost());
+void SiteEngagementHelper::InputTracker::SwitchRenderViewHost(
+ content::RenderViewHost* old_host,
+ content::RenderViewHost* new_host) {
+ DCHECK(is_tracking_);
+ DCHECK(new_host);
+
+ bool was_tracking = is_tracking_;
+ if (old_host) {
+ DCHECK_EQ(host_, old_host);
+ RemoveCallbacks();
+ }
+
+ host_ = new_host;
+
+ if (was_tracking)
+ AddCallbacks();
}
-void SiteEngagementHelper::InputTracker::StopTracking(
- content::RenderViewHost* host) {
+void SiteEngagementHelper::InputTracker::Stop() {
pause_timer_->Stop();
- if (callbacks_added_) {
- host->RemoveKeyPressEventCallback(key_press_event_callback_);
- host->RemoveMouseEventCallback(mouse_event_callback_);
- callbacks_added_ = false;
- }
+ RemoveCallbacks();
+ host_ = nullptr;
+ is_active_ = false;
}
-void SiteEngagementHelper::InputTracker::SetTimerForTesting(
+void SiteEngagementHelper::InputTracker::SetPauseTimerForTesting(
scoped_ptr<base::Timer> timer) {
pause_timer_ = timer.Pass();
}
+void SiteEngagementHelper::InputTracker::StartTimer(base::TimeDelta delay) {
+ pause_timer_->Start(
+ FROM_HERE, delay,
+ base::Bind(&SiteEngagementHelper::InputTracker::AddCallbacks,
+ base::Unretained(this)));
+}
+
+void SiteEngagementHelper::InputTracker::AddCallbacks() {
+ content::WebContents* contents = helper_->web_contents();
+ if (!contents)
+ return;
+
+ host_->AddKeyPressEventCallback(key_press_event_callback_);
+ host_->AddMouseEventCallback(mouse_event_callback_);
+ is_tracking_ = true;
+}
+
+void SiteEngagementHelper::InputTracker::RemoveCallbacks() {
+ if (is_tracking_) {
+ host_->RemoveKeyPressEventCallback(key_press_event_callback_);
+ host_->RemoveMouseEventCallback(mouse_event_callback_);
+ is_tracking_ = false;
+ }
+}
+
SiteEngagementHelper::~SiteEngagementHelper() {
content::WebContents* contents = web_contents();
if (contents)
- input_tracker_.StopTracking(contents->GetRenderViewHost());
+ input_tracker_.Stop();
}
SiteEngagementHelper::SiteEngagementHelper(content::WebContents* contents)
: content::WebContentsObserver(contents),
input_tracker_(this),
- record_engagement_(false) { }
+ record_engagement_(false) {}
void SiteEngagementHelper::RecordUserInput(
SiteEngagementMetrics::EngagementType type) {
@@ -132,31 +165,28 @@ void SiteEngagementHelper::RecordUserInput(
}
}
-bool SiteEngagementHelper::ShouldRecordEngagement() {
- return record_engagement_;
-}
-
void SiteEngagementHelper::DidNavigateMainFrame(
const content::LoadCommittedDetails& details,
const content::FrameNavigateParams& params) {
- content::WebContents* contents = web_contents();
- input_tracker_.StopTracking(contents->GetRenderViewHost());
+ input_tracker_.Stop();
- // Ignore all schemes except HTTP and HTTPS.
record_engagement_ = params.url.SchemeIsHTTPOrHTTPS();
- if (!ShouldRecordEngagement())
+ // Ignore all schemes except HTTP and HTTPS.
+ if (!record_engagement_)
return;
Profile* profile =
- Profile::FromBrowserContext(contents->GetBrowserContext());
+ Profile::FromBrowserContext(web_contents()->GetBrowserContext());
SiteEngagementService* service =
SiteEngagementServiceFactory::GetForProfile(profile);
if (service)
service->HandleNavigation(params.url, params.transition);
- input_tracker_.StartTracking(contents->GetRenderViewHost());
+ input_tracker_.Start(
+ web_contents()->GetRenderViewHost(),
+ base::TimeDelta::FromSeconds(g_seconds_tracking_delay_after_navigation));
}
void SiteEngagementHelper::RenderViewHostChanged(
@@ -164,29 +194,36 @@ void SiteEngagementHelper::RenderViewHostChanged(
content::RenderViewHost* new_host) {
// On changing the render view host, we need to re-register the callbacks
// listening for user input.
- if (ShouldRecordEngagement()) {
- if (old_host)
- input_tracker_.StopTracking(old_host);
- input_tracker_.StartTracking(new_host);
+ if (input_tracker_.is_tracking()) {
+ input_tracker_.SwitchRenderViewHost(old_host, new_host);
}
}
void SiteEngagementHelper::WasShown() {
// Ensure that the input callbacks are registered when we come into view.
- if (ShouldRecordEngagement())
- input_tracker_.StartTracking(web_contents()->GetRenderViewHost());
+ if (record_engagement_) {
+ input_tracker_.Start(
+ web_contents()->GetRenderViewHost(),
+ base::TimeDelta::FromSeconds(g_seconds_tracking_delay_after_show));
+ }
}
void SiteEngagementHelper::WasHidden() {
// Ensure that the input callbacks are not registered when hidden.
- if (ShouldRecordEngagement()) {
- content::WebContents* contents = web_contents();
- if (contents)
- input_tracker_.StopTracking(contents->GetRenderViewHost());
- }
+ input_tracker_.Stop();
}
// static
-void SiteEngagementHelper::SetSecondsBetweenUserInputCheck(double seconds) {
+void SiteEngagementHelper::SetSecondsBetweenUserInputCheck(int seconds) {
g_seconds_between_user_input_check = seconds;
}
+
+// static
+void SiteEngagementHelper::SetSecondsTrackingDelayAfterNavigation(int seconds) {
+ g_seconds_tracking_delay_after_navigation = seconds;
+}
+
+// static
+void SiteEngagementHelper::SetSecondsTrackingDelayAfterShow(int seconds) {
+ g_seconds_tracking_delay_after_show = seconds;
+}
« no previous file with comments | « chrome/browser/engagement/site_engagement_helper.h ('k') | chrome/browser/engagement/site_engagement_service_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698