Skip to content

GitLab

T
tangled
Commit cb09b044 authored by Ehsan Karamad's avatar Ehsan Karamad Committed by Commit Bot
Browse files
Options

Violation reports for 'layout-animations' 

This CL adds support for generating violation reports for 'layout-animations'
feature policy. The new implementation triggers a report when:
  1- CSS Parser finds usage of @keyframes for one of the banned styles.
  2- element.animate() changes a banned style.

Bug: 902836,867471
Change-Id: I79900603eb0166514d7986dc189ec914bd95f899
Reviewed-on: https://chromium-review.googlesource.com/c/1324138
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608984}
parent 8ffa1567
Show whitespace changes
Inline Side-by-side
Showing with 257 additions and 8 deletions
third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/layout-animations-disabled-violation-report-js-tentative.html 0 → 100644
View file @ cb09b044
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/feature-policy/experimental-features/resources/common.js"></script>
<title> 'layout-animations' Policy : violation reports from javascript
</title>
<body>
<script>
test(() => {
document.policy.allowedFeatures().forEach((enabled_feature) => {
assert_not_equals(enabled_feature, "layout-animations");
});
},
"Verify 'layout-animations' is not in document's feature list.");
promise_test(async () => {
let promise = wait_for_violation_in_file(
"layout-animations",
"animation-property-height.js");
let script = document.createElement("script");
script.src = "/feature-policy/experimental-features/resources/" +
"animation-property-height.js";
document.body.appendChild(script);
await promise;
},
"Verify that when 'layout-animations' is disabled, an 'element.animate' " +
"API including a keyframe that uses a blocked property generates violation " +
"report (linked scripts).");
promise_test(async () => {
let promise = wait_for_violation_in_file(
"layout-animations",
"layout-animations-disabled-violation-report-js-tentative.html");
let div = document.createElement("div");
document.body.appendChild(div);
div.animate([{width: "100px"}, {width: "200px"}]);
await promise;
},
"Verify that when 'layout-animations' is disabled, an 'element.animate' " +
"API including a keyframe that uses a blocked property generates violation " +
"report (inline scripts).");
</script>
</body>
third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/layout-animations-disabled-violation-report-keyframes-tentative.html 0 → 100644
View file @ cb09b044
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/feature-policy/experimental-features/resources/common.js"></script>
<title> 'layout-animations' Policy : violation reports from CSS keyframes
</title>
<body>
<script>
test(() => {
document.policy.allowedFeatures().forEach((enabled_feature) => {
assert_not_equals(enabled_feature, "layout-animations");
});
},
"Sanity-check: 'layout-animations' is not in document's feature list.");
promise_test(async () => {
let promise = wait_for_violation_in_file(
"layout-animations", null /* source not specified */);
let style = document.createElement("style");
style.innerHTML = `@keyframes animation_property_top {
from { top: 0px }
to { top: 100px }
}`;
document.body.appendChild(style);
await promise;
},
"Verify that when 'layout-animations' is disabled, a keyframes which " +
"includes a blocked property generates violation report.");
</script>
</body>
third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/resources/animation-property-height.js 0 → 100644
View file @ cb09b044
let div = document.createElement("div");
document.body.appendChild(div);
div.animate([{"height": "0px"}, {"height": "100px"}]);
third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/resources/common.js
View file @ cb09b044
......@@ -65,3 +65,28 @@ function wait_for_load(e) {
e.addEventListener("load", resolve);
});
}
window.reporting_observer_instance = new ReportingObserver((reports, observer) => {
if (window.reporting_observer_callback) {
reports.forEach(window.reporting_observer_callback);
}
}, {types: ["feature-policy"]});
window.reporting_observer_instance.observe();
window.reporting_observer_callback = null;
// Waits for a violation in |feature| and source file containing |file_name|.
function wait_for_violation_in_file(feature, file_name) {
return new Promise( (resolve) => {
assert_equals(null, window.reporting_observer_callback);
window.reporting_observer_callback = (report) => {
var feature_match = (feature === report.body.feature);
var file_name_match =
!file_name ||
(report.body.sourceFile.indexOf(file_name) !== -1);
if (feature_match && file_name_match) {
window.reporting_observer_callback = null;
resolve(report);
}
};
});
}
third_party/blink/renderer/core/animation/css_interpolation_types_map.cc
View file @ cb09b044
......@@ -46,6 +46,7 @@
#include "third_party/blink/renderer/core/css/css_syntax_descriptor.h"
#include "third_party/blink/renderer/core/css/properties/css_property.h"
#include "third_party/blink/renderer/core/css/property_registry.h"
#include "third_party/blink/renderer/core/feature_policy/layout_animations_policy.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
namespace blink {
......@@ -113,12 +114,7 @@ const InterpolationTypes& CSSInterpolationTypesMap::Get(
// TODO(crbug.com/838263): Support site-defined list of acceptable properties
// through feature policy declarations.
bool property_maybe_blocked_by_feature_policy =
css_property.IDEquals(CSSPropertyBottom) ||
css_property.IDEquals(CSSPropertyHeight) ||
css_property.IDEquals(CSSPropertyLeft) ||
css_property.IDEquals(CSSPropertyRight) ||
css_property.IDEquals(CSSPropertyTop) ||
css_property.IDEquals(CSSPropertyWidth);
LayoutAnimationsPolicy::AffectedCSSProperties().Contains(&css_property);
if (allow_all_animations_ || !property_maybe_blocked_by_feature_policy) {
switch (css_property.PropertyID()) {
case CSSPropertyBaselineShift:
......
third_party/blink/renderer/core/animation/element_animation.cc
View file @ cb09b044
......@@ -14,10 +14,30 @@
#include "third_party/blink/renderer/core/animation/timing_input.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/element.h"
#include "third_party/blink/renderer/core/feature_policy/feature_policy.h"
#include "third_party/blink/renderer/core/feature_policy/layout_animations_policy.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/bindings/script_state.h"
namespace blink {
namespace {
// A helper method which is used to trigger a violation report for cases where
// the |element.animate| API is used to animate a CSS property which is blocked
// by the feature policy 'layout-animations'.
void ReportFeaturePolicyViolationsIfNecessary(
const Document& document,
const KeyframeEffectModelBase& effect) {
if (document.IsFeatureEnabled(mojom::FeaturePolicyFeature::kLayoutAnimations))
return;
for (const auto* blocked_property :
LayoutAnimationsPolicy::AffectedCSSProperties()) {
if (effect.Affects(PropertyHandle(*blocked_property)))
LayoutAnimationsPolicy::ReportViolation(*blocked_property, document);
}
}
} // namespace
Animation* ElementAnimation::animate(
ScriptState* script_state,
......@@ -81,6 +101,7 @@ HeapVector<Member<Animation>> ElementAnimation::getAnimations(
Animation* ElementAnimation::animateInternal(Element& element,
KeyframeEffectModelBase* effect,
const Timing& timing) {
ReportFeaturePolicyViolationsIfNecessary(element.GetDocument(), *effect);
KeyframeEffect* keyframe_effect =
KeyframeEffect::Create(&element, effect, timing);
return element.GetDocument().Timeline().Play(keyframe_effect);
......
third_party/blink/renderer/core/css/parser/css_parser_context.cc
View file @ cb09b044
......@@ -4,9 +4,14 @@
#include "third_party/blink/renderer/core/css/parser/css_parser_context.h"
#include "third_party/blink/renderer/core/css/css_custom_ident_value.h"
#include "third_party/blink/renderer/core/css/css_style_sheet.h"
#include "third_party/blink/renderer/core/css/css_value_list.h"
#include "third_party/blink/renderer/core/css/style_rule_keyframe.h"
#include "third_party/blink/renderer/core/css/style_sheet_contents.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/feature_policy/feature_policy.h"
#include "third_party/blink/renderer/core/feature_policy/layout_animations_policy.h"
#include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
#include "third_party/blink/renderer/core/frame/deprecation.h"
#include "third_party/blink/renderer/core/frame/settings.h"
......@@ -239,6 +244,22 @@ bool CSSParserContext::IsDocumentHandleEqual(const Document* other) const {
return document_.Get() == other;
}
bool CSSParserContext::IsLayoutAnimationsPolicyEnforced() const {
return document_ && !document_->IsFeatureEnabled(
mojom::FeaturePolicyFeature::kLayoutAnimations);
}
void CSSParserContext::ReportLayoutAnimationsViolationIfNeeded(
const StyleRuleKeyframe& rule) const {
DCHECK(IsLayoutAnimationsPolicyEnforced());
for (size_t i = 0; i < rule.Properties().PropertyCount(); ++i) {
const CSSProperty& property = rule.Properties().PropertyAt(i).Property();
if (!LayoutAnimationsPolicy::AffectedCSSProperties().Contains(&property))
continue;
LayoutAnimationsPolicy::ReportViolation(property, *document_);
}
}
void CSSParserContext::Trace(blink::Visitor* visitor) {
visitor->Trace(document_);
}
......
third_party/blink/renderer/core/css/parser/css_parser_context.h
View file @ cb09b044
......@@ -20,6 +20,7 @@ namespace blink {
class CSSStyleSheet;
class Document;
class StyleRuleKeyframe;
class StyleSheetContents;
class CORE_EXPORT CSSParserContext
......@@ -110,6 +111,13 @@ class CORE_EXPORT CSSParserContext
return should_check_content_security_policy_;
}
// TODO(ekaramad): We currently only report @keyframes violations. We need to
// report CSS transitions as well (https://crbug.com/906147).
// TODO(ekaramad): We should provide a source location in the violation
// report (https://crbug.com/906150, ).
bool IsLayoutAnimationsPolicyEnforced() const;
void ReportLayoutAnimationsViolationIfNeeded(const StyleRuleKeyframe&) const;
void Trace(blink::Visitor*);
private:
......
third_party/blink/renderer/core/css/parser/css_parser_impl.cc
View file @ cb09b044
......@@ -547,7 +547,12 @@ StyleRuleBase* CSSParserImpl::ConsumeQualifiedRule(
return nullptr; // Parse error, EOF instead of qualified rule block
CSSParserTokenStream::BlockGuard guard(stream);
return ConsumeKeyframeStyleRule(prelude, prelude_offset, stream);
StyleRuleKeyframe* keyframe_style_rule =
ConsumeKeyframeStyleRule(prelude, prelude_offset, stream);
if (context_->IsLayoutAnimationsPolicyEnforced()) {
context_->ReportLayoutAnimationsViolationIfNeeded(*keyframe_style_rule);
}
return keyframe_style_rule;
}
NOTREACHED();
......
third_party/blink/renderer/core/feature_policy/BUILD.gn
View file @ cb09b044
......@@ -10,6 +10,8 @@ blink_core_sources("feature_policy") {
"feature_policy.cc",
"feature_policy.h",
"iframe_policy.h",
"layout_animations_policy.cc",
"layout_animations_policy.h",
"policy.cc",
"policy.h",
]
......
third_party/blink/renderer/core/feature_policy/layout_animations_policy.cc 0 → 100644
View file @ cb09b044
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/core/feature_policy/layout_animations_policy.h"
#include "third_party/blink/renderer/core/css/properties/css_property.h"
#include "third_party/blink/renderer/core/execution_context/security_context.h"
#include "third_party/blink/renderer/core/feature_policy/feature_policy.h"
#include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
namespace blink {
namespace {
String GetViolationMessage(const CSSProperty& property) {
return String::Format(
"Feature policy violation: CSS property '%s' violates feature policy "
"'%s' which is disabled in this document",
property.GetPropertyNameString().Utf8().data(),
GetNameForFeature(mojom::FeaturePolicyFeature::kLayoutAnimations)
.Utf8()
.data());
}
} // namespace
LayoutAnimationsPolicy::LayoutAnimationsPolicy() = default;
// static
const HashSet<const CSSProperty*>&
LayoutAnimationsPolicy::AffectedCSSProperties() {
DEFINE_STATIC_LOCAL(
HashSet<const CSSProperty*>, properties,
({&GetCSSPropertyBottom(), &GetCSSPropertyHeight(), &GetCSSPropertyLeft(),
&GetCSSPropertyRight(), &GetCSSPropertyTop(), &GetCSSPropertyWidth()}));
return properties;
}
// static
// static
void LayoutAnimationsPolicy::ReportViolation(
const CSSProperty& animated_property,
const SecurityContext& security_context) {
DCHECK(AffectedCSSProperties().Contains(&animated_property));
auto state = security_context.GetFeatureEnabledState(
mojom::FeaturePolicyFeature::kLayoutAnimations);
DCHECK_NE(FeatureEnabledState::kEnabled, state);
security_context.ReportFeaturePolicyViolation(
mojom::FeaturePolicyFeature::kLayoutAnimations,
state == FeatureEnabledState::kReportOnly
? mojom::FeaturePolicyDisposition::kReport
: mojom::FeaturePolicyDisposition::kEnforce,
GetViolationMessage(animated_property));
}
} // namespace blink
third_party/blink/renderer/core/feature_policy/layout_animations_policy.h 0 → 100644
View file @ cb09b044
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_LAYOUT_ANIMATIONS_POLICY_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_LAYOUT_ANIMATIONS_POLICY_H_
#include "third_party/blink/public/common/feature_policy/feature_policy.h"
#include "third_party/blink/renderer/platform/wtf/allocator.h"
#include "third_party/blink/renderer/platform/wtf/hash_set.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
namespace blink {
class CSSProperty;
class SecurityContext;
// Helper methods for for 'layout-animations' (kLayoutAnimations) feature
// policy.
class LayoutAnimationsPolicy {
DISALLOW_NEW();
public:
// Returns a set of the CSS properties which are affected by the feature
// policy 'layout-animations'.
static const HashSet<const CSSProperty*>& AffectedCSSProperties();
// Generates a violation report for the blocked |animation_property|.
static void ReportViolation(const CSSProperty& animated_property,
const SecurityContext& security_context);
private:
LayoutAnimationsPolicy();
DISALLOW_COPY_AND_ASSIGN(LayoutAnimationsPolicy);
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_FEATURE_POLICY_LAYOUT_ANIMATIONS_POLICY_H_
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