Skip to content

GitLab

T
tangled
Commit f1310529 authored by Yi Gu's avatar Yi Gu Committed by Commit Bot
Browse files
Options

Throw exception upon trying to play invalid worklet animations 

Currently we are printing out a console message upon trying to play an
invalid worklet animation, e.g. no-keyframe effect. Rather, an exception
should be thrown.

Note that regular animations accept effects without any keyframes, but
it currently does not make much sense in our implementation of worklet
animation.

It's a little bit strict to throw ATM, but once our implementation is
able to support it we can remove this exception.

Bug: 882939
Change-Id: Ia63b67b70adeb132f5df2c8679935551ec3ac1d7
Reviewed-on: https://chromium-review.googlesource.com/1225091Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592921}
parent 2da56e8e
Hide whitespace changes
Inline Side-by-side
Showing with 37 additions and 9 deletions
third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/worklet-animation-with-invalid-effect.html 0 → 100644
View file @ f1310529
<!DOCTYPE html>
<style>
#target {
width: 100px;
height: 100px;
}
</style>
<div id="target"></div>
<script src='../../../../resources/testharness.js'></script>
<script src='../../../../resources/testharnessreport.js'></script>
<script>
test(function() {
let playFunc = function() {
let effect = new KeyframeEffect(
document.getElementById('target'),
[
// No keyframe.
],
{ duration: 1000 }
);
let animation = new WorkletAnimation('test', effect);
animation.play();
}
assert_throws('InvalidStateError', playFunc);
}, 'Trying to play invalid worklet animation should throw an exception.');
</script>
third_party/blink/renderer/modules/animationworklet/worklet_animation.cc
View file @ f1310529
...@@ -310,7 +310,7 @@ String WorkletAnimation::playState() { ...@@ -310,7 +310,7 @@ String WorkletAnimation::playState() {
return Animation::PlayStateString(play_state_); return Animation::PlayStateString(play_state_);
} }
void WorkletAnimation::play() { void WorkletAnimation::play(ExceptionState& exception_state) {
DCHECK(IsMainThread()); DCHECK(IsMainThread());
if (play_state_ == Animation::kPending) if (play_state_ == Animation::kPending)
return; return;
...@@ -318,10 +318,8 @@ void WorkletAnimation::play() { ...@@ -318,10 +318,8 @@ void WorkletAnimation::play() {
String failure_message; String failure_message;
if (!CheckCanStart(&failure_message)) { if (!CheckCanStart(&failure_message)) {
// TODO(yigu): Throw an exception instead of a console message. exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
// https://crbug.com/882939. failure_message);
document_->AddConsoleMessage(ConsoleMessage::Create(
kOtherMessageSource, kErrorMessageLevel, failure_message));
return; return;
} }
......
third_party/blink/renderer/modules/animationworklet/worklet_animation.h
View file @ f1310529
...@@ -66,7 +66,7 @@ class MODULES_EXPORT WorkletAnimation : public WorkletAnimationBase, ...@@ -66,7 +66,7 @@ class MODULES_EXPORT WorkletAnimation : public WorkletAnimationBase,
AnimationTimeline* timeline() { return timeline_; } AnimationTimeline* timeline() { return timeline_; }
String playState(); String playState();
void play(); void play(ExceptionState& exception_state);
void cancel(); void cancel();
// AnimationEffectOwner implementation: // AnimationEffectOwner implementation:
......
third_party/blink/renderer/modules/animationworklet/worklet_animation.idl
View file @ f1310529
...@@ -18,6 +18,6 @@ ...@@ -18,6 +18,6 @@
] interface WorkletAnimation { ] interface WorkletAnimation {
readonly attribute AnimationTimeline? timeline; readonly attribute AnimationTimeline? timeline;
readonly attribute AnimationPlayState playState; readonly attribute AnimationPlayState playState;
void play(); [RaisesException] void play();
void cancel(); void cancel();
}; };
third_party/blink/renderer/modules/animationworklet/worklet_animation_test.cc
View file @ f1310529
...@@ -76,7 +76,8 @@ class WorkletAnimationTest : public RenderingTest { ...@@ -76,7 +76,8 @@ class WorkletAnimationTest : public RenderingTest {
}; };
TEST_F(WorkletAnimationTest, WorkletAnimationInElementAnimations) { TEST_F(WorkletAnimationTest, WorkletAnimationInElementAnimations) {
worklet_animation_->play(); DummyExceptionStateForTesting exception_state;
worklet_animation_->play(exception_state);
EXPECT_EQ(1u, EXPECT_EQ(1u,
element_->EnsureElementAnimations().GetWorkletAnimations().size()); element_->EnsureElementAnimations().GetWorkletAnimations().size());
worklet_animation_->cancel(); worklet_animation_->cancel();
...@@ -88,7 +89,8 @@ TEST_F(WorkletAnimationTest, StyleHasCurrentAnimation) { ...@@ -88,7 +89,8 @@ TEST_F(WorkletAnimationTest, StyleHasCurrentAnimation) {
scoped_refptr<ComputedStyle> style = scoped_refptr<ComputedStyle> style =
GetDocument().EnsureStyleResolver().StyleForElement(element_).get(); GetDocument().EnsureStyleResolver().StyleForElement(element_).get();
EXPECT_EQ(false, style->HasCurrentOpacityAnimation()); EXPECT_EQ(false, style->HasCurrentOpacityAnimation());
worklet_animation_->play(); DummyExceptionStateForTesting exception_state;
worklet_animation_->play(exception_state);
element_->EnsureElementAnimations().UpdateAnimationFlags(*style); element_->EnsureElementAnimations().UpdateAnimationFlags(*style);
EXPECT_EQ(true, style->HasCurrentOpacityAnimation()); EXPECT_EQ(true, style->HasCurrentOpacityAnimation());
} }
......
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