Commit ce5ba6cd authored by Charlie Hu's avatar Charlie Hu Committed by Commit Bot

Use array to represent internal feature state of document policy

Previously, document policy class is designed to hold a field for each
feature's policy value. Those fields are supposed to be generated from
document_policy_features.json5. Such design minimizes both speed and
memory overhead but adds complexity to the system by introducing code
generation.

This CL uses array to represent internal feature state, which still
minimizes speed overhead as indexing into array is the same as indexing
into memory, and adds very little memory overhead(Each PolicyValue
instance contains extra information).

Change-Id: I4bc5e9b4383c628018e2605ad896e31b17c9c0e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003436
Commit-Queue: Charlie Hu <chenleihu@google.com>
Reviewed-by: default avatarIan Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732682}
parent 84569e13
...@@ -226,17 +226,8 @@ DocumentPolicy::FeatureState DocumentPolicy::MergeFeatureState( ...@@ -226,17 +226,8 @@ DocumentPolicy::FeatureState DocumentPolicy::MergeFeatureState(
bool DocumentPolicy::IsFeatureEnabled( bool DocumentPolicy::IsFeatureEnabled(
mojom::FeaturePolicyFeature feature) const { mojom::FeaturePolicyFeature feature) const {
mojom::PolicyValueType feature_type = GetFeatureDefaults().at(feature).Type(); mojom::PolicyValueType feature_type = GetFeatureDefaults().at(feature).Type();
// TODO(iclelland): Generate this switch block return IsFeatureEnabled(feature,
switch (feature) { PolicyValue::CreateMaxPolicyValue(feature_type));
case mojom::FeaturePolicyFeature::kFontDisplay:
return font_display_;
case mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages:
return PolicyValue(unoptimized_lossless_images_) >=
PolicyValue::CreateMaxPolicyValue(feature_type);
default:
NOTREACHED();
return true;
}
} }
bool DocumentPolicy::IsFeatureEnabled( bool DocumentPolicy::IsFeatureEnabled(
...@@ -247,16 +238,7 @@ bool DocumentPolicy::IsFeatureEnabled( ...@@ -247,16 +238,7 @@ bool DocumentPolicy::IsFeatureEnabled(
PolicyValue DocumentPolicy::GetFeatureValue( PolicyValue DocumentPolicy::GetFeatureValue(
mojom::FeaturePolicyFeature feature) const { mojom::FeaturePolicyFeature feature) const {
// TODO(iclelland): Generate this switch block return internal_feature_state_[static_cast<size_t>(feature)];
switch (feature) {
case mojom::FeaturePolicyFeature::kFontDisplay:
return PolicyValue(font_display_);
case mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages:
return PolicyValue(unoptimized_lossless_images_);
default:
NOTREACHED();
return PolicyValue(false);
}
} }
bool DocumentPolicy::IsFeatureSupported( bool DocumentPolicy::IsFeatureSupported(
...@@ -273,30 +255,11 @@ bool DocumentPolicy::IsFeatureSupported( ...@@ -273,30 +255,11 @@ bool DocumentPolicy::IsFeatureSupported(
void DocumentPolicy::UpdateFeatureState(const FeatureState& feature_state) { void DocumentPolicy::UpdateFeatureState(const FeatureState& feature_state) {
for (const auto& feature_and_value : feature_state) { for (const auto& feature_and_value : feature_state) {
// TODO(iclelland): Generate this switch block internal_feature_state_[static_cast<size_t>(feature_and_value.first)] =
switch (feature_and_value.first) { feature_and_value.second;
case mojom::FeaturePolicyFeature::kFontDisplay:
font_display_ = feature_and_value.second.BoolValue();
break;
case mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages:
unoptimized_lossless_images_ = feature_and_value.second.DoubleValue();
break;
default:
NOTREACHED();
}
} }
} }
DocumentPolicy::FeatureState DocumentPolicy::GetFeatureState() const {
FeatureState feature_state;
// TODO(iclelland): Generate this block
feature_state[mojom::FeaturePolicyFeature::kFontDisplay] =
PolicyValue(font_display_);
feature_state[mojom::FeaturePolicyFeature::kUnoptimizedLosslessImages] =
PolicyValue(unoptimized_lossless_images_);
return feature_state;
}
DocumentPolicy::DocumentPolicy(const FeatureState& defaults) { DocumentPolicy::DocumentPolicy(const FeatureState& defaults) {
UpdateFeatureState(defaults); UpdateFeatureState(defaults);
} }
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "third_party/blink/public/common/common_export.h" #include "third_party/blink/public/common/common_export.h"
#include "third_party/blink/public/common/feature_policy/policy_value.h" #include "third_party/blink/public/common/feature_policy/policy_value.h"
#include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-forward.h" #include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom.h"
#include "third_party/blink/public/mojom/feature_policy/policy_value.mojom.h" #include "third_party/blink/public/mojom/feature_policy/policy_value.mojom.h"
namespace blink { namespace blink {
...@@ -88,11 +88,6 @@ class BLINK_COMMON_EXPORT DocumentPolicy { ...@@ -88,11 +88,6 @@ class BLINK_COMMON_EXPORT DocumentPolicy {
// Returns the value of the given feature on the given origin. // Returns the value of the given feature on the given origin.
PolicyValue GetFeatureValue(mojom::FeaturePolicyFeature feature) const; PolicyValue GetFeatureValue(mojom::FeaturePolicyFeature feature) const;
// Returns the current threshold values assigned to all document policies.
// the declared header policy as well as any unadvertised required policies
// (such as sandbox policies).
FeatureState GetFeatureState() const;
// Returns true if the incoming policy is compatible with the given required // Returns true if the incoming policy is compatible with the given required
// policy, i.e. incoming policy is at least as strict as required policy. // policy, i.e. incoming policy is at least as strict as required policy.
static bool IsPolicyCompatible(const FeatureState& required_policy, static bool IsPolicyCompatible(const FeatureState& required_policy,
...@@ -124,11 +119,10 @@ class BLINK_COMMON_EXPORT DocumentPolicy { ...@@ -124,11 +119,10 @@ class BLINK_COMMON_EXPORT DocumentPolicy {
void UpdateFeatureState(const FeatureState& feature_state); void UpdateFeatureState(const FeatureState& feature_state);
// Threshold values for each defined feature. // Internal feature state is represented as an array to avoid overhead
// TODO(iclelland): Generate these members; pack booleans in bitfields if // in using container classes.
// possible. PolicyValue internal_feature_state_
bool font_display_ = true; [static_cast<size_t>(mojom::FeaturePolicyFeature::kMaxValue) + 1];
double unoptimized_lossless_images_ = std::numeric_limits<double>::infinity();
DISALLOW_COPY_AND_ASSIGN(DocumentPolicy); DISALLOW_COPY_AND_ASSIGN(DocumentPolicy);
}; };
......
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