am fcaef687: Merge "metricsd: Don\'t cache the metrics status in the daemon."

* commit 'fcaef687010bc63293087c8232dad2cd0af02b5f':
  metricsd: Don't cache the metrics status in the daemon.

metricsd/include/metrics/metrics_library.h

@@ -48,6 +48,12 @@ class MetricsLibrary : public MetricsLibraryInterface {
   // Initializes the library.
   void Init() override;
 
+  // Initializes the library and disables the cache of whether or not the
+  // metrics collection is enabled.
+  // By disabling this, we may have to check for the metrics status more often
+  // but the result will never be stale.
+  void InitWithNoCaching();
+
   // Returns whether or not the machine is running in guest mode.
   bool IsGuestMode();
 
@@ -125,6 +131,7 @@ class MetricsLibrary : public MetricsLibraryInterface {
   friend class MetricsLibraryTest;
   friend class UploadServiceTest;
   FRIEND_TEST(MetricsLibraryTest, AreMetricsEnabled);
+  FRIEND_TEST(MetricsLibraryTest, AreMetricsEnabledNoCaching);
   FRIEND_TEST(MetricsLibraryTest, FormatChromeMessage);
   FRIEND_TEST(MetricsLibraryTest, FormatChromeMessageTooLong);
   FRIEND_TEST(MetricsLibraryTest, IsDeviceMounted);
@@ -147,6 +154,9 @@ class MetricsLibrary : public MetricsLibraryInterface {
   // Cached state of whether or not metrics were enabled.
   bool cached_enabled_;
 
+  // True iff we should cache the enabled/disabled status.
+  bool use_caching_;
+
   base::FilePath uma_events_file_;
   base::FilePath consent_file_;
 

metricsd/metrics_daemon_main.cc

@@ -90,7 +90,7 @@ int main(int argc, char** argv) {
   }
 
   MetricsLibrary metrics_lib;
-  metrics_lib.Init();
+  metrics_lib.InitWithNoCaching();
   MetricsDaemon daemon;
   daemon.Init(FLAGS_uploader_test,
               FLAGS_uploader | FLAGS_uploader_test,

metricsd/metrics_library.cc

@@ -126,7 +126,7 @@ bool MetricsLibrary::IsGuestMode() {
 bool MetricsLibrary::AreMetricsEnabled() {
   static struct stat stat_buffer;
   time_t this_check_time = time(nullptr);
-  if (this_check_time != cached_enabled_time_) {
+  if (!use_caching_ || this_check_time != cached_enabled_time_) {
     cached_enabled_time_ = this_check_time;
     cached_enabled_ = stat(consent_file_.value().data(), &stat_buffer) >= 0;
   }
@@ -139,6 +139,12 @@ void MetricsLibrary::Init() {
   consent_file_ = dir.Append(metrics::kConsentFileName);
   cached_enabled_ = false;
   cached_enabled_time_ = 0;
+  use_caching_ = true;
+}
+
+void MetricsLibrary::InitWithNoCaching() {
+  Init();
+  use_caching_ = false;
 }
 
 void MetricsLibrary::InitForTest(const base::FilePath& metrics_directory) {
@@ -146,6 +152,7 @@ void MetricsLibrary::InitForTest(const base::FilePath& metrics_directory) {
   consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
   cached_enabled_ = false;
   cached_enabled_time_ = 0;
+  use_caching_ = true;
 }
 
 bool MetricsLibrary::SendToUMA(const std::string& name,

metricsd/metrics_library_test.cc

@@ -93,3 +93,16 @@ TEST_F(MetricsLibraryTest, AreMetricsEnabledCaching) {
   VerifyEnabledCacheEviction(false);
   VerifyEnabledCacheEviction(true);
 }
+
+TEST_F(MetricsLibraryTest, AreMetricsEnabledNoCaching) {
+  // disable caching.
+  lib_.use_caching_ = false;
+
+  // Checking the consent repeatedly should return the right result.
+  for (int i=0; i<100; ++i) {
+    SetMetricsConsent(true);
+    ASSERT_EQ(true, lib_.AreMetricsEnabled());
+    SetMetricsConsent(false);
+    ASSERT_EQ(false, lib_.AreMetricsEnabled());
+  }
+}