Make ScopedMutuallyExclusiveFeatureList work with non-span ranges

net::test::ScopedMutuallyExclusiveFeatureList's constructor required its second parameter to be a base::span. This was unnecessarily restrictive. Permit any forward range as the second parameter, and adjust the requirements to match. Also add documentation for the class. Modify existing users to not create a span. Change-Id: I00b9c759a6208fb0af74536582d268e824ddecc3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796335 Commit-Queue: Adam Rice <[email protected]> Reviewed-by: Andrew Williams <[email protected]> Reviewed-by: David Trainor <[email protected]> Cr-Commit-Position: refs/heads/main@{#1347156}
azx1573 · Aug 27, 2024 · c30c091 · c30c091
1 parent 5757e64
commit c30c091
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 48 deletions.
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
@@ -19,7 +19,6 @@
 
 #include "base/base64.h"
 #include "base/command_line.h"
-#include "base/containers/span.h"
 #include "base/feature_list.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
@@ -2332,7 +2331,7 @@ enum class SplitCacheTestCase {
   kEnabledTriplePlusNavInitiator
 };
 
-const struct TestCaseToFeatureMapping {
+const struct {
   const SplitCacheTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -2342,8 +2341,6 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByMainFrameNavigationInitiator},
     {SplitCacheTestCase::kEnabledTriplePlusNavInitiator,
      net::features::kSplitCacheByNavigationInitiator}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 std::string GetSplitCacheTestName(SplitCacheTestCase test_case) {
   switch (test_case) {
@@ -2364,7 +2361,7 @@ class DownloadTestSplitCacheEnabled
  public:
   DownloadTestSplitCacheEnabled()
       : split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {}
+                                             kTestCaseToFeatureMapping) {}
 
  private:
   net::test::ScopedMutuallyExclusiveFeatureList
@@ -2390,7 +2387,7 @@ class PdfDownloadTestSplitCacheEnabled
  public:
   PdfDownloadTestSplitCacheEnabled()
       : split_cache_experiment_feature_list_(GetSplitCacheTestCase(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     if (UseOopif()) {
       oopif_feature_list_.InitAndEnableFeature(chrome_pdf::features::kPdfOopif);
     } else {

diff --git a/chrome/browser/net/profile_network_context_service_browsertest.cc b/chrome/browser/net/profile_network_context_service_browsertest.cc
@@ -13,7 +13,6 @@
 
 #include "base/check_op.h"
 #include "base/containers/contains.h"
-#include "base/containers/span.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -449,7 +448,7 @@ enum class HttpCache2024ExperimentTestCase {
   kControlGroup
 };
 
-const struct TestCaseToFeatureMapping {
+const struct {
   const HttpCache2024ExperimentTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -462,16 +461,14 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByNavigationInitiator},
     {HttpCache2024ExperimentTestCase::kControlGroup,
      net::features::kHttpCacheKeyingExperimentControlGroup2024}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 class ProfileNetworkContextServiceCacheKeySchemeExperimentBrowserTest
     : public ProfileNetworkContextServiceBrowsertest,
       public testing::WithParamInterface<HttpCache2024ExperimentTestCase> {
  public:
   ProfileNetworkContextServiceCacheKeySchemeExperimentBrowserTest()
       : split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     // Override any configured experiments for the
     // SplitCacheByNetworkIsolationKey feature.
     split_cache_always_enabled_feature_list_.InitAndEnableFeatureWithParameters(

diff --git a/...e/browser/preloading/prefetch/no_state_prefetch/prerender_nostate_prefetch_browsertest.cc b/...e/browser/preloading/prefetch/no_state_prefetch/prerender_nostate_prefetch_browsertest.cc
@@ -6,7 +6,6 @@
 
 #include "base/command_line.h"
 #include "base/containers/contains.h"
-#include "base/containers/span.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/memory/raw_ptr.h"
@@ -117,7 +116,7 @@ enum class SplitCacheTestCase {
   kEnabledTriplePlusNavInitiator
 };
 
-const struct TestCaseToFeatureMapping {
+const struct {
   const SplitCacheTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -127,8 +126,6 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByMainFrameNavigationInitiator},
     {SplitCacheTestCase::kEnabledTriplePlusNavInitiator,
      net::features::kSplitCacheByNavigationInitiator}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 }  // namespace
 
@@ -501,7 +498,7 @@ class NoStatePrefetchBrowserTestHttpCache
  protected:
   NoStatePrefetchBrowserTestHttpCache()
       : split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     if (IsSplitCacheEnabled()) {
       split_cache_enabled_feature_list_.InitAndEnableFeature(
           net::features::kSplitCacheByNetworkIsolationKey);

diff --git a/chrome/browser/ui/browser_navigator_browsertest.cc b/chrome/browser/ui/browser_navigator_browsertest.cc
@@ -6,7 +6,6 @@
 
 #include <memory>
 
-#include "base/containers/span.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -1814,7 +1813,7 @@ enum class SplitCacheTestCase {
   kEnabledTriplePlusMainFrameNavInitiator,
   kEnabledTriplePlusNavInitiator
 };
-const struct TestCaseToFeatureMapping {
+const struct {
   const SplitCacheTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -1824,16 +1823,14 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByMainFrameNavigationInitiator},
     {SplitCacheTestCase::kEnabledTriplePlusNavInitiator,
      net::features::kSplitCacheByNavigationInitiator}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 class BrowserNavigatorSplitHttpCacheTest
     : public BrowserNavigatorTest,
       public testing::WithParamInterface<SplitCacheTestCase> {
  protected:
   BrowserNavigatorSplitHttpCacheTest()
       : split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     split_cache_always_enabled_feature_list_.InitAndEnableFeature(
         net::features::kSplitCacheByNetworkIsolationKey);
   }

diff --git a/content/browser/loader/prefetch_browsertest.cc b/content/browser/loader/prefetch_browsertest.cc
@@ -5,7 +5,6 @@
 #include <string>
 #include <vector>
 
-#include "base/containers/span.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -55,7 +54,7 @@ enum class SplitCacheTestCase {
   // initiator features are, and we won't test these combinations.
 };
 
-const struct TestCaseToFeatureMapping {
+const struct {
   const SplitCacheTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -67,8 +66,6 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByMainFrameNavigationInitiator},
     {SplitCacheTestCase::kEnabledTriplePlusNavInitiator,
      net::features::kSplitCacheByNavigationInitiator}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 }  // namespace
 
@@ -80,7 +77,7 @@ class PrefetchBrowserTest
       : cross_origin_server_(std::make_unique<net::EmbeddedTestServer>()),
         split_cache_test_case_(GetParam()),
         split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     if (IsSplitCacheEnabled()) {
       split_cache_enabled_feature_list_.InitAndEnableFeature(
           net::features::kSplitCacheByNetworkIsolationKey);

diff --git a/content/browser/network/split_cache_browsertest.cc b/content/browser/network/split_cache_browsertest.cc
@@ -4,7 +4,6 @@
 
 #include <atomic>
 
-#include "base/containers/span.h"
 #include "base/path_service.h"
 #include "base/strings/pattern.h"
 #include "base/strings/stringprintf.h"
@@ -435,7 +434,7 @@ enum class SplitCacheTestCase {
   kEnabledTriplePlusNavInitiator
 };
 
-const struct TestCaseToFeatureMapping {
+const struct {
   const SplitCacheTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -445,16 +444,14 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByMainFrameNavigationInitiator},
     {SplitCacheTestCase::kEnabledTriplePlusNavInitiator,
      net::features::kSplitCacheByNavigationInitiator}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 class SplitCacheContentBrowserTestEnabled
     : public SplitCacheContentBrowserTest,
       public testing::WithParamInterface<SplitCacheTestCase> {
  public:
   SplitCacheContentBrowserTestEnabled()
       : split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     split_cache_always_enabled_feature_list_.InitAndEnableFeature(
         net::features::kSplitCacheByNetworkIsolationKey);
   }

diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
@@ -13,7 +13,6 @@
 #include <utility>
 #include <vector>
 
-#include "base/containers/span.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/format_macros.h"
 #include "base/functional/bind.h"
@@ -1277,7 +1276,7 @@ enum class SplitCacheTestCase {
   kEnabledTriplePlusNavInitiator
 };
 
-const struct TestCaseToFeatureMapping {
+const struct {
   const SplitCacheTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -1287,16 +1286,14 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByMainFrameNavigationInitiator},
     {SplitCacheTestCase::kEnabledTriplePlusNavInitiator,
      net::features::kSplitCacheByNavigationInitiator}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 class HttpCacheTestSplitCacheFeature
     : public HttpCacheTest,
       public ::testing::WithParamInterface<SplitCacheTestCase> {
  public:
   HttpCacheTestSplitCacheFeature()
       : split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     if (IsSplitCacheEnabled()) {
       split_cache_enabled_feature_list_.InitAndEnableFeature(
           net::features::kSplitCacheByNetworkIsolationKey);

diff --git a/net/test/scoped_mutually_exclusive_feature_list.h b/net/test/scoped_mutually_exclusive_feature_list.h
@@ -6,21 +6,86 @@
 #define NET_TEST_SCOPED_MUTUALLY_EXCLUSIVE_FEATURE_LIST_H_
 
 #include <concepts>
+#include <ranges>
+#include <vector>
 
-#include "base/containers/span.h"
 #include "base/test/scoped_feature_list.h"
 
 namespace net::test {
 
+// A helper for parameterized tests where one of a set of features should be
+// enabled depending on the value of an enum parameter. Features in the set that
+// are not enabled will be explicitly disabled so that the test functions the
+// same regardless of the features' default values.
+//
+// Example usage:
+//
+//
+// using ::testing::Values;
+//
+// // First define an enum:
+// enum class FooConfiguration {
+//   kConfig1,
+//   kConfig2,
+// };
+//
+// // Define the mapping from enum to feature. The struct must have exactly two
+// // members, named `test_case` and `feature`, in that order.
+// const struct {
+//   const FooConfiguration test_case;
+//   base::test::FeatureRef feature;
+// } kFooConfigurationToFeatureMapping[] = {
+//   { FooConfiguration::kConfig1, features::FooFeature1 },
+//   { FooConfiguration::kConfig2, features::FooFeature2 },
+// };
+//
+// // Define the text fixture.
+// class FooConfigurationTest :
+//   public testing::TestWithParam<FooConfiguration> {
+//  public:
+//   FooConfigurationTest() :
+//     foo_feature_list_(GetParam(), kFooConfigurationToFeatureMapping) {}
+//
+//  private:
+//   ScopedMutuallyExclusiveFeatureList foo_feature_list_;
+// };
+//
+// // Instantiate the test suite.
+// INSTANTIATE_TEST_SUITE_P(
+//   All,
+//   FooConfigurationTest,
+//   Values(FooConfiguration::kConfig1, FooConfiguration::kConfig2),
+//   [](const testing::TestParamInfo<FooConfiguration>& info) {
+//     return info.param == kConfig1 ? "Config1" : "Config2";
+//   });
+//
+// // Write some tests.
+//
+// TEST_P(FooConfigurationTest, Something) {
+//   Foo foo;
+//   foo.DoSomething();
+//   EXPECT_TRUE(foo.something_was_done());
+// }
+//
+//
+// This will result in two tests being run,
+// All/FooConfigurationTest.Something/Config1 and
+// All/FooConfigurationTest.Something/Config2. The first will run with
+// FooFeature1 enabled and FooFeature2 disabled, and the second will run with
+// the opposite configuration.
+
 class ScopedMutuallyExclusiveFeatureList {
  public:
-  template <typename Enum, typename Struct>
-    requires requires(Struct s) {
-      { s.test_case } -> std::convertible_to<Enum>;
-      { s.feature } -> std::same_as<base::test::FeatureRef&>;
+  template <typename Enum, typename Range>
+    requires std::ranges::forward_range<Range> && requires(const Range& r) {
+      {
+        std::ranges::begin(r)->test_case
+      } -> std::equality_comparable_with<Enum>;
+      {
+        std::ranges::begin(r)->feature
+      } -> std::same_as<const base::test::FeatureRef&>;
     }
-  ScopedMutuallyExclusiveFeatureList(Enum param,
-                                     base::span<const Struct> mapping) {
+  ScopedMutuallyExclusiveFeatureList(Enum param, const Range& mapping) {
     std::vector<base::test::FeatureRef> enabled_features;
     std::vector<base::test::FeatureRef> disabled_features;
 

diff --git a/services/network/network_context_unittest.cc b/services/network/network_context_unittest.cc
@@ -22,7 +22,6 @@
 #include "base/barrier_closure.h"
 #include "base/command_line.h"
 #include "base/containers/contains.h"
-#include "base/containers/span.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -7605,7 +7604,7 @@ enum class SplitCacheTestCase {
   kEnabledTriplePlusNavInitiator
 };
 
-const struct TestCaseToFeatureMapping {
+const struct {
   const SplitCacheTestCase test_case;
   base::test::FeatureRef feature;
 } kTestCaseToFeatureMapping[] = {
@@ -7615,8 +7614,6 @@ const struct TestCaseToFeatureMapping {
      net::features::kSplitCacheByMainFrameNavigationInitiator},
     {SplitCacheTestCase::kEnabledTriplePlusNavInitiator,
      net::features::kSplitCacheByNavigationInitiator}};
-const base::span<const TestCaseToFeatureMapping> kTestCaseToFeatureMappingSpan(
-    kTestCaseToFeatureMapping);
 
 class NetworkContextSplitCacheTest
     : public NetworkContextTest,
@@ -7625,7 +7622,7 @@ class NetworkContextSplitCacheTest
   NetworkContextSplitCacheTest()
       : split_cache_test_case_(GetParam()),
         split_cache_experiment_feature_list_(GetParam(),
-                                             kTestCaseToFeatureMappingSpan) {
+                                             kTestCaseToFeatureMapping) {
     split_cache_always_enabled_feature_list_.InitAndEnableFeature(
         net::features::kSplitCacheByNetworkIsolationKey);