Revert "Make the metrics library respect the policy settings instead of the consent file."
This reverts commit 8c874ca26dd0d07471cfc66ded527ad5c3a1cd20
Change-Id: Iaa91d046ac27a9bdc96e6cd9438741d98f1cef66
Reviewed-on: http://gerrit.chromium.org/gerrit/4583
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Tested-by: Julian Pastarmov <pastarmovj@chromium.org>
diff --git a/metrics/Makefile b/metrics/Makefile
index 2920547..032ea73 100644
--- a/metrics/Makefile
+++ b/metrics/Makefile
@@ -39,17 +39,16 @@
TESTCOUNTER_LIBS = -lgmock -lgtest -lbase -lrt -lpthread -lglib-2.0
DAEMON_LDFLAGS = $(LDFLAGS) $(LDCONFIG) -lrt -lbase -lpthread -lgflags \
- -lglib-2.0 -lrootdev -lpolicy
+ -lglib-2.0 -lrootdev
TESTDAEMON_LIBS = -lgmock -lgtest
-TESTLIB_LIBS = -lgtest -lgmock -lbase -lrt -lpthread -lglib-2.0
-POLICY_LIBS = -lpolicy
+TESTLIB_LIBS = -lgtest -lbase -lrt -lpthread -lglib-2.0
all: $(LIB) $(SHAREDLIB) $(CLIENT) $(DAEMON)
tests: $(COUNTER_TEST) $(DAEMON_TEST) $(LIB_TEST)
$(CLIENT): $(CLIENT_OBJS) $(SHAREDLIB)
- $(CXX) $(LDFLAGS) $(POLICY_LIBS) -lrt $^ -o $@
+ $(CXX) $(LDFLAGS) $^ -o $@
$(COUNTER_TEST): $(TESTCOUNTER_OBJS)
$(CXX) -o $@ $^ $(TESTCOUNTER_LIBS)
@@ -64,10 +63,10 @@
$(AR) rcs $@ $^
$(SHAREDLIB): $(LIB_OBJS)
- $(CXX) $(LDFLAGS) $(POLICY_LIBS) -shared $^ -o $@
+ $(CXX) $(LDFLAGS) -shared $^ -o $@
$(LIB_TEST): $(TESTLIB_OBJS) $(SHAREDLIB)
- $(CXX) -o $@ $^ $(LDFLAGS) $(POLICY_LIBS) $(TESTLIB_LIBS)
+ $(CXX) -o $@ $^ $(LDFLAGS) $(TESTLIB_LIBS)
%.o: %.cc
$(CXX) $(CXXFLAGS) -c $< -o $@
diff --git a/metrics/metrics_library.cc b/metrics/metrics_library.cc
index ae6c8e0..d2c1b0f 100644
--- a/metrics/metrics_library.cc
+++ b/metrics/metrics_library.cc
@@ -13,7 +13,6 @@
#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)
@@ -61,8 +60,7 @@
MetricsLibrary::MetricsLibrary()
: uma_events_file_(NULL),
- consent_file_(kConsentFile),
- policy_provider_(NULL) {}
+ consent_file_(kConsentFile) {}
// We take buffer and buffer_size as parameters in order to simplify testing
// of various alignments of the |device_name| with |buffer_size|.
@@ -132,21 +130,13 @@
}
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 (enabled && !IsGuestMode())
+ if (stat(consent_file_, &stat_buffer) >= 0 &&
+ !IsGuestMode())
cached_enabled_ = true;
else
cached_enabled_ = false;
@@ -293,7 +283,3 @@
// 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 456610f..3f860eb 100644
--- a/metrics/metrics_library.h
+++ b/metrics/metrics_library.h
@@ -8,11 +8,8 @@
#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;
@@ -125,9 +122,6 @@
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_;
@@ -136,8 +130,6 @@
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 3b3dd95..762be4a 100644
--- a/metrics/metrics_library_test.cc
+++ b/metrics/metrics_library_test.cc
@@ -5,24 +5,20 @@
#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";
-using ::testing::_;
-using ::testing::Return;
-using ::testing::AnyNumber;
-
-ACTION_P(SetMetricsPolicy, enabled) {
- *arg0 = enabled;
- return true;
+static void SetMetricsEnabled(bool enabled) {
+ if (enabled)
+ ASSERT_EQ(1, file_util::WriteFile(FilePath(kTestConsent) , "0", 1));
+ else
+ file_util::Delete(FilePath(kTestConsent), false);
}
class MetricsLibraryTest : public testing::Test {
@@ -32,20 +28,14 @@
lib_.Init();
EXPECT_TRUE(NULL != lib_.uma_events_file_);
lib_.uma_events_file_ = kTestUMAEventsFile.value().c_str();
- 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_);
+ SetMetricsEnabled(true);
// 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);
}
@@ -54,8 +44,6 @@
void VerifyEnabledCacheEviction(bool to_value);
MetricsLibrary lib_;
- policy::MockDevicePolicy* device_policy_;
- policy::PolicyProvider* provider_;
};
TEST_F(MetricsLibraryTest, IsDeviceMounted) {
@@ -118,8 +106,7 @@
}
TEST_F(MetricsLibraryTest, AreMetricsEnabledFalse) {
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(false));
+ SetMetricsEnabled(false);
EXPECT_FALSE(lib_.AreMetricsEnabled());
}
@@ -132,11 +119,9 @@
// times in a row.
for (int i = 0; i < 100; ++i) {
lib_.cached_enabled_time_ = 0;
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(!to_value));
+ SetMetricsEnabled(!to_value);
ASSERT_EQ(!to_value, lib_.AreMetricsEnabled());
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(to_value));
+ SetMetricsEnabled(to_value);
if (lib_.AreMetricsEnabled() == !to_value)
return;
}
@@ -145,11 +130,9 @@
void MetricsLibraryTest::VerifyEnabledCacheEviction(bool to_value) {
lib_.cached_enabled_time_ = 0;
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(!to_value));
+ SetMetricsEnabled(!to_value);
ASSERT_EQ(!to_value, lib_.AreMetricsEnabled());
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(to_value));
+ SetMetricsEnabled(to_value);
ASSERT_LT(abs(time(NULL) - lib_.cached_enabled_time_), 5);
// Sleep one second (or cheat to be faster).
--lib_.cached_enabled_time_;
@@ -191,8 +174,7 @@
}
TEST_F(MetricsLibraryTest, SendEnumToUMANotEnabled) {
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(false));
+ SetMetricsEnabled(false);
EXPECT_TRUE(lib_.SendEnumToUMA("Test.EnumMetric", 1, 3));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -227,8 +209,7 @@
}
TEST_F(MetricsLibraryTest, SendToUMANotEnabled) {
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(false));
+ SetMetricsEnabled(false);
EXPECT_TRUE(lib_.SendToUMA("Test.Metric", 2, 1, 100, 50));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -245,8 +226,7 @@
}
TEST_F(MetricsLibraryTest, SendUserActionToUMANotEnabled) {
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(false));
+ SetMetricsEnabled(false);
EXPECT_TRUE(lib_.SendUserActionToUMA("SomeOtherKeyPressed"));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -263,8 +243,7 @@
}
TEST_F(MetricsLibraryTest, SendCrashToUMANotEnabled) {
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(false));
+ SetMetricsEnabled(false);
EXPECT_TRUE(lib_.SendCrashToUMA("kernel"));
EXPECT_FALSE(file_util::PathExists(kTestUMAEventsFile));
}
@@ -278,16 +257,9 @@
CMetricsLibraryInit(lib_);
EXPECT_TRUE(NULL != ml.uma_events_file_);
ml.uma_events_file_ = kTestUMAEventsFile.value().c_str();
- 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_);
+ SetMetricsEnabled(true);
reinterpret_cast<MetricsLibrary*>(lib_)->cached_enabled_time_ = 0;
+ reinterpret_cast<MetricsLibrary*>(lib_)->consent_file_ = kTestConsent;
}
virtual void TearDown() {
@@ -296,13 +268,10 @@
}
CMetricsLibrary lib_;
- policy::MockDevicePolicy* device_policy_;
- policy::PolicyProvider* provider_;
};
TEST_F(CMetricsLibraryTest, AreMetricsEnabledFalse) {
- EXPECT_CALL(*device_policy_, GetMetricsEnabled(_))
- .WillOnce(SetMetricsPolicy(false));
+ SetMetricsEnabled(false);
EXPECT_FALSE(CMetricsLibraryAreMetricsEnabled(lib_));
}
