Add desktop engagement metrics.

chrome/browser/metrics/desktop_engagement/desktop_engagement_service.cc

+// Copyright 2016 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.
+
+#include "chrome/browser/metrics/desktop_engagement/desktop_engagement_service.h"
+
+#include "base/bind.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/string_number_conversions.h"
+#include "chrome/browser/metrics/desktop_engagement/audible_contents_tracker.h"
+#include "chrome/browser/metrics/desktop_engagement/chrome_visibility_observer.h"
+#include "components/variations/variations_associated_data.h"
+
+namespace {
+
+DesktopEngagementService* g_instance = nullptr;
+
+// Small adapter for observing AudibleContentsTracker notifications and
+// dispatching them to the DesktopEngagementService.
+class AudibleContentsObserver
+    : public metrics::AudibleContentsTracker::Observer {

[Alexei Svitkine (slow), 2016/07/26 22:00:40]

Please make things consistent re: using metrics namespace or not. I suggest
putting all the new classes in the namespace.

[gayane -on leave until 09-2017, 2016/07/27 21:10:25]

Done.

+ public:
+  AudibleContentsObserver() {}
+
+  // A one way initialization function. This causes a tracker/observer pair to
+  // be created, which live for the lifetime of the browser. They are
+  // deliberately leaked at shutdown.
+  static void Initialize() {
+    DCHECK(!tracker_);
+    DCHECK(!observer_);
+    observer_ = new AudibleContentsObserver();
+    tracker_ = new metrics::AudibleContentsTracker(observer_);
+  }
+
+  // AudibleContentsTracker::Observer:
+  void OnAudioStart() override { g_instance->OnAudioStart(); }
+  void OnAudioEnd() override { g_instance->OnAudioEnd(); }
+
+ private:
+  // Singletons for performing the tracking and observing state changes. These
+  // are deliberately leaked at shutdown.
+  static metrics::AudibleContentsTracker* tracker_;
+  static AudibleContentsObserver* observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(AudibleContentsObserver);
+};

[Alexei Svitkine (slow), 2016/07/26 22:00:40]

This seems overly complicated. Why not just have DesktopEngagementService be the
metrics::AudibleContentsTracker::Observer? Then you don't need this whole block
of code with extra statics - they can just be members of
DesktopEngagementService.

[gayane -on leave until 09-2017, 2016/07/27 21:10:25]

Moved this implementation to AudibleContentsTracker

+
+metrics::AudibleContentsTracker* AudibleContentsObserver::tracker_ = nullptr;
+AudibleContentsObserver* AudibleContentsObserver::observer_ = nullptr;
+
+}  // namespace
+
+// static
+void DesktopEngagementService::Initialize() {
+  g_instance = new DesktopEngagementService;
+  AudibleContentsObserver::Initialize();
+  metrics::ChromeVisibilityObserver::Initialize();
+}
+
+// static
+bool DesktopEngagementService::IsInitialized() {
+  return g_instance != nullptr;
+}
+
+// static
+DesktopEngagementService* DesktopEngagementService::Get() {
+  DCHECK(g_instance);
+  return g_instance;
+}
+
+void DesktopEngagementService::StartTimer(base::TimeDelta duration) {
+  timer_.Start(FROM_HERE, duration,
+               base::Bind(&DesktopEngagementService::OnTimerFired,
+                          weak_factory_.GetWeakPtr()));
+}
+
+void DesktopEngagementService::OnVisibilityChanged(bool visible) {
+  is_visible_ = visible;
+  if (is_visible_ && !is_first_session_) {
+    OnUserEvent();
+  } else if (in_session_ && !is_audio_playing_) {
+    DVLOG(4) << "Ending session due to visibility change";
+    EndSession();
+  }
+}
+
+void DesktopEngagementService::OnUserEvent() {
+  if (!is_visible_)
+    return;
+
+  last_user_event_ = base::TimeTicks::Now();
+  // This may start session.
+  if (!in_session_) {
+    DVLOG(4) << "Starting session due to user event";
+    StartSession();
+  }
+}
+
+void DesktopEngagementService::OnAudioStart() {
+  // This may start session.
+  is_audio_playing_ = true;
+  if (!in_session_) {
+    DVLOG(4) << "Starting session due to audio start";
+    StartSession();
+  }
+}
+
+void DesktopEngagementService::OnAudioEnd() {
+  is_audio_playing_ = false;
+
+  // If the timer is not running, this means that no user events happened in the
+  // last 5 minutes so the session can be terminated.
+  if (!timer_.IsRunning()) {
+    DVLOG(4) << "Ending session due to audio ending";
+    EndSession();
+  }
+}
+
+DesktopEngagementService::DesktopEngagementService()
+    : session_start_(base::TimeTicks::Now()),
+      last_user_event_(session_start_),
+      weak_factory_(this) {
+  InitInactivityTimeout();
+}
+
+DesktopEngagementService::~DesktopEngagementService() {}
+
+void DesktopEngagementService::OnTimerFired() {
+  base::TimeDelta remaining =
+      inactivity_timeout_ - (base::TimeTicks::Now() - last_user_event_);
+  if (remaining.ToInternalValue() > 0) {
+    StartTimer(remaining);
+    return;
+  }
+
+  // No user events happened in the last 5 min. Terminate the session now.
+  if (!is_audio_playing_) {
+    DVLOG(4) << "Ending session after delay";
+    EndSession();
+  }
+}
+
+void DesktopEngagementService::StartSession() {
+  in_session_ = true;
+  is_first_session_ = false;
+  session_start_ = base::TimeTicks::Now();
+  StartTimer(inactivity_timeout_);
+}
+
+void DesktopEngagementService::EndSession() {
+  in_session_ = false;
+  base::TimeDelta delta = base::TimeTicks::Now() - session_start_;
+
+  constexpr unsigned kNumTimeSlices = 60 / 5 * 24;
+  DVLOG(4) << "Logging session length of " << delta.InSeconds()
+              << " seconds.";

[Alexei Svitkine (slow), 2016/07/26 22:00:40]

Nit: Align <<'s

Or just git cl format, which I think does the right thing.

[gayane -on leave until 09-2017, 2016/07/27 21:10:25]

Done.

+  UMA_HISTOGRAM_CUSTOM_TIMES("Session.TotalDuration", delta,
+                             base::TimeDelta::FromMilliseconds(1),
+                             base::TimeDelta::FromHours(24), kNumTimeSlices);

[Alexei Svitkine (slow), 2016/07/26 22:00:41]

Looks like the metric on android uses a different macro, here:

 UMA_HISTOGRAM_LONG_TIMES("Session.TotalDuration", duration);

https://cs.chromium.org/chromium/src/chrome/browser/android/metrics/uma_sessi...

I suggest just using the same macro as there so that the buckets are the same
when not aggregating by platform.

Please also add a comment to both here and the other file mentioning the other
call site, so if someone decides to update the macro used, they'll know of the
other location.

[gayane -on leave until 09-2017, 2016/07/27 21:10:25]

Done.

+}
+
+void DesktopEngagementService::InitInactivityTimeout() {
+  const int kDefaultInactivityTimeoutMinutes = 5;
+
+  int timeout_minutes = kDefaultInactivityTimeoutMinutes;
+  std::string param_value = variations::GetVariationParamValue(
+      "DesktopEngagement", "inactivity_timeout");
+  if (!param_value.empty())
+    base::StringToInt(param_value, &timeout_minutes);
+
+  inactivity_timeout_ = base::TimeDelta::FromMinutes(timeout_minutes);
+}