Post task when rejecting Animation promises inside ScriptForbiddenScope

third_party/WebKit/Source/core/animation/Animation.cpp

Index: third_party/WebKit/Source/core/animation/Animation.cpp
diff --git a/third_party/WebKit/Source/core/animation/Animation.cpp b/third_party/WebKit/Source/core/animation/Animation.cpp
index b6aa9faaca7005f07091ef3fa11042228d128c2c..4b5d1b7ce8c5f0c1b7f64487a3008dc0e9341acc 100644
--- a/third_party/WebKit/Source/core/animation/Animation.cpp
+++ b/third_party/WebKit/Source/core/animation/Animation.cpp
@@ -1038,7 +1038,7 @@ Animation::PlayStateUpdateScope::~PlayStateUpdateScope() {
     if (new_play_state == kIdle) {
       if (animation_->ready_promise_->GetState() ==
           AnimationPromise::kPending) {
-        animation_->ready_promise_->Reject(DOMException::Create(kAbortError));
+        animation_->RejectPromise(animation_->ready_promise_.Get());
       }
       animation_->ready_promise_->Reset();
       animation_->ResolvePromiseMaybeAsync(animation_->ready_promise_.Get());
@@ -1055,8 +1055,7 @@ Animation::PlayStateUpdateScope::~PlayStateUpdateScope() {
     if (new_play_state == kIdle) {
       if (animation_->finished_promise_->GetState() ==
           AnimationPromise::kPending) {
-        animation_->finished_promise_->Reject(
-            DOMException::Create(kAbortError));
+        animation_->RejectPromise(animation_->finished_promise_);
       }
       animation_->finished_promise_->Reset();
     } else if (new_play_state == kFinished) {
@@ -1150,6 +1149,16 @@ void Animation::ResolvePromiseMaybeAsync(AnimationPromise* promise) {
   }
 }
 
+void Animation::RejectPromise(AnimationPromise* promise) {
+  // Animation promises can be rejected inside a ScriptForbiddenScope. When

[haraken, 2017/04/27 00:17:13]

Would you help me understand where the ScriptForbiddenScope is set?

[adithyas, 2017/04/27 19:09:52]

It's set in Document::updateStyleAndLayout:
https://chromium.googlesource.com/chromium/src/+/9afc2bc78b53f5439bce60e2de93...

+  // rejecting the promise with a DOMException, ToV8 is called on the exception,
+  // making it possible for the DOMException constructor to be called inside a
+  // forbidden scope. Since the constructor for DOMException is not user-defined
+  // (and cannot be overwritten), we can allow it to run in a forbidden scope.

[haraken, 2017/04/27 00:17:13]

Your analysis looks correct but for safety I'd prefer not allowing the script
execution while you're in ScriptForbiddenScope.

If you really need to reject the promise here, would it be an option to post a
task to reject the promise (which will run after the ScriptForbiddenScope
exits)?

[adithyas, 2017/04/27 19:09:52]

I've updated the CL to do this (i.e. post a task if inside a forbidden scope),
but I think it's simpler to reject the promise synchronously (considering we're
sure that no script is actually being executed). 

+  ScriptForbiddenScope::AllowUserAgentScript allow_script;
+  promise->Reject(DOMException::Create(kAbortError));
+}
+
 DEFINE_TRACE(Animation) {
   visitor->Trace(content_);
   visitor->Trace(timeline_);