Make the metrics library respect the policy settings instead of the consent file.
BUG=chromium-os:17012
TEST=metrics_library_test
Change-Id: I133b19a672f52d5ba4f150aa75d1b32af2896c3f
Reviewed-on: http://gerrit.chromium.org/gerrit/3865
Reviewed-by: Ken Mixter <kmixter@chromium.org>
Tested-by: Ken Mixter <kmixter@chromium.org>
diff --git a/metrics/Makefile b/metrics/Makefile
index 032ea73..2920547 100644
--- a/metrics/Makefile
+++ b/metrics/Makefile
@@ -39,16 +39,17 @@
TESTCOUNTER_LIBS = -lgmock -lgtest -lbase -lrt -lpthread -lglib-2.0
DAEMON_LDFLAGS = $(LDFLAGS) $(LDCONFIG) -lrt -lbase -lpthread -lgflags \
- -lglib-2.0 -lrootdev
+ -lglib-2.0 -lrootdev -lpolicy
TESTDAEMON_LIBS = -lgmock -lgtest
-TESTLIB_LIBS = -lgtest -lbase -lrt -lpthread -lglib-2.0
+TESTLIB_LIBS = -lgtest -lgmock -lbase -lrt -lpthread -lglib-2.0
+POLICY_LIBS = -lpolicy
all: $(LIB) $(SHAREDLIB) $(CLIENT) $(DAEMON)
tests: $(COUNTER_TEST) $(DAEMON_TEST) $(LIB_TEST)
$(CLIENT): $(CLIENT_OBJS) $(SHAREDLIB)
- $(CXX) $(LDFLAGS) $^ -o $@
+ $(CXX) $(LDFLAGS) $(POLICY_LIBS) -lrt $^ -o $@
$(COUNTER_TEST): $(TESTCOUNTER_OBJS)
$(CXX) -o $@ $^ $(TESTCOUNTER_LIBS)
@@ -63,10 +64,10 @@
$(AR) rcs $@ $^
$(SHAREDLIB): $(LIB_OBJS)
- $(CXX) $(LDFLAGS) -shared $^ -o $@
+ $(CXX) $(LDFLAGS) $(POLICY_LIBS) -shared $^ -o $@
$(LIB_TEST): $(TESTLIB_OBJS) $(SHAREDLIB)
- $(CXX) -o $@ $^ $(LDFLAGS) $(TESTLIB_LIBS)
+ $(CXX) -o $@ $^ $(LDFLAGS) $(POLICY_LIBS) $(TESTLIB_LIBS)
%.o: %.cc
$(CXX) $(CXXFLAGS) -c $< -o $@
diff --git a/metrics/metrics_library.cc b/metrics/metrics_library.cc
index d2c1b0f..ae6c8e0 100644
--- a/metrics/metrics_library.cc
+++ b/metrics/metrics_library.cc
@@ -13,6 +13,7 @@
#include <cstring>
#include "base/eintr_wrapper.h" // HANDLE_EINTR macro, no libbase required.
+#include "policy/device_policy.h"
#define READ_WRITE_ALL_FILE_FLAGS \
(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)
@@ -60,7 +61,8 @@
MetricsLibrary::MetricsLibrary()
: uma_events_file_(NULL),
- consent_file_(kConsentFile) {}
+ consent_file_(kConsentFile),
+ policy_provider_(NULL) {}
// We take buffer and buffer_size as parameters in order to simplify testing
// of various alignments of the |device_name| with |buffer_size|.
@@ -130,13 +132,21 @@
}
bool MetricsLibrary::AreMetricsEnabled() {
- static struct stat stat_buffer;
time_t this_check_time = time(NULL);
+ if (!policy_provider_.get())
+ policy_provider_.reset(new policy::PolicyProvider());
+ else
+ policy_provider_->Reload();
+
+ // We initialize with the default value which is false and will be preserved
+ // if the policy is not set.
+ bool enabled = false;
+ if (policy_provider_->device_policy_is_loaded())
+ policy_provider_->GetDevicePolicy().GetMetricsEnabled(&enabled);
if (this_check_time != cached_enabled_time_) {
cached_enabled_time_ = this_check_time;
- if (stat(consent_file_, &stat_buffer) >= 0 &&
- !IsGuestMode())
+ if (enabled && !IsGuestMode())
cached_enabled_ = true;
else
cached_enabled_ = false;
@@ -283,3 +293,7 @@
// Send the message.
return SendMessageToChrome(message_length, message);
}
+
+void MetricsLibrary::SetPolicyProvider(policy::PolicyProvider* provider) {
+ policy_provider_.reset(provider);
+}
diff --git a/metrics/metrics_library.h b/metrics/metrics_library.h
index 3f860eb..456610f 100644
--- a/metrics/metrics_library.h
+++ b/metrics/metrics_library.h
@@ -8,8 +8,11 @@
#include <sys/types.h>
#include <string>
+#include <base/scoped_ptr.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
+#include "policy/libpolicy.h"
+
class MetricsLibraryInterface {
public:
virtual void Init() = 0;
@@ -122,6 +125,9 @@
int32_t FormatChromeMessage(int32_t buffer_size, char* buffer,
const char* format, ...);
+ // This function is used by tests only to mock the device policies.
+ void SetPolicyProvider(policy::PolicyProvider* provider);
+
// Time at which we last checked if metrics were enabled.
static time_t cached_enabled_time_;
@@ -130,6 +136,8 @@
const char* uma_events_file_;
const char* consent_file_;
+
+ scoped_ptr<policy::PolicyProvider> policy_provider_;
};
#endif // METRICS_LIBRARY_H_
diff --git a/metrics/metrics_library_test.cc b/metrics/metrics_library_test.cc
index 762be4a..3b3dd95 100644
--- a/metrics/metrics_library_test.cc
+++ b/metrics/metrics_library_test.cc
@@ -5,20 +5,24 @@
#include <cstring>
#include <base/file_util.h>
+#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include <policy/mock_device_policy.h>
+#include <policy/libpolicy.h>
#include "c_metrics_library.h"
#include "metrics_library.h"
static const FilePath kTestUMAEventsFile("test-uma-events");
-static const char kTestConsent[] = "test-consent";
static const char kTestMounts[] = "test-mounts";
-static void SetMetricsEnabled(bool enabled) {
- if (enabled)
- ASSERT_EQ(1, file_util::WriteFile(FilePath(kTestConsent) , "0", 1));
- else
- file_util::Delete(FilePath(kTestConsent), false);
+using ::testing::_;
+using ::testing::Return;
+using ::testing::AnyNumber;
+
+ACTION_P(SetMetricsPolicy, enabled) {
+ *arg0 = enabled;
+ return true;
}
class MetricsLibraryTest : public testing::Test {
@@ -28,14 +32,20 @@
lib_.Init();
EXPECT_TRUE(NULL != lib_.uma_events_file_);
lib_.uma_events_file_ = kTestUMAEventsFile.value().c_str();
- SetMetricsEnabled(true);
+ device_policy_ = new policy::MockDevicePolicy();
+ EXPECT_CALL(*device_policy_, LoadPolicy())
+ .Times(AnyNumber())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .Times(AnyNumber())
+ .WillRepeatedly(SetMetricsPolicy(true));
+ provider_ = new policy::PolicyProvider(device_policy_);
+ lib_.SetPolicyProvider(provider_);
// Defeat metrics enabled caching between tests.
lib_.cached_enabled_time_ = 0;
- lib_.consent_file_ = kTestConsent;
}
virtual void TearDown() {
- file_util::Delete(FilePath(kTestConsent), false);
file_util::Delete(FilePath(kTestMounts), false);
file_util::Delete(kTestUMAEventsFile, false);
}
@@ -44,6 +54,8 @@
void VerifyEnabledCacheEviction(bool to_value);
MetricsLibrary lib_;
+ policy::MockDevicePolicy* device_policy_;
+ policy::PolicyProvider* provider_;
};
TEST_F(MetricsLibraryTest, IsDeviceMounted) {
@@ -106,7 +118,8 @@
}
TEST_F(MetricsLibraryTest, AreMetricsEnabledFalse) {
- SetMetricsEnabled(false);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(false));
EXPECT_FALSE(lib_.AreMetricsEnabled());
}
@@ -119,9 +132,11 @@
// times in a row.
for (int i = 0; i < 100; ++i) {
lib_.cached_enabled_time_ = 0;
- SetMetricsEnabled(!to_value);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(!to_value));
ASSERT_EQ(!to_value, lib_.AreMetricsEnabled());
- SetMetricsEnabled(to_value);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(to_value));
if (lib_.AreMetricsEnabled() == !to_value)
return;
}
@@ -130,9 +145,11 @@
void MetricsLibraryTest::VerifyEnabledCacheEviction(bool to_value) {
lib_.cached_enabled_time_ = 0;
- SetMetricsEnabled(!to_value);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(!to_value));
ASSERT_EQ(!to_value, lib_.AreMetricsEnabled());
- SetMetricsEnabled(to_value);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(to_value));
ASSERT_LT(abs(time(NULL) - lib_.cached_enabled_time_), 5);
// Sleep one second (or cheat to be faster).
--lib_.cached_enabled_time_;
@@ -174,7 +191,8 @@
}
TEST_F(MetricsLibraryTest, SendEnumToUMANotEnabled) {
- SetMetricsEnabled(false);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(false));
EXPECT_TRUE(lib_.SendEnumToUMA("Test.EnumMetric", 1, 3));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -209,7 +227,8 @@
}
TEST_F(MetricsLibraryTest, SendToUMANotEnabled) {
- SetMetricsEnabled(false);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(false));
EXPECT_TRUE(lib_.SendToUMA("Test.Metric", 2, 1, 100, 50));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -226,7 +245,8 @@
}
TEST_F(MetricsLibraryTest, SendUserActionToUMANotEnabled) {
- SetMetricsEnabled(false);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(false));
EXPECT_TRUE(lib_.SendUserActionToUMA("SomeOtherKeyPressed"));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -243,7 +263,8 @@
}
TEST_F(MetricsLibraryTest, SendCrashToUMANotEnabled) {
- SetMetricsEnabled(false);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(false));
EXPECT_TRUE(lib_.SendCrashToUMA("kernel"));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -257,9 +278,16 @@
CMetricsLibraryInit(lib_);
EXPECT_TRUE(NULL != ml.uma_events_file_);
ml.uma_events_file_ = kTestUMAEventsFile.value().c_str();
- SetMetricsEnabled(true);
+ device_policy_ = new policy::MockDevicePolicy();
+ EXPECT_CALL(*device_policy_, LoadPolicy())
+ .Times(AnyNumber())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .Times(AnyNumber())
+ .WillRepeatedly(SetMetricsPolicy(true));
+ provider_ = new policy::PolicyProvider(device_policy_);
+ ml.SetPolicyProvider(provider_);
reinterpret_cast<MetricsLibrary*>(lib_)->cached_enabled_time_ = 0;
- reinterpret_cast<MetricsLibrary*>(lib_)->consent_file_ = kTestConsent;
}
virtual void TearDown() {
@@ -268,10 +296,13 @@
}
CMetricsLibrary lib_;
+ policy::MockDevicePolicy* device_policy_;
+ policy::PolicyProvider* provider_;
};
TEST_F(CMetricsLibraryTest, AreMetricsEnabledFalse) {
- SetMetricsEnabled(false);
+ EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
+ .WillOnce(SetMetricsPolicy(false));
EXPECT_FALSE(CMetricsLibraryAreMetricsEnabled(lib_));
}
