Commit 9c2422bd authored by Thomas Anderson's avatar Thomas Anderson Committed by Commit Bot

Revert "Add bool constructor with type for PolicyValue"

This reverts commit 30d9990c.

Reason for revert: Suspected cause of build failure on ilder Deterministic Linux (dbg) Build 18293:
https://ci.chromium.org/p/chromium/builders/ci/Deterministic%20Linux%20%28dbg%29/18293?blamelist=1#blamelist-tab

 Checking libblink_common.so difference: (10672 deps)
   obj/third_party/blink/common/common/document_policy_features.o

Original change's description:
> Add bool constructor with type for PolicyValue
> 
> Previously, boolean PolicyValue is missing a constructor with type
> specification on PolicyValue class. It caused some inconvenience
> on code generation and testing. With the introduction of enum
> PolicyValue, this becomes more obvious because enum type's constructor
> also comes with a type parameter.
> 
> In order to simplify the codegen and testing logic, this CL adds a dummy
> constructor for boolean type PolicyValue.
> 
> Note: the constructor with type parameter aims to distinguish policy
> values that have same C++ type representation but different policy value
> type, e.g. IncDouble and DecDouble, Enum and Int.
> 
> Change-Id: I11ba793c73a770d7211e21c6a94d29f5557ac284
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347365
> Commit-Queue: Charlie Hu <chenleihu@google.com>
> Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
> Reviewed-by: Ian Clelland <iclelland@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#798714}

TBR=iclelland@chromium.org,foolip@chromium.org,chenleihu@google.com

Change-Id: I79f53710b14f22a7c81b4c757cbee03160c6addb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360796Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798853}
parent d4c213d3
...@@ -20,9 +20,6 @@ PolicyValue::PolicyValue(bool bool_value) ...@@ -20,9 +20,6 @@ PolicyValue::PolicyValue(bool bool_value)
PolicyValue::PolicyValue(double double_value) PolicyValue::PolicyValue(double double_value)
: type_(mojom::PolicyValueType::kDecDouble), double_value_(double_value) {} : type_(mojom::PolicyValueType::kDecDouble), double_value_(double_value) {}
PolicyValue::PolicyValue(bool bool_value, mojom::PolicyValueType type)
: type_(type), bool_value_(bool_value) {}
PolicyValue::PolicyValue(double double_value, mojom::PolicyValueType type) PolicyValue::PolicyValue(double double_value, mojom::PolicyValueType type)
: type_(type), double_value_(double_value) {} : type_(type), double_value_(double_value) {}
......
...@@ -25,7 +25,6 @@ class BLINK_COMMON_EXPORT PolicyValue { ...@@ -25,7 +25,6 @@ class BLINK_COMMON_EXPORT PolicyValue {
explicit PolicyValue(bool bool_value); explicit PolicyValue(bool bool_value);
explicit PolicyValue(double double_value); explicit PolicyValue(double double_value);
PolicyValue(bool bool_value, mojom::PolicyValueType type);
PolicyValue(double double_value, mojom::PolicyValueType type); PolicyValue(double double_value, mojom::PolicyValueType type);
// A 'max' PolicyValue is the most permissive value for the policy. // A 'max' PolicyValue is the most permissive value for the policy.
......
...@@ -17,8 +17,8 @@ class MakeDocumentPolicyFeaturesTest(unittest.TestCase): ...@@ -17,8 +17,8 @@ class MakeDocumentPolicyFeaturesTest(unittest.TestCase):
parse_default_value("min", "DecDouble"), parse_default_value("min", "DecDouble"),
"PolicyValue::CreateMinPolicyValue(mojom::PolicyValueType::kDecDouble)" "PolicyValue::CreateMinPolicyValue(mojom::PolicyValueType::kDecDouble)"
) )
self.assertEqual(parse_default_value("false", "Bool"), self.assertEqual(
"PolicyValue(false, mojom::PolicyValueType::kBool)") parse_default_value("false", "Bool"), "PolicyValue(false)")
self.assertEqual( self.assertEqual(
parse_default_value("0.5", "DecDouble"), parse_default_value("0.5", "DecDouble"),
"PolicyValue(0.5, mojom::PolicyValueType::kDecDouble)") "PolicyValue(0.5, mojom::PolicyValueType::kDecDouble)")
......
...@@ -5,11 +5,16 @@ ...@@ -5,11 +5,16 @@
def parse_default_value(default_value, def parse_default_value(default_value,
value_type, value_type,
recognized_types=('Bool', 'DecDouble')): recognized_types=('Bool', 'DecDouble'),
single_ctor_param_types=('Bool')):
""" Parses default_value string to actual usable C++ expression. """ Parses default_value string to actual usable C++ expression.
@param default_value_str: default_value field specified in document_policy_features.json5 @param default_value_str: default_value field specified in document_policy_features.json5
@param value_type: value_type field specified in document_policy_features.json5 @param value_type: value_type field specified in document_policy_features.json5
@param recognized_types: types that are valid for value_type @param recognized_types: types that are valid for value_type
@param single_ctor_param_types: types with single param PolicyValue constructor
- Bool corresponds to PolicyValue(bool) which only has one constructor param
- DecDouble and IncDouble both take an extra constructor param for value_type
in constructor PolicyValue(double, mojom::PolicyValueType)
@returns: a C++ expression that has type mojom::PolicyValue @returns: a C++ expression that has type mojom::PolicyValue
""" """
if (value_type not in recognized_types): if (value_type not in recognized_types):
...@@ -25,4 +30,8 @@ def parse_default_value(default_value, ...@@ -25,4 +30,8 @@ def parse_default_value(default_value,
return "PolicyValue::CreateMinPolicyValue({})".format( return "PolicyValue::CreateMinPolicyValue({})".format(
policy_value_type) policy_value_type)
return "PolicyValue({}, {})".format(default_value, policy_value_type) # types that have only one corresponding PolicyValueType
if value_type in single_ctor_param_types:
return "PolicyValue({})".format(default_value)
else:
return "PolicyValue({}, {})".format(default_value, policy_value_type)
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