Skip to content

Commit

Permalink
Remove WebAnimationsAPI runtime-enabled feature.
Browse files Browse the repository at this point in the history
It has been shipping for over three years:
  https://chromium-review.googlesource.com/c/chromium/src/+/2161345

While here, remove animation_effect_timing.idl, which was not attached
to the build at all.

style-change-events.html was sensitive to the order in which properties
are enumerated, which is affected by the feature status (since
conditionally enabled features are added later), and so the expectations
need to be rebaselined.

Bug: 978551
Change-Id: Id9d10c06256725cc61cc3f9c3a322fb34fd1b78f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5014558
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1222539}
  • Loading branch information
jeremyroman authored and Chromium LUCI CQ committed Nov 9, 2023
1 parent f8d4ce8 commit 0974851
Show file tree
Hide file tree
Showing 19 changed files with 34 additions and 228 deletions.
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/animation/animatable.idl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
// https://w3.org/TR/web-animations-1/#the-animatable-interface-mixin
interface mixin Animatable {
[CallWith=ScriptState, Measure, RaisesException] Animation animate(object? keyframes, optional (unrestricted double or KeyframeAnimationOptions) options);
[RuntimeEnabled=WebAnimationsAPI] sequence<Animation> getAnimations(optional GetAnimationsOptions options = {});
sequence<Animation> getAnimations(optional GetAnimationsOptions options = {});
};

// https://drafts.csswg.org/web-animations-1/#extensions-to-the-element-interface
Expand Down
14 changes: 7 additions & 7 deletions third_party/blink/renderer/core/animation/animation.idl
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,27 @@ enum ReplaceState { "active", "removed", "persisted" };
] interface Animation : EventTarget {
[CallWith=ExecutionContext, RaisesException] constructor(optional AnimationEffect? effect = null, optional AnimationTimeline? timeline);
[Measure] attribute AnimationEffect? effect;
[RuntimeEnabled=WebAnimationsAPI] attribute AnimationTimeline? timeline;
attribute AnimationTimeline? timeline;
[Measure, RaisesException=Setter] attribute CSSNumberish? startTime;
[Measure, RaisesException=Setter] attribute CSSNumberish? currentTime;
[Measure, RaisesException=Setter] attribute double playbackRate;
[RuntimeEnabled=ScrollTimeline, Measure, RaisesException=Setter] attribute (TimelineRangeOffset or DOMString) rangeStart;
[RuntimeEnabled=ScrollTimeline, Measure, RaisesException=Setter] attribute (TimelineRangeOffset or DOMString) rangeEnd;
[Measure] readonly attribute AnimationPlayState playState;
[RuntimeEnabled=WebAnimationsAPI, Measure] readonly attribute ReplaceState replaceState;
[Measure] readonly attribute ReplaceState replaceState;
[Measure] readonly attribute boolean pending;
[RuntimeEnabled=WebAnimationsAPI, Measure, RaisesException, CEReactions] void commitStyles();
[Measure, RaisesException, CEReactions] void commitStyles();
[Measure, RaisesException] void finish();
[Measure, RaisesException] void play();
[Measure, RaisesException] void pause();
[Measure, RaisesException] void reverse();
[Measure, RaisesException] void updatePlaybackRate(double playback_rate);
[RuntimeEnabled=WebAnimationsAPI, Measure] void persist();
[Measure] void persist();
[Measure] attribute DOMString id;
[Measure] void cancel();
[Measure] attribute EventHandler onfinish;
[Measure] attribute EventHandler oncancel;
[RuntimeEnabled=WebAnimationsAPI, Measure] attribute EventHandler onremove;
[RuntimeEnabled=WebAnimationsAPI, CallWith=ScriptState] readonly attribute Promise<Animation> finished;
[RuntimeEnabled=WebAnimationsAPI, CallWith=ScriptState] readonly attribute Promise<Animation> ready;
[Measure] attribute EventHandler onremove;
[CallWith=ScriptState] readonly attribute Promise<Animation> finished;
[CallWith=ScriptState] readonly attribute Promise<Animation> ready;
};

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ TEST_F(AnimationSimTest, CustomPropertyBaseComputedStyle) {
// around and not be valid in the exit frame of the next custom property
// animation.

ScopedWebAnimationsAPIForTest web_animations(true);

SimRequest main_resource("https://example.com/", "text/html");
LoadURL("https://example.com/");
main_resource.Complete("<div id=\"target\"></div>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

enum TimelinePhase { "inactive", "before", "active", "after" };
[
RuntimeEnabled=WebAnimationsAPI,
Exposed=Window
] interface AnimationTimeline {
readonly attribute CSSNumberish? currentTime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

// https://drafts.csswg.org/css-animations-2/#the-CSSAnimation-interface

[Exposed=Window, RuntimeEnabled=WebAnimationsAPI]
[Exposed=Window]
interface CSSAnimation : Animation {
readonly attribute CSSOMString animationName;
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

// https://drafts.csswg.org/css-transitions-2/#the-CSSTransition-interface

[Exposed=Window, RuntimeEnabled=WebAnimationsAPI]
[Exposed=Window]
interface CSSTransition : Animation {
readonly attribute CSSOMString transitionProperty;
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
// https://w3.org/TR/web-animations-1/#extensions-to-the-document-interface

[
ImplementedAs=DocumentAnimation,
RuntimeEnabled=WebAnimationsAPI
ImplementedAs=DocumentAnimation
] partial interface Document {
readonly attribute DocumentTimeline timeline;
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
// https://w3.org/TR/web-animations-1/#the-documenttimeline-interface

[
RuntimeEnabled=WebAnimationsAPI,
Exposed=Window
] interface DocumentTimeline : AnimationTimeline {
[CallWith=ExecutionContext] constructor(optional DocumentTimelineOptions options = {});
Expand Down
118 changes: 3 additions & 115 deletions third_party/blink/renderer/core/animation/effect_input.cc
Original file line number Diff line number Diff line change
Expand Up @@ -315,70 +315,6 @@ void SetKeyframeValue(Element* element,
keyframe.SetSVGAttributeValue(*svg_attribute, value);
}

bool ValidatePartialKeyframes(const StringKeyframeVector& keyframes) {
// WebAnimationsAPIEnabled guards both additive animations and allowing
// partial (implicit) keyframes.
if (RuntimeEnabledFeatures::WebAnimationsAPIEnabled())
return true;

// An implicit keyframe is inserted in the below cases. Note that the 'first'
// keyframe is actually all keyframes with offset 0.0, and the 'last' keyframe
// is actually all keyframes with offset 1.0.
//
// 1. A given property is present somewhere in the full set of keyframes,
// but is either not present in the first keyframe (requiring an implicit
// start value for that property) or last keyframe (requiring an implicit
// end value for that property).
//
// 2. There is no first keyframe (requiring an implicit start keyframe), or
// no last keyframe (requiring an implicit end keyframe).
//
// We only care about CSS properties here; animating SVG elements is protected
// by a different runtime flag.

Vector<double> computed_offsets =
KeyframeEffectModelBase::GetComputedOffsets(keyframes);

PropertyHandleSet properties_with_offset_0;
PropertyHandleSet properties_with_offset_1;
for (wtf_size_t i = 0; i < keyframes.size(); i++) {
for (const PropertyHandle& property : keyframes[i]->Properties()) {
if (!property.IsCSSProperty())
continue;

if (computed_offsets[i] == 0.0) {
properties_with_offset_0.insert(property);
} else {
if (!properties_with_offset_0.Contains(property))
return false;
if (computed_offsets[i] == 1.0) {
properties_with_offset_1.insert(property);
}
}
}
}

// At this point we have compared all keyframes with offset > 0 against the
// properties contained in the first keyframe, and found that they match. Now
// we just need to make sure that there aren't any properties in the first
// keyframe that aren't in the last keyframe.
return properties_with_offset_0.size() == properties_with_offset_1.size();
}

// Ensures that a CompositeOperation is of an allowed value for a given
// StringKeyframe and the current runtime flags.
EffectModel::CompositeOperation ResolveCompositeOperationForKeyframe(
EffectModel::CompositeOperation composite,
StringKeyframe* keyframe) {
bool additive_composite = composite == EffectModel::kCompositeAdd ||
composite == EffectModel::kCompositeAccumulate;
if (!RuntimeEnabledFeatures::WebAnimationsAPIEnabled() &&
keyframe->HasCssProperty() && additive_composite) {
return EffectModel::kCompositeReplace;
}
return composite;
}

bool IsAnimatableKeyframeAttribute(const String& property,
Element* element,
const Document& document) {
Expand Down Expand Up @@ -570,8 +506,7 @@ StringKeyframeVector ConvertArrayForm(Element* element,
absl::optional<EffectModel::CompositeOperation> composite =
EffectModel::StringToCompositeOperation(base_keyframe->composite());
if (composite) {
keyframe->SetComposite(
ResolveCompositeOperationForKeyframe(composite.value(), keyframe));
keyframe->SetComposite(composite.value());
}

// 8.2. Let the timing function of frame be the result of parsing the
Expand Down Expand Up @@ -822,8 +757,7 @@ StringKeyframeVector ConvertObjectForm(Element* element,
absl::optional<EffectModel::CompositeOperation> composite =
composite_operations[i % composite_operations.size()];
if (composite) {
keyframe->SetComposite(
ResolveCompositeOperationForKeyframe(composite.value(), keyframe));
keyframe->SetComposite(composite.value());
}
}

Expand Down Expand Up @@ -851,22 +785,6 @@ StringKeyframeVector ConvertObjectForm(Element* element,
return results;
}

bool HasAdditiveCompositeCSSKeyframe(
const KeyframeEffectModelBase::KeyframeGroupMap& keyframe_groups) {
for (const auto& keyframe_group : keyframe_groups) {
PropertyHandle property = keyframe_group.key;
if (!property.IsCSSProperty())
continue;
for (const auto& keyframe : keyframe_group.value->Keyframes()) {
if (keyframe->Composite() == EffectModel::kCompositeAdd ||
keyframe->Composite() == EffectModel::kCompositeAccumulate) {
return true;
}
}
}
return false;
}

} // namespace

KeyframeEffectModelBase* EffectInput::Convert(
Expand All @@ -880,19 +798,8 @@ KeyframeEffectModelBase* EffectInput::Convert(
if (exception_state.HadException())
return nullptr;

composite = ResolveCompositeOperation(composite, parsed_keyframes);

auto* keyframe_effect_model = MakeGarbageCollected<StringKeyframeEffectModel>(
return MakeGarbageCollected<StringKeyframeEffectModel>(
parsed_keyframes, composite, LinearTimingFunction::Shared());

if (!RuntimeEnabledFeatures::WebAnimationsAPIEnabled()) {
// This should be enforced by the parsing code.
DCHECK(!HasAdditiveCompositeCSSKeyframe(
keyframe_effect_model->GetPropertySpecificKeyframeGroups()));
}

DCHECK(!exception_state.HadException());
return keyframe_effect_model;
}

StringKeyframeVector EffectInput::ParseKeyframesArgument(
Expand Down Expand Up @@ -940,26 +847,7 @@ StringKeyframeVector EffectInput::ParseKeyframesArgument(
keyframe->SetLogicalPropertyResolutionContext(text_direction, writing_mode);
}

if (!ValidatePartialKeyframes(parsed_keyframes)) {
exception_state.ThrowDOMException(DOMExceptionCode::kNotSupportedError,
"Partial keyframes are not supported.");
return {};
}
return parsed_keyframes;
}

EffectModel::CompositeOperation EffectInput::ResolveCompositeOperation(
EffectModel::CompositeOperation composite,
const StringKeyframeVector& keyframes) {
EffectModel::CompositeOperation result = composite;
for (const Member<StringKeyframe>& keyframe : keyframes) {
// Replace is always supported, so we can early-exit if and when we have
// that as our composite value.
if (result == EffectModel::kCompositeReplace)
break;
result = ResolveCompositeOperationForKeyframe(result, keyframe);
}
return result;
}

} // namespace blink
9 changes: 0 additions & 9 deletions third_party/blink/renderer/core/animation/effect_input.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,6 @@ class CORE_EXPORT EffectInput {
const ScriptValue& keyframes,
ScriptState*,
ExceptionState&);

// Ensures that a CompositeOperation is of an allowed value for a set of
// StringKeyframes and the current runtime flags.
//
// Under certain runtime flags, additive composite operations are not allowed
// for CSS properties.
static EffectModel::CompositeOperation ResolveCompositeOperation(
EffectModel::CompositeOperation,
const StringKeyframeVector&);
};

} // namespace blink
Expand Down
15 changes: 6 additions & 9 deletions third_party/blink/renderer/core/animation/effect_stack.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,17 @@ void CopyToActiveInterpolationsMap(
target.insert(property, MakeGarbageCollected<ActiveInterpolations>());
ActiveInterpolations* active_interpolations = entry.stored_value->value;

// Assuming stacked effects are enabled, interpolations that depend on
// underlying values (e.g. have a non-replace composite mode) should be
// added onto the 'stack' of active interpolations. However any 'replace'
// effect erases everything that came before it, so we must clear the stack
// when that happens.
const bool allow_stacked_effects =
RuntimeEnabledFeatures::WebAnimationsAPIEnabled() ||
!property.IsCSSProperty() || property.IsPresentationAttribute();
// Interpolations that depend on underlying values (e.g. have a non-replace
// composite mode) should be added onto the 'stack' of active
// interpolations. However any 'replace' effect erases everything that came
// before it, so we must clear the stack when that happens.
const bool effect_depends_on_underlying_value =
interpolation->IsInvalidatableInterpolation() &&
To<InvalidatableInterpolation>(*interpolation.Get())
.DependsOnUnderlyingValue();
if (!allow_stacked_effects || !effect_depends_on_underlying_value)
if (!effect_depends_on_underlying_value) {
active_interpolations->clear();
}
active_interpolations->push_back(interpolation);
}
}
Expand Down
6 changes: 1 addition & 5 deletions third_party/blink/renderer/core/animation/keyframe_effect.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ KeyframeEffect* KeyframeEffect::Create(
composite =
EffectModel::StringToCompositeOperation(effect_options->composite())
.value();
if (RuntimeEnabledFeatures::WebAnimationsAPIEnabled() &&
!effect_options->pseudoElement().empty()) {
if (!effect_options->pseudoElement().empty()) {
pseudo = effect_options->pseudoElement();
if (!ValidateAndCanonicalizePseudo(pseudo)) {
// TODO(gtsteel): update when
Expand Down Expand Up @@ -355,9 +354,6 @@ void KeyframeEffect::setKeyframes(ScriptState* script_state,
}

void KeyframeEffect::SetKeyframes(StringKeyframeVector keyframes) {
Model()->SetComposite(
EffectInput::ResolveCompositeOperation(Model()->Composite(), keyframes));

To<StringKeyframeEffectModel>(Model())->SetFrames(keyframes);

// Changing the keyframes will invalidate any sampled effect, as well as
Expand Down
8 changes: 4 additions & 4 deletions third_party/blink/renderer/core/animation/keyframe_effect.idl
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ enum CompositeOperation { "replace", "add", "accumulate" };
[CallWith=ScriptState, RaisesException, Measure] constructor(Element? target, object? keyframes, optional (unrestricted double or KeyframeEffectOptions) options);
[CallWith=ScriptState, RaisesException, Measure] constructor(KeyframeEffect source);
attribute Element? target;
[RuntimeEnabled=WebAnimationsAPI, RaisesException=Setter] attribute CSSOMString? pseudoElement;
[RuntimeEnabled=WebAnimationsAPI] attribute CompositeOperation composite;
[CallWith=ScriptState, RuntimeEnabled=WebAnimationsAPI] sequence<object> getKeyframes();
[CallWith=ScriptState, RaisesException, RuntimeEnabled=WebAnimationsAPI] void setKeyframes(object? keyframes);
[RaisesException=Setter] attribute CSSOMString? pseudoElement;
attribute CompositeOperation composite;
[CallWith=ScriptState] sequence<object> getKeyframes();
[CallWith=ScriptState, RaisesException] void setKeyframes(object? keyframes);
};
36 changes: 0 additions & 36 deletions third_party/blink/renderer/core/animation/keyframe_effect_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -333,42 +333,6 @@ TEST_F(AnimationKeyframeEffectV8Test, SpecifiedDurationGetter) {
EXPECT_EQ("auto", duration2->GetAsString());
}

TEST_F(AnimationKeyframeEffectV8Test, SetKeyframesAdditiveCompositeOperation) {
// AnimationWorklet also needs to be disabled since it depends on
// WebAnimationsAPI and prevents us from turning it off if enabled.
ScopedAnimationWorkletForTest no_animation_worklet(false);
ScopedWebAnimationsAPIForTest no_web_animations(false);
V8TestingScope scope;
ScriptState* script_state = scope.GetScriptState();
ScriptValue js_keyframes = ScriptValue::CreateNull(scope.GetIsolate());
v8::Local<v8::Object> timing_input = v8::Object::New(scope.GetIsolate());
DummyExceptionStateForTesting exception_state;
KeyframeEffectOptions* timing_input_dictionary =
NativeValueTraits<KeyframeEffectOptions>::NativeValue(
scope.GetIsolate(), timing_input, exception_state);
ASSERT_FALSE(exception_state.HadException());

// Since there are no CSS-targeting keyframes, we can create a KeyframeEffect
// with composite = 'add'.
timing_input_dictionary->setComposite("add");
KeyframeEffect* effect = CreateAnimationFromOption(
script_state, element.Get(), js_keyframes, timing_input_dictionary);
EXPECT_EQ(effect->Model()->Composite(), EffectModel::kCompositeAdd);

// But if we then setKeyframes with CSS-targeting keyframes, the composite
// should fallback to 'replace'.
HeapVector<ScriptValue> blink_keyframes = {
V8ObjectBuilder(script_state).AddString("width", "10px").GetScriptValue(),
V8ObjectBuilder(script_state).AddString("width", "0px").GetScriptValue()};
ScriptValue new_js_keyframes(
scope.GetIsolate(),
ToV8Traits<IDLSequence<IDLObject>>::ToV8(script_state, blink_keyframes)
.ToLocalChecked());
effect->setKeyframes(script_state, new_js_keyframes, exception_state);
ASSERT_FALSE(exception_state.HadException());
EXPECT_EQ(effect->Model()->Composite(), EffectModel::kCompositeReplace);
}

TEST_F(KeyframeEffectTest, TimeToEffectChange) {
Timing timing;
timing.iteration_duration = ANIMATION_TIME_DELTA_FROM_SECONDS(100);
Expand Down
Loading

0 comments on commit 0974851

Please sign in to comment.