Use helper macros to define base::Features in //components, part 2

This allows: - features to be defined with a consistent set of qualifiers, and for that set of qualifiers to be updated over time as appropriate. - better PRESUBMIT checks to ensure that base::Features are not defined in headers. - simplifies things for scripts trying to extract feature definitions out of C++ code. The primary CL was generated using a script that automatically rewrites base::Feature declarations and definitions to the macro form. Changes to any files with known incompatibilities with the macros (base::Features without static storage duration and base::Features declared as static class members) were then fully reverted; those changes will be manually handled in followups. This is a manual cleanup pass to fix up features that were defined as static class members. Bug: 1364289 Change-Id: Iffbf8c564373f00aa428bfcf302ec6f5e10468af Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3928433 Commit-Queue: Lei Zhang <thestig@chromium.org> Auto-Submit: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Owners-Override: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/main@{#1054023}
chromium · Oct 3, 2022 · be539e0 · be539e0
1 parent f40175e
commit be539e0
Show file tree

Hide file tree

Showing 30 changed files with 78 additions and 91 deletions.
diff --git a/android_webview/lib/aw_main_delegate.cc b/android_webview/lib/aw_main_delegate.cc
@@ -293,8 +293,7 @@ absl::optional<int> AwMainDelegate::BasicStartupComplete() {
 
     features.DisableIfNotSet(::features::kInstalledApp);
 
-    features.EnableIfNotSet(
-        metrics::UnsentLogStoreMetrics::kRecordLastUnsentLogMetadataMetrics);
+    features.EnableIfNotSet(metrics::kRecordLastUnsentLogMetadataMetrics);
 
     features.DisableIfNotSet(::features::kPeriodicBackgroundSync);
 

diff --git a/chrome/browser/chromeos/reporting/metric_default_utils.cc b/chrome/browser/chromeos/reporting/metric_default_utils.cc
@@ -15,8 +15,7 @@ const base::TimeDelta GetDefaultRate(base::TimeDelta default_rate,
                                      base::TimeDelta testing_rate) {
   // If telemetry testing rates flag is enabled, use `testing_rate` to reduce
   // time before metric collection and reporting.
-  return base::FeatureList::IsEnabled(
-             MetricRateController::kEnableTelemetryTestingRates)
+  return base::FeatureList::IsEnabled(kEnableTelemetryTestingRates)
              ? testing_rate
              : default_rate;
 }

diff --git a/chrome/browser/metrics/majority_age_user_metrics_provider_browsertest.cc b/chrome/browser/metrics/majority_age_user_metrics_provider_browsertest.cc
@@ -53,7 +53,7 @@ class MajorityAgeUserMetricsProviderTest
  public:
   MajorityAgeUserMetricsProviderTest() : SyncTest(SINGLE_CLIENT) {
     scoped_feature_list_.InitAndEnableFeature(
-        metrics::DemographicMetricsProvider::kDemographicMetricsReporting);
+        metrics::kDemographicMetricsReporting);
   }
 
   int GetAge() { return GetParam(); }
@@ -117,7 +117,7 @@ class MajorityAgeUserMetricsProviderGuestModeTest
  public:
   MajorityAgeUserMetricsProviderGuestModeTest() {
     scoped_feature_list_.InitAndEnableFeature(
-        metrics::DemographicMetricsProvider::kDemographicMetricsReporting);
+        metrics::kDemographicMetricsReporting);
   }
 
  private:

diff --git a/chrome/browser/metrics/metrics_service_user_demographics_browsertest.cc b/chrome/browser/metrics/metrics_service_user_demographics_browsertest.cc
@@ -47,16 +47,15 @@ class MetricsServiceUserDemographicsBrowserTest
       // Enable UMA and reporting of the synced user's birth year and gender.
       scoped_feature_list_.InitWithFeatures(
           // enabled_features =
-          {internal::kMetricsReportingFeature,
-           DemographicMetricsProvider::kDemographicMetricsReporting},
+          {internal::kMetricsReportingFeature, kDemographicMetricsReporting},
           // disabled_features =
           {});
     } else {
       scoped_feature_list_.InitWithFeatures(
           // enabled_features =
           {internal::kMetricsReportingFeature},
           // disabled_features =
-          {DemographicMetricsProvider::kDemographicMetricsReporting});
+          {kDemographicMetricsReporting});
     }
   }
 

diff --git a/chrome/browser/metrics/ukm_browsertest.cc b/chrome/browser/metrics/ukm_browsertest.cc
@@ -426,17 +426,17 @@ class UkmBrowserTestWithDemographics
     if (param.enable_feature) {
       scoped_feature_list_.InitWithFeatures(
           // enabled_features
-          {DemographicMetricsProvider::kDemographicMetricsReporting,
-           ukm::UkmService::kReportUserNoisedUserBirthYearAndGender},
+          {kDemographicMetricsReporting,
+           ukm::kReportUserNoisedUserBirthYearAndGender},
           // disabled_features
           {});
     } else {
       scoped_feature_list_.InitWithFeatures(
           // enabled_features
           {},
           // disabled_features
-          {DemographicMetricsProvider::kDemographicMetricsReporting,
-           ukm::UkmService::kReportUserNoisedUserBirthYearAndGender});
+          {kDemographicMetricsReporting,
+           ukm::kReportUserNoisedUserBirthYearAndGender});
     }
   }
 

diff --git a/components/metrics/call_stack_profile_metrics_provider.cc b/components/metrics/call_stack_profile_metrics_provider.cc
@@ -229,9 +229,9 @@ PendingProfiles::PendingProfiles() = default;
 
 // CallStackProfileMetricsProvider --------------------------------------------
 
-const base::Feature
-    CallStackProfileMetricsProvider::kSamplingProfilerReporting = {
-        "SamplingProfilerReporting", base::FEATURE_ENABLED_BY_DEFAULT};
+BASE_FEATURE(kSamplingProfilerReporting,
+             "SamplingProfilerReporting",
+             base::FEATURE_ENABLED_BY_DEFAULT);
 
 CallStackProfileMetricsProvider::CallStackProfileMetricsProvider() = default;
 CallStackProfileMetricsProvider::~CallStackProfileMetricsProvider() = default;

diff --git a/components/metrics/call_stack_profile_metrics_provider.h b/components/metrics/call_stack_profile_metrics_provider.h
@@ -17,6 +17,9 @@ namespace metrics {
 
 class ChromeUserMetricsExtension;
 
+// base::Feature for reporting CPU profiles. Provided here for test use.
+BASE_DECLARE_FEATURE(kSamplingProfilerReporting);
+
 // Performs metrics logging for the stack sampling profiler.
 class CallStackProfileMetricsProvider : public MetricsProvider {
  public:
@@ -61,9 +64,6 @@ class CallStackProfileMetricsProvider : public MetricsProvider {
       ChromeUserMetricsExtension* uma_proto) override;
 
  protected:
-  // base::Feature for reporting CPU profiles. Provided here for test use.
-  static const base::Feature kSamplingProfilerReporting;
-
   // Reset the static state to the defaults after startup.
   static void ResetStaticStateForTesting();
 };

diff --git a/components/metrics/call_stack_profile_metrics_provider_unittest.cc b/components/metrics/call_stack_profile_metrics_provider_unittest.cc
@@ -19,8 +19,7 @@ class CallStackProfileMetricsProviderTest : public testing::Test {
  public:
   CallStackProfileMetricsProviderTest() {
     TestState::ResetStaticStateForTesting();
-    scoped_feature_list_.InitAndEnableFeature(
-        TestState::kSamplingProfilerReporting);
+    scoped_feature_list_.InitAndEnableFeature(kSamplingProfilerReporting);
   }
 
   CallStackProfileMetricsProviderTest(
@@ -32,7 +31,6 @@ class CallStackProfileMetricsProviderTest : public testing::Test {
   // Exposes the feature from the CallStackProfileMetricsProvider.
   class TestState : public CallStackProfileMetricsProvider {
    public:
-    using CallStackProfileMetricsProvider::kSamplingProfilerReporting;
     using CallStackProfileMetricsProvider::ResetStaticStateForTesting;
   };
 
@@ -260,8 +258,7 @@ TEST_F(CallStackProfileMetricsProviderTest, HeapProfileProvidedWhenEnabled) {
 // Finch is disabled.
 TEST_F(CallStackProfileMetricsProviderTest, CpuProfileNotProvidedWithoutFinch) {
   base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndDisableFeature(
-      TestState::kSamplingProfilerReporting);
+  scoped_feature_list.InitAndDisableFeature(kSamplingProfilerReporting);
   CallStackProfileMetricsProvider provider;
   base::TimeTicks profile_start_time = base::TimeTicks::Now();
 

diff --git a/components/metrics/demographics/demographic_metrics_provider.cc b/components/metrics/demographics/demographic_metrics_provider.cc
@@ -35,8 +35,9 @@ bool CanUploadDemographicsToGoogle(syncer::SyncService* sync_service) {
 }  // namespace
 
 // static
-const base::Feature DemographicMetricsProvider::kDemographicMetricsReporting = {
-    "DemographicMetricsReporting", base::FEATURE_ENABLED_BY_DEFAULT};
+BASE_FEATURE(kDemographicMetricsReporting,
+             "DemographicMetricsReporting",
+             base::FEATURE_ENABLED_BY_DEFAULT);
 
 DemographicMetricsProvider::DemographicMetricsProvider(
     std::unique_ptr<ProfileClient> profile_client,

diff --git a/components/metrics/demographics/demographic_metrics_provider.h b/components/metrics/demographics/demographic_metrics_provider.h
@@ -25,6 +25,9 @@ class SyncService;
 
 namespace metrics {
 
+// Feature switch to report user's noised birth year and gender.
+BASE_DECLARE_FEATURE(kDemographicMetricsReporting);
+
 // Provider of the synced user’s noised birth year and gender to the UMA metrics
 // server. The synced user's birth year and gender were provided to Google when
 // the user created their Google account, to use in accordance with Google's
@@ -96,9 +99,6 @@ class DemographicMetricsProvider : public MetricsProvider,
   void ProvideSyncedUserNoisedBirthYearAndGenderToReport(
       ukm::Report* report) override;
 
-  // Feature switch to report user's noised birth year and gender.
-  static const base::Feature kDemographicMetricsReporting;
-
  private:
   // Provides the synced user's noised birth year and gender.
   absl::optional<UserDemographics> ProvideSyncedUserNoisedBirthYearAndGender();

diff --git a/components/metrics/demographics/demographic_metrics_provider_unittest.cc b/components/metrics/demographics/demographic_metrics_provider_unittest.cc
@@ -250,8 +250,7 @@ TEST(DemographicMetricsProviderTest,
      ProvideSyncedUserNoisedBirthYearAndGender_FeatureDisabled) {
   // Disable demographics reporting feature.
   base::test::ScopedFeatureList local_feature;
-  local_feature.InitAndDisableFeature(
-      DemographicMetricsProvider::kDemographicMetricsReporting);
+  local_feature.InitAndDisableFeature(kDemographicMetricsReporting);
 
   base::HistogramTester histogram;
 

diff --git a/components/metrics/unsent_log_store_metrics.cc b/components/metrics/unsent_log_store_metrics.cc
@@ -7,8 +7,9 @@
 namespace metrics {
 
 // static
-const base::Feature UnsentLogStoreMetrics::kRecordLastUnsentLogMetadataMetrics =
-    {"RecordLastUnsentLogMetadataMetrics", base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kRecordLastUnsentLogMetadataMetrics,
+             "RecordLastUnsentLogMetadataMetrics",
+             base::FEATURE_DISABLED_BY_DEFAULT);
 
 UnsentLogStoreMetrics::UnsentLogStoreMetrics() = default;
 UnsentLogStoreMetrics::~UnsentLogStoreMetrics() = default;

diff --git a/components/metrics/unsent_log_store_metrics.h b/components/metrics/unsent_log_store_metrics.h
@@ -10,6 +10,10 @@
 
 namespace metrics {
 
+// The feature to record the unsent log info metrics, refer to
+// UnsentLogStoreMetricsImpl::RecordLastUnsentLogMetadataMetrics.
+BASE_DECLARE_FEATURE(kRecordLastUnsentLogMetadataMetrics);
+
 // Interface for recording metrics from UnsentLogStore.
 class UnsentLogStoreMetrics {
  public:
@@ -50,10 +54,6 @@ class UnsentLogStoreMetrics {
   virtual void RecordLastUnsentLogMetadataMetrics(int unsent_samples_count,
                                                   int sent_samples_count,
                                                   int persisted_size_in_kb);
-
-  // The feature to record the unsent log info metrics, refer to
-  // UnsentLogStoreMetricsImpl::RecordLastUnsentLogMetadataMetrics.
-  static const base::Feature kRecordLastUnsentLogMetadataMetrics;
 };
 
 }  // namespace metrics

diff --git a/components/metrics/unsent_log_store_metrics_impl_unittest.cc b/components/metrics/unsent_log_store_metrics_impl_unittest.cc
@@ -28,8 +28,7 @@ TEST(UnsentLogStoreMetricsImplTest, RecordDroppedLogsNum) {
 
 TEST(UnsentLogStoreMetricsImplTest, RecordLastUnsentLogMetadataMetrics) {
   base::test::ScopedFeatureList feature_override;
-  feature_override.InitAndEnableFeature(
-      UnsentLogStoreMetrics::kRecordLastUnsentLogMetadataMetrics);
+  feature_override.InitAndEnableFeature(kRecordLastUnsentLogMetadataMetrics);
   UnsentLogStoreMetricsImpl impl;
   base::HistogramTester histogram_tester;
 
@@ -58,8 +57,7 @@ TEST(UnsentLogStoreMetricsImplTest, DisableRecordLastUnsentLogMetadataMetrics) {
 
 TEST(UnsentLogStoreMetricsImplTest, BothUnsentAndSentZeroSample) {
   base::test::ScopedFeatureList feature_override;
-  feature_override.InitAndEnableFeature(
-      UnsentLogStoreMetrics::kRecordLastUnsentLogMetadataMetrics);
+  feature_override.InitAndEnableFeature(kRecordLastUnsentLogMetadataMetrics);
   UnsentLogStoreMetricsImpl impl;
   base::HistogramTester histogram_tester;
 
@@ -71,8 +69,7 @@ TEST(UnsentLogStoreMetricsImplTest, BothUnsentAndSentZeroSample) {
 
 TEST(UnsentLogStoreMetricsImplTest, ZeroUnsentSample) {
   base::test::ScopedFeatureList feature_override;
-  feature_override.InitAndEnableFeature(
-      UnsentLogStoreMetrics::kRecordLastUnsentLogMetadataMetrics);
+  feature_override.InitAndEnableFeature(kRecordLastUnsentLogMetadataMetrics);
   UnsentLogStoreMetricsImpl impl;
   base::HistogramTester histogram_tester;
 
@@ -84,8 +81,7 @@ TEST(UnsentLogStoreMetricsImplTest, ZeroUnsentSample) {
 
 TEST(UnsentLogStoreMetricsImplTest, ZeroSentSample) {
   base::test::ScopedFeatureList feature_override;
-  feature_override.InitAndEnableFeature(
-      UnsentLogStoreMetrics::kRecordLastUnsentLogMetadataMetrics);
+  feature_override.InitAndEnableFeature(kRecordLastUnsentLogMetadataMetrics);
   UnsentLogStoreMetricsImpl impl;
   base::HistogramTester histogram_tester;
 

diff --git a/components/navigation_interception/intercept_navigation_throttle.cc b/components/navigation_interception/intercept_navigation_throttle.cc
@@ -12,8 +12,9 @@
 namespace navigation_interception {
 
 // Note: this feature is a no-op on non-Android platforms.
-const base::Feature InterceptNavigationThrottle::kAsyncCheck{
-    "AsyncNavigationIntercept", base::FEATURE_ENABLED_BY_DEFAULT};
+BASE_FEATURE(kAsyncCheck,
+             "AsyncNavigationIntercept",
+             base::FEATURE_ENABLED_BY_DEFAULT);
 
 InterceptNavigationThrottle::InterceptNavigationThrottle(
     content::NavigationHandle* navigation_handle,

diff --git a/components/navigation_interception/intercept_navigation_throttle.h b/components/navigation_interception/intercept_navigation_throttle.h
@@ -18,6 +18,8 @@ class NavigationHandle;
 
 namespace navigation_interception {
 
+BASE_DECLARE_FEATURE(kAsyncCheck);
+
 enum class SynchronyMode {
   // Support async interception in some cases (See ShouldCheckAsynchronously).
   kAsync,
@@ -33,8 +35,6 @@ class InterceptNavigationThrottle : public content::NavigationThrottle {
       content::NavigationHandle* /* navigation_handle */)>
       CheckCallback;
 
-  static const base::Feature kAsyncCheck;
-
   InterceptNavigationThrottle(content::NavigationHandle* navigation_handle,
                               CheckCallback should_ignore_callback,
                               SynchronyMode async_mode);

diff --git a/components/navigation_interception/intercept_navigation_throttle_unittest.cc b/components/navigation_interception/intercept_navigation_throttle_unittest.cc
@@ -68,11 +68,9 @@ class InterceptNavigationThrottleTest
   InterceptNavigationThrottleTest()
       : mock_callback_receiver_(new MockInterceptCallbackReceiver()) {
     if (GetParam()) {
-      scoped_feature_.InitAndEnableFeature(
-          InterceptNavigationThrottle::kAsyncCheck);
+      scoped_feature_.InitAndEnableFeature(kAsyncCheck);
     } else {
-      scoped_feature_.InitAndDisableFeature(
-          InterceptNavigationThrottle::kAsyncCheck);
+      scoped_feature_.InitAndDisableFeature(kAsyncCheck);
     }
   }
 

diff --git a/components/reporting/client/report_queue_provider.cc b/components/reporting/client/report_queue_provider.cc
@@ -93,8 +93,9 @@ bool ReportQueueProvider::IsEncryptedReportingPipelineEnabled() {
 }
 
 // static
-const base::Feature ReportQueueProvider::kEncryptedReportingPipeline{
-    "EncryptedReportingPipeline", base::FEATURE_ENABLED_BY_DEFAULT};
+BASE_FEATURE(kEncryptedReportingPipeline,
+             "EncryptedReportingPipeline",
+             base::FEATURE_ENABLED_BY_DEFAULT);
 
 ReportQueueProvider::ReportQueueProvider(
     StorageModuleCreateCallback storage_create_cb)

diff --git a/components/reporting/client/report_queue_provider.h b/components/reporting/client/report_queue_provider.h
@@ -23,6 +23,8 @@
 
 namespace reporting {
 
+BASE_DECLARE_FEATURE(kEncryptedReportingPipeline);
+
 // ReportQueueProvider acts a single point for instantiating
 // |reporting::ReportQueue|s. By performing initialization atomically it ensures
 // that all ReportQueues are created with the same global settings.
@@ -100,8 +102,6 @@ class ReportQueueProvider {
   using ReportQueueConfiguredCallback = base::OnceCallback<void(
       StatusOr<std::unique_ptr<ReportQueueConfiguration>>)>;
 
-  static const base::Feature kEncryptedReportingPipeline;
-
   explicit ReportQueueProvider(StorageModuleCreateCallback storage_create_cb);
   ReportQueueProvider(const ReportQueueProvider& other) = delete;
   ReportQueueProvider& operator=(const ReportQueueProvider& other) = delete;

diff --git a/components/reporting/client/report_queue_provider_unittest.cc b/components/reporting/client/report_queue_provider_unittest.cc
@@ -256,8 +256,7 @@ TEST_F(ReportQueueProviderTest, CreateMultipleSpeculativeQueues) {
 TEST_F(ReportQueueProviderTest,
        CreateReportQueueWithEncryptedReportingPipelineDisabled) {
   base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndDisableFeature(
-      ReportQueueProvider::kEncryptedReportingPipeline);
+  feature_list.InitAndDisableFeature(kEncryptedReportingPipeline);
 
   // Create configuration
   auto config_result = ReportQueueConfiguration::Create(
@@ -276,8 +275,7 @@ TEST_F(ReportQueueProviderTest,
 TEST_F(ReportQueueProviderTest,
        CreateSpeculativeReportQueueWithEncryptedReportingPipelineDisabled) {
   base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndDisableFeature(
-      ReportQueueProvider::kEncryptedReportingPipeline);
+  feature_list.InitAndDisableFeature(kEncryptedReportingPipeline);
 
   // Create configuration
   auto config_result = ReportQueueConfiguration::Create(

diff --git a/components/reporting/metrics/metric_rate_controller.cc b/components/reporting/metrics/metric_rate_controller.cc
@@ -11,8 +11,9 @@
 namespace reporting {
 
 // static
-const base::Feature MetricRateController::kEnableTelemetryTestingRates{
-    "EnableTelemetryTestingRates", base::FEATURE_DISABLED_BY_DEFAULT};
+BASE_FEATURE(kEnableTelemetryTestingRates,
+             "EnableTelemetryTestingRates",
+             base::FEATURE_DISABLED_BY_DEFAULT);
 
 MetricRateController::MetricRateController(
     base::RepeatingClosure task,

diff --git a/components/reporting/metrics/metric_rate_controller.h b/components/reporting/metrics/metric_rate_controller.h
@@ -17,12 +17,12 @@ namespace reporting {
 
 class ReportingSettings;
 
+BASE_DECLARE_FEATURE(kEnableTelemetryTestingRates);
+
 // Control reporting rate based on the reporting setting specified by the
 // setting path.
 class MetricRateController {
  public:
-  static const base::Feature kEnableTelemetryTestingRates;
-
   MetricRateController(base::RepeatingClosure task,
                        ReportingSettings* reporting_settings,
                        const std::string& rate_setting_path,

diff --git a/components/ukm/ukm_service.cc b/components/ukm/ukm_service.cc
@@ -168,8 +168,9 @@ void PurgeDataFromUnsentLogStore(metrics::UnsentLogStore* ukm_log_store,
 }  // namespace
 
 // static
-const base::Feature UkmService::kReportUserNoisedUserBirthYearAndGender = {
-    "UkmReportNoisedUserBirthYearAndGender", base::FEATURE_ENABLED_BY_DEFAULT};
+BASE_FEATURE(kReportUserNoisedUserBirthYearAndGender,
+             "UkmReportNoisedUserBirthYearAndGender",
+             base::FEATURE_ENABLED_BY_DEFAULT);
 
 bool UkmService::LogCanBeParsed(const std::string& serialized_data) {
   Report report;