metrics: Listen to session manager for screen lock/unlock signals.
This should fix a pretty broken DailyUseTime metric. Also, use
contstants from service_constants.h where available.
BUG=chromium:216382,chromium:234799
TEST=unit tests; stop metrics_daemon on the device, then ran metrics_daemon
--nodaemon and inspected console log output while logging in/out as well as on
suspend/resume.
CQ-DEPEND=Ibfcc54c8dbf145cccd5ef871c7c9701b8312eb9e
Change-Id: I69ca0ea9594a496453f0933147ec66b0fa334718
Reviewed-on: https://gerrit.chromium.org/gerrit/49468
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Chris Masone <cmasone@chromium.org>
Commit-Queue: Darin Petkov <petkov@chromium.org>
diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc
index 798d022..a786548 100644
--- a/metrics/metrics_daemon.cc
+++ b/metrics/metrics_daemon.cc
@@ -13,6 +13,7 @@
#include <base/logging.h>
#include <base/string_util.h>
#include <base/stringprintf.h>
+#include <chromeos/dbus/service_constants.h>
#include <dbus/dbus-glib-lowlevel.h>
#include "counter.h"
@@ -26,9 +27,9 @@
#define SAFE_MESSAGE(e) (e.message ? e.message : "unknown error")
-#define DBUS_IFACE_CRASH_REPORTER "org.chromium.CrashReporter"
-#define DBUS_IFACE_POWER_MANAGER "org.chromium.PowerManager"
-#define DBUS_IFACE_SESSION_MANAGER "org.chromium.SessionManagerInterface"
+
+static const char kCrashReporterInterface[] = "org.chromium.CrashReporter";
+static const char kCrashReporterUserCrashSignal[] = "UserCrash";
static const int kSecondsPerMinute = 60;
static const int kMinutesPerHour = 60;
@@ -130,25 +131,6 @@
// persistent metrics path
const char MetricsDaemon::kMetricsPath[] = "/var/log/metrics";
-
-// static
-const char* MetricsDaemon::kDBusMatches_[] = {
- "type='signal',"
- "interface='" DBUS_IFACE_CRASH_REPORTER "',"
- "path='/',"
- "member='UserCrash'",
-
- "type='signal',"
- "interface='" DBUS_IFACE_POWER_MANAGER "',"
- "path='/'",
-
- "type='signal',"
- "sender='org.chromium.SessionManager',"
- "interface='" DBUS_IFACE_SESSION_MANAGER "',"
- "path='/org/chromium/SessionManager',"
- "member='SessionStateChanged'",
-};
-
// static
const char* MetricsDaemon::kPowerStates_[] = {
#define STATE(name, capname) #name,
@@ -318,9 +300,26 @@
dbus_connection_setup_with_g_main(connection, NULL);
+ vector<string> matches;
+ matches.push_back(
+ StringPrintf("type='signal',interface='%s',path='/',member='%s'",
+ kCrashReporterInterface,
+ kCrashReporterUserCrashSignal));
+ matches.push_back(
+ StringPrintf("type='signal',interface='%s',path='%s',member='%s'",
+ power_manager::kPowerManagerInterface,
+ power_manager::kPowerManagerServicePath,
+ power_manager::kPowerStateChangedSignal));
+ matches.push_back(
+ StringPrintf("type='signal',sender='%s',interface='%s',path='%s'",
+ login_manager::kSessionManagerServiceName,
+ login_manager::kSessionManagerInterface,
+ login_manager::kSessionManagerServicePath));
+
// Registers D-Bus matches for the signals we would like to catch.
- for (unsigned int m = 0; m < arraysize(kDBusMatches_); m++) {
- const char* match = kDBusMatches_[m];
+ for (vector<string>::const_iterator it = matches.begin();
+ it != matches.end(); ++it) {
+ const char* match = it->c_str();
DLOG(INFO) << "adding dbus match: " << match;
dbus_bus_add_match(connection, match, &error);
LOG_IF(FATAL, dbus_error_is_set(&error)) <<
@@ -359,28 +358,27 @@
DBusMessageIter iter;
dbus_message_iter_init(message, &iter);
- if (strcmp(interface, DBUS_IFACE_CRASH_REPORTER) == 0) {
+ if (strcmp(interface, kCrashReporterInterface) == 0) {
CHECK(strcmp(dbus_message_get_member(message),
- "UserCrash") == 0);
+ kCrashReporterUserCrashSignal) == 0);
daemon->ProcessUserCrash();
- } else if (strcmp(interface, DBUS_IFACE_POWER_MANAGER) == 0) {
- const char* member = dbus_message_get_member(message);
- if (strcmp(member, "ScreenIsLocked") == 0) {
- daemon->SetUserActiveState(false, now);
- } else if (strcmp(member, "ScreenIsUnlocked") == 0) {
- daemon->SetUserActiveState(true, now);
- } else if (strcmp(member, "PowerStateChanged") == 0) {
- char* state_name;
- dbus_message_iter_get_basic(&iter, &state_name);
- daemon->PowerStateChanged(state_name, now);
- }
- } else if (strcmp(interface, DBUS_IFACE_SESSION_MANAGER) == 0) {
+ } else if (strcmp(interface, power_manager::kPowerManagerInterface) == 0) {
CHECK(strcmp(dbus_message_get_member(message),
- "SessionStateChanged") == 0);
-
+ power_manager::kPowerStateChangedSignal) == 0);
char* state_name;
dbus_message_iter_get_basic(&iter, &state_name);
- daemon->SessionStateChanged(state_name, now);
+ daemon->PowerStateChanged(state_name, now);
+ } else if (strcmp(interface, login_manager::kSessionManagerInterface) == 0) {
+ const char* member = dbus_message_get_member(message);
+ if (strcmp(member, login_manager::kScreenIsLockedSignal) == 0) {
+ daemon->SetUserActiveState(false, now);
+ } else if (strcmp(member, login_manager::kScreenIsUnlockedSignal) == 0) {
+ daemon->SetUserActiveState(true, now);
+ } else if (strcmp(member, login_manager::kSessionStateChangedSignal) == 0) {
+ char* state_name;
+ dbus_message_iter_get_basic(&iter, &state_name);
+ daemon->SessionStateChanged(state_name, now);
+ }
} else {
DLOG(WARNING) << "unexpected interface: " << interface;
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h
index 0bebbc7..07eaa01 100644
--- a/metrics/metrics_daemon.h
+++ b/metrics/metrics_daemon.h
@@ -158,9 +158,6 @@
static const char kMetricsDiskStatsPath[];
static const char kMetricsVmStatsPath[];
- // D-Bus message match strings.
- static const char* kDBusMatches_[];
-
// Array of power states.
static const char* kPowerStates_[kNumberPowerStates];
diff --git a/metrics/metrics_daemon_test.cc b/metrics/metrics_daemon_test.cc
index d7ce735..370634c 100644
--- a/metrics/metrics_daemon_test.cc
+++ b/metrics/metrics_daemon_test.cc
@@ -9,6 +9,7 @@
#include <base/file_util.h>
#include <base/stringprintf.h>
+#include <chromeos/dbus/service_constants.h>
#include <gtest/gtest.h>
#include "counter_mock.h"
@@ -355,9 +356,9 @@
signal_args.clear();
signal_args.push_back("on");
- msg = NewDBusSignalString("/",
- "org.chromium.PowerManager",
- "PowerStateChanged",
+ msg = NewDBusSignalString(power_manager::kPowerManagerServicePath,
+ power_manager::kPowerManagerInterface,
+ power_manager::kPowerStateChangedSignal,
signal_args);
EXPECT_EQ(MetricsDaemon::kUnknownPowerState, daemon_.power_state_);
res = MetricsDaemon::MessageFilter(/* connection */ NULL, msg, &daemon_);
@@ -367,9 +368,9 @@
signal_args.clear();
IgnoreActiveUseUpdate();
- msg = NewDBusSignalString("/",
- "org.chromium.PowerManager",
- "ScreenIsUnlocked",
+ msg = NewDBusSignalString(login_manager::kSessionManagerServicePath,
+ login_manager::kSessionManagerInterface,
+ login_manager::kScreenIsUnlockedSignal,
signal_args);
EXPECT_FALSE(daemon_.user_active_);
res = MetricsDaemon::MessageFilter(/* connection */ NULL, msg, &daemon_);
@@ -381,9 +382,9 @@
signal_args.clear();
signal_args.push_back("started");
signal_args.push_back("bob"); // arbitrary username
- msg = NewDBusSignalString("/org/chromium/SessionManager",
- "org.chromium.SessionManagerInterface",
- "SessionStateChanged",
+ msg = NewDBusSignalString(login_manager::kSessionManagerServicePath,
+ login_manager::kSessionManagerInterface,
+ login_manager::kSessionStateChangedSignal,
signal_args);
EXPECT_EQ(MetricsDaemon::kUnknownSessionState, daemon_.session_state_);
res = MetricsDaemon::MessageFilter(/* connection */ NULL, msg, &daemon_);