Test group policy for UKM.

Refactored the pref-setting logic out of metrics_reporting_state_browsertest in a way it could be reused in UKM. Setup a parametrized UKM test which tests the enabled/disabled group policy state based on prefs and verified service is enabled/disabled.

Bug: 805983
Change-Id: Ic303e4116826990e7780125f186b0028a8fd6a5e
Reviewed-on: https://chromium-review.googlesource.com/895390
Commit-Queue: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534086}

chrome/browser/BUILD.gn

@@ -735,6 +735,8 @@ split_static_library("browser") {
     "metrics/sampling_metrics_provider.h",
     "metrics/subprocess_metrics_provider.cc",
     "metrics/subprocess_metrics_provider.h",
+    "metrics/testing/metrics_reporting_pref_helper.cc",
+    "metrics/testing/metrics_reporting_pref_helper.h",
     "metrics/thread_watcher.cc",
     "metrics/thread_watcher.h",
     "metrics/thread_watcher_android.cc",

chrome/browser/metrics/chrome_metrics_service_accessor.h

@@ -45,6 +45,10 @@ class ChromeMetricsPrivateDelegate;
 class FileManagerPrivateIsUMAEnabledFunction;
 }
 
+namespace metrics {
+class UkmConsentParamBrowserTest;
+}
+
 namespace metrics_services_manager {
 class MetricsServicesManager;
 }
@@ -139,6 +143,7 @@ class ChromeMetricsServiceAccessor : public metrics::MetricsServiceAccessor {
 
   // Testing related friends.
   friend class MetricsReportingStateTest;
+  friend class metrics::UkmConsentParamBrowserTest;
   FRIEND_TEST_ALL_PREFIXES(ChromeMetricsServiceAccessorTest,
                            MetricsReportingEnabled);
 

chrome/browser/metrics/metrics_reporting_state_browsertest.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/chrome_browser_main.h"
 #include "chrome/browser/chrome_browser_main_extra_parts.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/browser/metrics/testing/metrics_reporting_pref_helper.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/pref_names.h"
@@ -29,22 +30,6 @@
 #include "components/policy/proto/device_management_backend.pb.h"
 #endif
 
-#if defined(OS_CHROMEOS)
-void SetMetricsReportingEnabledChromeOS(
-    bool value,
-    base::DictionaryValue* local_state_dict) {
-  namespace em = enterprise_management;
-  em::ChromeDeviceSettingsProto device_settings_proto;
-  device_settings_proto.mutable_metrics_enabled()->set_metrics_enabled(value);
-  em::PolicyData policy_data;
-  policy_data.set_policy_type("google/chromeos/device");
-  policy_data.set_policy_value(device_settings_proto.SerializeAsString());
-  local_state_dict->SetString(
-      prefs::kDeviceSettingsCache,
-      chromeos::device_settings_cache::PolicyDataToString(policy_data));
-}
-#endif
-
 // ChromeBrowserMainExtraParts implementation that asserts the metrics and
 // reporting state matches a particular value in PreCreateThreads().
 class ChromeBrowserMainExtraPartsChecker : public ChromeBrowserMainExtraParts {
@@ -83,25 +68,9 @@ class MetricsReportingStateTest : public InProcessBrowserTest,
 
   // InProcessBrowserTest overrides:
   bool SetUpUserDataDirectory() override {
-    // Set up local state so that that value for reporting is
-    // is_metrics_reporting_enabled_initial_value().
-    base::DictionaryValue local_state_dict;
-    local_state_dict.SetBoolean(metrics::prefs::kMetricsReportingEnabled,
-                                is_metrics_reporting_enabled_initial_value());
-
-    base::FilePath user_data_dir;
-    if (!PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
-      return false;
-
-#if defined(OS_CHROMEOS)
-    // ChromeOS checks a separate place for reporting enabled.
-    SetMetricsReportingEnabledChromeOS(
-        is_metrics_reporting_enabled_initial_value(), &local_state_dict);
-#endif
-
-    local_state_path_ = user_data_dir.Append(chrome::kLocalStateFilename);
-    return JSONFileValueSerializer(local_state_path_)
-        .Serialize(local_state_dict);
+    local_state_path_ = metrics::SetUpUserDataDirectoryForTesting(
+        is_metrics_reporting_enabled_initial_value());
+    return !local_state_path_.empty();
   }
 
   void CreatedBrowserMainParts(content::BrowserMainParts* parts) override {

chrome/browser/metrics/testing/metrics_reporting_pref_helper.cc

+// Copyright 2018 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/testing/metrics_reporting_pref_helper.h"
+
+#include "base/files/file_path.h"
+#include "base/json/json_file_value_serializer.h"
+#include "base/path_service.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/pref_names.h"
+#include "components/metrics/metrics_pref_names.h"
+
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/settings/device_settings_cache.h"
+#include "components/policy/proto/chrome_device_policy.pb.h"
+#include "components/policy/proto/device_management_backend.pb.h"
+#endif
+
+#if defined(OS_CHROMEOS)
+namespace {
+
+void SetMetricsReportingEnabledChromeOS(
+    bool is_enabled,
+    base::DictionaryValue* local_state_dict) {
+  namespace em = enterprise_management;
+  em::ChromeDeviceSettingsProto device_settings_proto;
+  device_settings_proto.mutable_metrics_enabled()->set_metrics_enabled(
+      is_enabled);
+  em::PolicyData policy_data;
+  policy_data.set_policy_type("google/chromeos/device");
+  policy_data.set_policy_value(device_settings_proto.SerializeAsString());
+  local_state_dict->SetString(
+      prefs::kDeviceSettingsCache,
+      chromeos::device_settings_cache::PolicyDataToString(policy_data));
+}
+
+}  // namespace
+#endif
+
+namespace metrics {
+
+base::FilePath SetUpUserDataDirectoryForTesting(bool is_enabled) {
+  base::DictionaryValue local_state_dict;
+  local_state_dict.SetBoolean(metrics::prefs::kMetricsReportingEnabled,
+                              is_enabled);
+
+  base::FilePath user_data_dir;
+  if (!PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
+    return base::FilePath();
+
+#if defined(OS_CHROMEOS)
+  // ChromeOS checks a separate place for reporting enabled.
+  SetMetricsReportingEnabledChromeOS(is_enabled, &local_state_dict);
+#endif
+
+  base::FilePath local_state_path =
+      user_data_dir.Append(chrome::kLocalStateFilename);
+  if (!JSONFileValueSerializer(local_state_path).Serialize(local_state_dict))
+    return base::FilePath();
+  return local_state_path;
+}
+
+}  // namespace metrics

chrome/browser/metrics/testing/metrics_reporting_pref_helper.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_METRICS_TESTING_METRICS_REPORTING_PREF_HELPER_H_
+#define CHROME_BROWSER_METRICS_TESTING_METRICS_REPORTING_PREF_HELPER_H_
+
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/path_service.h"
+#include "base/values.h"
+
+namespace metrics {
+
+// Configures on-disk prefs to mark that metrics reporting is enabled/disabled
+// based on input |is_enabled|. This is used in browser tests to setup the
+// correct input conditions to be validated. This should generally be called
+// within SetUpUserDataDirectory. Returns the filepath of the local state file
+// which can be used for future verification. Returns empty filepath if there
+// was an error.
+base::FilePath SetUpUserDataDirectoryForTesting(bool is_enabled);
+
+}  // namespace metrics
+
+#endif  // CHROME_BROWSER_METRICS_TESTING_METRICS_REPORTING_PREF_HELPER_H_

chrome/browser/metrics/ukm_browsertest.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/metrics/chrome_metrics_services_manager_client.h"
+#include "chrome/browser/metrics/testing/metrics_reporting_pref_helper.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
@@ -189,6 +190,42 @@ class UkmBrowserTest : public SyncTest {
     return g_browser_process->GetMetricsServicesManager()->GetUkmService();
   }
   base::test::ScopedFeatureList scoped_feature_list_;
+  DISALLOW_COPY_AND_ASSIGN(UkmBrowserTest);
+};
+
+// This tests if UKM service is enabled/disabled appropriately based on an
+// input bool param. The bool reflects if metrics reporting state is
+// enabled/disabled via prefs.
+class UkmConsentParamBrowserTest : public UkmBrowserTest,
+                                   public testing::WithParamInterface<bool> {
+ public:
+  UkmConsentParamBrowserTest() : UkmBrowserTest() {}
+
+  static bool IsMetricsAndCrashReportingEnabled() {
+    return ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled(
+        g_browser_process->local_state());
+  }
+
+  // InProcessBrowserTest overrides.
+  bool SetUpUserDataDirectory() override {
+    local_state_path_ = SetUpUserDataDirectoryForTesting(
+        is_metrics_reporting_enabled_initial_value());
+    return !local_state_path_.empty();
+  }
+
+  void CreatedBrowserMainParts(content::BrowserMainParts* parts) override {
+    // IsMetricsReportingEnabled() in non-official builds always returns false.
+    // Enable the official build checks so that this test can work in both
+    // official and non-official builds.
+    ChromeMetricsServiceAccessor::SetForceIsMetricsReportingEnabledPrefLookup(
+        true);
+  }
+
+  bool is_metrics_reporting_enabled_initial_value() const { return GetParam(); }
+
+ private:
+  base::FilePath local_state_path_;
+  DISALLOW_COPY_AND_ASSIGN(UkmConsentParamBrowserTest);
 };
 
 class UkmEnabledChecker : public SingleClientStatusChangeChecker {
@@ -212,6 +249,7 @@ class UkmEnabledChecker : public SingleClientStatusChangeChecker {
  private:
   UkmBrowserTest* const test_;
   const bool want_enabled_;
+  DISALLOW_COPY_AND_ASSIGN(UkmEnabledChecker);
 };
 
 // Make sure that UKM is disabled while an incognito window is open.
@@ -577,4 +615,26 @@ IN_PROC_BROWSER_TEST_F(UkmBrowserTest, HistoryDeleteCheck) {
   CloseBrowserSynchronously(sync_browser);
 }
 
+IN_PROC_BROWSER_TEST_P(UkmConsentParamBrowserTest, GroupPolicyConsentCheck) {
+  // Note we are not using the synthetic MetricsConsentOverride since we are
+  // testing directly from prefs.
+
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  std::unique_ptr<ProfileSyncServiceHarness> harness =
+      EnableSyncForProfile(profile);
+
+  // The input param controls whether we set the prefs related to group policy
+  // enabled or not. Based on its value, we should report the same value for
+  // both if reporting is enabled and if UKM service is enabled.
+  bool is_enabled = is_metrics_reporting_enabled_initial_value();
+  EXPECT_EQ(is_enabled,
+            UkmConsentParamBrowserTest::IsMetricsAndCrashReportingEnabled());
+  EXPECT_EQ(is_enabled, ukm_enabled());
+}
+
+// Verify UKM is enabled/disabled for both potential settings of group policy.
+INSTANTIATE_TEST_CASE_P(UkmConsentParamBrowserTests,
+                        UkmConsentParamBrowserTest,
+                        testing::Bool());
+
 }  // namespace metrics