Post task when rejecting Animation promises inside ScriptForbiddenScope

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

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 1020 matching lines @@
           "blink.animations,devtools.timeline,benchmark,rail", "Animation",
           animation_, "data", InspectorAnimationStateEvent::Data(*animation_));
   }
 
   // Ordering is important, the ready promise should resolve/reject before
   // the finished promise.
   if (animation_->ready_promise_ && new_play_state != old_play_state) {
     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());
     } else if (old_play_state == kPending) {
       animation_->ResolvePromiseMaybeAsync(animation_->ready_promise_.Get());
     } else if (new_play_state == kPending) {
       DCHECK_NE(animation_->ready_promise_->GetState(),
                 AnimationPromise::kPending);
       animation_->ready_promise_->Reset();
     }
   }
 
   if (animation_->finished_promise_ && new_play_state != old_play_state) {
     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) {
       animation_->ResolvePromiseMaybeAsync(animation_->finished_promise_.Get());
     } else if (old_play_state == kFinished) {
       animation_->finished_promise_->Reset();
     }
   }
 
   if (old_play_state != new_play_state &&
@@ skipping 73 matching lines @@
   if (ScriptForbiddenScope::IsScriptForbidden()) {
     TaskRunnerHelper::Get(TaskType::kDOMManipulation, GetExecutionContext())
         ->PostTask(BLINK_FROM_HERE,
                    WTF::Bind(&AnimationPromise::Resolve<Animation*>,
                              WrapPersistent(promise), WrapPersistent(this)));
   } else {
     promise->Resolve(this);
   }
 }
 
+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_);
   visitor->Trace(pending_finished_event_);
   visitor->Trace(pending_cancelled_event_);
   visitor->Trace(finished_promise_);
   visitor->Trace(ready_promise_);
   visitor->Trace(compositor_player_);
   EventTargetWithInlineData::Trace(visitor);
   ContextLifecycleObserver::Trace(visitor);
@@ skipping 19 matching lines @@
   DCHECK(!compositor_player_);
 }
 
 void Animation::CompositorAnimationPlayerHolder::Detach() {
   DCHECK(compositor_player_);
   compositor_player_->SetAnimationDelegate(nullptr);
   animation_ = nullptr;
   compositor_player_.reset();
 }
 }  // namespace blink