Commit bed3fd1a authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Fix nested Scoped*ForTest when the inner is implied by the outer

Previously, if feature A implies B, suppose both are disabled by
default, in the following situation:
  ScopedAForTest a;
  ScopedBForTest b;
feature B will be left enabled after the scope.

Now let Scoped*ForTest use the data field of the feature instead of
calling *Enabled() and Set*Enabled() (which are asymmetric when the
feature is implied).

RuntimeEnabledFeature backup and restore had the similar issue. Also
let them use data field instead of calling the methods.

Bug: 1061443
Change-Id: I28bfee510bf3e4491c11887d7f8c992d433b9702
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2103317Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750430}
parent 83b9ee52
...@@ -185,7 +185,8 @@ class RuntimeFeatureTestHelpersWriter(BaseRuntimeFeatureWriter): ...@@ -185,7 +185,8 @@ class RuntimeFeatureTestHelpersWriter(BaseRuntimeFeatureWriter):
def _template_inputs(self): def _template_inputs(self):
return { return {
'features': self._features, # The test helpers don't support custom features.
'features': self._standard_features,
'input_files': self._input_files, 'input_files': self._input_files,
'header_guard': self._header_guard, 'header_guard': self._header_guard,
} }
......
...@@ -14,18 +14,14 @@ namespace blink { ...@@ -14,18 +14,14 @@ namespace blink {
RuntimeEnabledFeatures::Backup::Backup() RuntimeEnabledFeatures::Backup::Backup()
: {% for feature in standard_features -%} : {% for feature in standard_features -%}
{% if feature.in_origin_trial %} {{feature.data_member_name}}(RuntimeEnabledFeatures::{{feature.data_member_name}})
{{feature.name.to_class_data_member()}}(RuntimeEnabledFeatures::{{feature.name}}EnabledByRuntimeFlag())
{% else %}
{{feature.name.to_class_data_member()}}(RuntimeEnabledFeatures::{{feature.name}}Enabled())
{% endif %}
{%- if not loop.last %}, {%- if not loop.last %},
{%+ endif -%} {%+ endif -%}
{% endfor %} {} {% endfor %} {}
void RuntimeEnabledFeatures::Backup::Restore() { void RuntimeEnabledFeatures::Backup::Restore() {
{% for feature in standard_features %} {% for feature in standard_features %}
RuntimeEnabledFeatures::Set{{feature.name}}Enabled({{feature.name.to_class_data_member()}}); RuntimeEnabledFeatures::{{feature.data_member_name}} = {{feature.data_member_name}};
{% endfor %} {% endfor %}
} }
......
...@@ -49,7 +49,7 @@ class PLATFORM_EXPORT RuntimeEnabledFeatures { ...@@ -49,7 +49,7 @@ class PLATFORM_EXPORT RuntimeEnabledFeatures {
private: private:
{% for feature in standard_features %} {% for feature in standard_features %}
bool {{feature.name.to_class_data_member()}}; bool {{feature.data_member_name}};
{% endfor %} {% endfor %}
}; };
...@@ -95,6 +95,8 @@ class PLATFORM_EXPORT RuntimeEnabledFeatures { ...@@ -95,6 +95,8 @@ class PLATFORM_EXPORT RuntimeEnabledFeatures {
{% endfor %} {% endfor %}
private: private:
friend class RuntimeEnabledFeaturesTestHelpers;
{% for feature in standard_features %} {% for feature in standard_features %}
static bool {{feature.data_member_name}}; static bool {{feature.data_member_name}};
{% endfor %} {% endfor %}
......
...@@ -11,33 +11,32 @@ ...@@ -11,33 +11,32 @@
namespace blink { namespace blink {
template <bool (&getter)(), void (&setter)(bool)> // Don't use this class directly. Use Scoped*ForTest instead.
class ScopedRuntimeEnabledFeatureForTest { class RuntimeEnabledFeaturesTestHelpers {
public: public:
ScopedRuntimeEnabledFeatureForTest(bool enabled) template <bool& data_member>
: enabled_(enabled), original_(getter()) { class ScopedRuntimeEnabledFeature {
setter(enabled); public:
} ScopedRuntimeEnabledFeature(bool enabled)
: enabled_(enabled), original_(data_member) { data_member = enabled; }
~ScopedRuntimeEnabledFeatureForTest() { ~ScopedRuntimeEnabledFeature() {
CHECK_EQ(enabled_, getter()); CHECK_EQ(enabled_, data_member);
setter(original_); data_member = original_;
} }
private:
private: bool enabled_;
bool enabled_; bool original_;
bool original_; };
{% for feature in features %}
using Scoped{{feature.name}} = ScopedRuntimeEnabledFeature<
RuntimeEnabledFeatures::{{feature.data_member_name}}>;
{% endfor %}
}; };
{% for feature in features %} {% for feature in features %}
typedef ScopedRuntimeEnabledFeatureForTest< using Scoped{{feature.name}}ForTest =
{% if feature.in_origin_trial %} RuntimeEnabledFeaturesTestHelpers::Scoped{{feature.name}};
RuntimeEnabledFeatures::{{feature.name}}EnabledByRuntimeFlag,
{% else %}
RuntimeEnabledFeatures::{{feature.name}}Enabled,
{% endif %}
RuntimeEnabledFeatures::Set{{feature.name}}Enabled>
Scoped{{feature.name}}ForTest;
{% endfor %} {% endfor %}
} // namespace blink } // namespace blink
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment