[M54] Merge DesktopEngagementService fixes.

chrome/browser/metrics/desktop_engagement/desktop_engagement_service.cc

Index: chrome/browser/metrics/desktop_engagement/desktop_engagement_service.cc
diff --git a/chrome/browser/metrics/desktop_engagement/desktop_engagement_service.cc b/chrome/browser/metrics/desktop_engagement/desktop_engagement_service.cc
index 92e6c59601e2a4dcaed014cd8a6ef4c6081c6b84..c63c2fd2b060fca32f3de1f4c1270c4f40ac0212 100644
--- a/chrome/browser/metrics/desktop_engagement/desktop_engagement_service.cc
+++ b/chrome/browser/metrics/desktop_engagement/desktop_engagement_service.cc
@@ -15,6 +15,8 @@ namespace {
 
 DesktopEngagementService* g_instance = nullptr;
 
+const base::TimeDelta kZeroTime = base::TimeDelta::FromSeconds(0);

[Alexei Svitkine (slow), 2016/10/12 15:34:38]

Hmm, I'm actually surprised this didn't introduce a static initializer in the
original CL, which we generally don't permit. Anyway, no concerns here since
it's merging the same code that landed, but I'm curious why this didn't make
bots yell at us.

[chrisha, 2016/10/12 17:36:21]

Due to fancy newfangled constexpr semantics, this is now safe to do:

https://cs.chromium.org/chromium/src/base/time/time.h?q=base::TimeDelta&sq=pa...

+
 }  // namespace
 
 // static
@@ -39,13 +41,17 @@ void DesktopEngagementService::StartTimer(base::TimeDelta duration) {
                           weak_factory_.GetWeakPtr()));
 }
 
-void DesktopEngagementService::OnVisibilityChanged(bool visible) {
+void DesktopEngagementService::OnVisibilityChanged(
+    bool visible,
+    base::TimeDelta time_ago) {
   is_visible_ = visible;
   if (is_visible_ && !is_first_session_) {
+    DCHECK(time_ago.is_zero());
     OnUserEvent();
   } else if (in_session_ && !is_audio_playing_) {
+    DCHECK(!visible);
     DVLOG(4) << "Ending session due to visibility change";
-    EndSession();
+    EndSession(time_ago);
   }
 }
 
@@ -77,7 +83,7 @@ void DesktopEngagementService::OnAudioEnd() {
   // last 5 minutes so the session can be terminated.
   if (!timer_.IsRunning()) {
     DVLOG(4) << "Ending session due to audio ending";
-    EndSession();
+    EndSession(kZeroTime);
   }
 }
 
@@ -102,7 +108,7 @@ void DesktopEngagementService::OnTimerFired() {
   // No user events happened in the last 5 min. Terminate the session now.
   if (!is_audio_playing_) {
     DVLOG(4) << "Ending session after delay";
-    EndSession();
+    EndSession(inactivity_timeout_);
   }
 }
 
@@ -113,10 +119,17 @@ void DesktopEngagementService::StartSession() {
   StartTimer(inactivity_timeout_);
 }
 
-void DesktopEngagementService::EndSession() {
+void DesktopEngagementService::EndSession(base::TimeDelta time_to_discount) {
   in_session_ = false;
 
   base::TimeDelta delta = base::TimeTicks::Now() - session_start_;
+
+  // Trim any timeouts from the session length and lower bound to a session of
+  // length 0.
+  delta -= time_to_discount;
+  if (delta < kZeroTime)
+    delta = kZeroTime;
+
   DVLOG(4) << "Logging session length of " << delta.InSeconds() << " seconds.";
 
   // Note: This metric is recorded separately for Android in