-
Notifications
You must be signed in to change notification settings - Fork 921
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add support for default feature state overrides using static map (#10878
) * Add support for default feature state override. * Convert runtime feature overrides to static overrides. * Support overrides set via a single macro instantiation. * Remove unused include.
- Loading branch information
Showing
35 changed files
with
611 additions
and
93 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| /* Copyright (c) 2021 The Brave Authors. All rights reserved. | ||
| * This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this file, | ||
| * You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| #include "base/feature_override.h" | ||
| #include "base/feature_list.h" | ||
| #include "base/logging.h" | ||
| #include "base/test/mock_callback.h" | ||
| #include "testing/gmock/include/gmock/gmock.h" | ||
| #include "testing/gtest/include/gtest/gtest.h" | ||
|
|
||
| using testing::_; | ||
|
|
||
| namespace base { | ||
| namespace { | ||
|
|
||
| const Feature kTestControlEnabledFeature{"TestControlEnabledFeature", | ||
| FEATURE_ENABLED_BY_DEFAULT}; | ||
| const Feature kTestControlDisabledFeature{"TestControlDisabledFeature", | ||
| FEATURE_DISABLED_BY_DEFAULT}; | ||
|
|
||
| const Feature kTestEnabledButOverridenFeature{"TestEnabledButOverridenFeature", | ||
| FEATURE_DISABLED_BY_DEFAULT}; | ||
| const Feature kTestDisabledButOverridenFeature{ | ||
| "TestDisabledButOverridenFeature", FEATURE_ENABLED_BY_DEFAULT}; | ||
|
|
||
| constexpr Feature kTestConstexprEnabledButOverridenFeature{ | ||
| "TestConstexprEnabledButOverridenFeature", FEATURE_DISABLED_BY_DEFAULT}; | ||
| constexpr Feature kTestConstexprDisabledButOverridenFeature{ | ||
| "TestConstexprDisabledButOverridenFeature", FEATURE_ENABLED_BY_DEFAULT}; | ||
|
|
||
| OVERRIDE_FEATURE_DEFAULT_STATES({{ | ||
| {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT}, | ||
| {kTestDisabledButOverridenFeature, FEATURE_ENABLED_BY_DEFAULT}, | ||
| {kTestConstexprEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT}, | ||
| {kTestConstexprDisabledButOverridenFeature, FEATURE_ENABLED_BY_DEFAULT}, | ||
| }}); | ||
|
|
||
| } // namespace | ||
|
|
||
| TEST(FeatureOverrideTest, OverridesTest) { | ||
| struct TestCase { | ||
| const Feature& feature; | ||
| const bool is_enabled; | ||
| }; | ||
| const TestCase kTestCases[] = { | ||
| // Untouched features. | ||
| {kTestControlEnabledFeature, true}, | ||
| {kTestControlDisabledFeature, false}, | ||
|
|
||
| // Overridden features. | ||
| {kTestEnabledButOverridenFeature, false}, | ||
| {kTestDisabledButOverridenFeature, true}, | ||
|
|
||
| // Overridden constexpr features. | ||
| {kTestConstexprEnabledButOverridenFeature, false}, | ||
| {kTestConstexprDisabledButOverridenFeature, true}, | ||
| }; | ||
| for (const auto& test_case : kTestCases) { | ||
| EXPECT_EQ(test_case.is_enabled, FeatureList::IsEnabled(test_case.feature)) | ||
| << test_case.feature.name; | ||
| } | ||
| } | ||
|
|
||
| #if DCHECK_IS_ON() | ||
| TEST(FeatureOverrideTest, FeatureDuplicateDChecks) { | ||
| base::MockCallback<logging::LogAssertHandlerFunction> mock_log_handler; | ||
| logging::ScopedLogAssertHandler scoped_log_handler(mock_log_handler.Get()); | ||
| EXPECT_CALL( | ||
| mock_log_handler, | ||
| Run(_, _, | ||
| testing::HasSubstr("TestEnabledButOverridenFeature is duplicated"), | ||
| _)); | ||
| EXPECT_CALL( | ||
| mock_log_handler, | ||
| Run(_, _, | ||
| testing::HasSubstr( | ||
| "TestEnabledButOverridenFeature has already been overridden"), | ||
| _)) | ||
| .Times(2); | ||
|
|
||
| internal::FeatureDefaultStateOverrider test_overrider{{ | ||
| {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT}, | ||
| {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT}, | ||
| }}; | ||
| } | ||
| #endif // DCHECK_IS_ON() | ||
|
|
||
| } // namespace base |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /* Copyright (c) 2021 The Brave Authors. All rights reserved. | ||
| * This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this file, | ||
| * You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| #include "base/feature_list.h" | ||
| #include "base/containers/contains.h" | ||
| #include "base/containers/flat_map.h" | ||
| #include "base/containers/flat_set.h" | ||
| #include "base/feature_override.h" | ||
| #include "base/no_destructor.h" | ||
|
|
||
| namespace base { | ||
| namespace internal { | ||
| namespace { | ||
|
|
||
| using DefaultStateOverrides = | ||
| base::flat_map<std::reference_wrapper<const Feature>, FeatureState>; | ||
|
|
||
| DefaultStateOverrides& GetDefaultStateOverrides() { | ||
| static base::NoDestructor<DefaultStateOverrides> default_state_overrides; | ||
| return *default_state_overrides; | ||
| } | ||
|
|
||
| inline FeatureState GetDefaultOrOverriddenFeatureState(const Feature& feature) { | ||
| const auto& default_state_overrides = GetDefaultStateOverrides(); | ||
| const auto default_state_override_it = default_state_overrides.find(feature); | ||
| return default_state_override_it != default_state_overrides.end() | ||
| ? default_state_override_it->second | ||
| : feature.default_state; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| FeatureDefaultStateOverrider::FeatureDefaultStateOverrider( | ||
| std::initializer_list<FeatureOverrideInfo> overrides) { | ||
| auto& default_state_overrides = GetDefaultStateOverrides(); | ||
| #if DCHECK_IS_ON() | ||
| { | ||
| base::flat_set<std::reference_wrapper<const Feature>> new_overrides; | ||
| new_overrides.reserve(overrides.size()); | ||
| for (const auto& override : overrides) { | ||
| DCHECK(new_overrides.insert(override.first).second) | ||
| << "Feature " << override.first.get().name | ||
| << " is duplicated in the current override macros"; | ||
| DCHECK(!base::Contains(default_state_overrides, override.first)) | ||
| << "Feature " << override.first.get().name | ||
| << " has already been overridden"; | ||
| } | ||
| } | ||
| #endif | ||
| default_state_overrides.insert(overrides.begin(), overrides.end()); | ||
| } | ||
|
|
||
| } // namespace internal | ||
|
|
||
| // Custom comparator to use std::reference_wrapper as a key in a map/set. | ||
| static inline bool operator<(const std::reference_wrapper<const Feature>& lhs, | ||
| const std::reference_wrapper<const Feature>& rhs) { | ||
| // Compare internal pointers directly, because there must only ever be one | ||
| // struct instance for a given feature name. | ||
| return &lhs.get() < &rhs.get(); | ||
| } | ||
|
|
||
| } // namespace base | ||
|
|
||
| // This replaces |default_state| compare blocks with a modified one that | ||
| // includes the state override check. | ||
| #define default_state \ | ||
| name&& internal::GetDefaultOrOverriddenFeatureState(feature) | ||
|
|
||
| #include "../../../base/feature_list.cc" | ||
|
|
||
| #undef default_state |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| /* Copyright (c) 2021 The Brave Authors. All rights reserved. | ||
| * This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this file, | ||
| * You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| #ifndef BRAVE_CHROMIUM_SRC_BASE_FEATURE_OVERRIDE_H_ | ||
| #define BRAVE_CHROMIUM_SRC_BASE_FEATURE_OVERRIDE_H_ | ||
|
|
||
| #include <functional> | ||
| #include <initializer_list> | ||
| #include <utility> | ||
|
|
||
| #include "base/feature_list.h" | ||
|
|
||
| // Helpers to override base::Feature::default_state without patches. | ||
| // | ||
| // Usage: | ||
| // 1. Create chromium_src/.../features.cc override for a file that contains | ||
| // features to override. | ||
| // 2. #include "base/feature_override.h" | ||
| // 3. Use OVERRIDE_FEATURE_DEFAULT_STATES macro: | ||
| // | ||
| // OVERRIDE_FEATURE_DEFAULT_STATES({{ | ||
| // {kUpstreamFeature, base::FEATURE_ENABLED_BY_DEFAULT}, | ||
| // #if defined(OS_ANDROID) | ||
| // {kAnotherUpstreamFeature, base::FEATURE_DISABLED_BY_DEFAULT}, | ||
| // #endif | ||
| // }}); | ||
|
|
||
| namespace base { | ||
| namespace internal { | ||
|
|
||
| // Perform base::Feature duplicates check and fills overriden states into a | ||
| // map that is used at runtime to get an override if available. | ||
| class BASE_EXPORT FeatureDefaultStateOverrider { | ||
| public: | ||
| using FeatureOverrideInfo = | ||
| std::pair<std::reference_wrapper<const Feature>, FeatureState>; | ||
|
|
||
| FeatureDefaultStateOverrider( | ||
| std::initializer_list<FeatureOverrideInfo> overrides); | ||
| }; | ||
|
|
||
| } // namespace internal | ||
| } // namespace base | ||
|
|
||
| // Feature override uses global constructors, we disable `global-constructors` | ||
| // warning inside this macro to instantiate the overrider without warnings. | ||
| // clang-format off | ||
| #define OVERRIDE_FEATURE_DEFAULT_STATES(...) \ | ||
| _Pragma("clang diagnostic push") \ | ||
| _Pragma("clang diagnostic ignored \"-Wglobal-constructors\"") \ | ||
| static const ::base::internal::FeatureDefaultStateOverrider \ | ||
| g_feature_default_state_overrider __VA_ARGS__; \ | ||
| _Pragma("clang diagnostic pop") \ | ||
| static_assert(true, "") /* for a semicolon requirement */ | ||
| // clang-format on | ||
|
|
||
| #endif // BRAVE_CHROMIUM_SRC_BASE_FEATURE_OVERRIDE_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 18 additions & 0 deletions
18
chromium_src/chrome/browser/apps/app_discovery_service/app_discovery_features.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| /* Copyright (c) 2021 The Brave Authors. All rights reserved. | ||
| * This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this file, | ||
| * You can obtain one at https://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| #include "../../../../../../chrome/browser/apps/app_discovery_service/app_discovery_features.cc" | ||
|
|
||
| #include "base/feature_override.h" | ||
|
|
||
| namespace apps { | ||
|
|
||
| OVERRIDE_FEATURE_DEFAULT_STATES({{ | ||
| #if !defined(OS_ANDROID) | ||
| {kAppDiscoveryRemoteUrlSearch, base::FEATURE_DISABLED_BY_DEFAULT}, | ||
| #endif | ||
| }}); | ||
|
|
||
| } // namespace apps |
Oops, something went wrong.