commit 9580bba0e0de87cf9ddbfbd84904fa1411813e84
author Fredrik Söderquist <fs@opera.com> Wed Jan 30 14:52:25 2019
committer Commit Bot <commit-bot@chromium.org> Wed Jan 30 14:52:25 2019
tree 85661259196e74957eba84ea77f55b7c123ef907
parent b5d8c8bc2c8b3ea986f29e5ef96f1fd90ff1847b

Call ResetAnimation synchronously in ImageResource::AllClientsAndObserversRemoved

Running ResetAnimation asynchronously gives the "wake-up timer" used by
the SMIL animation engine an opportunity to race with the actual
sweeping of the surrounding objects (ImageResourceContent, SVGImage with
contained Page). Said sweeping could thus take place when the handler
for the "wake-up timer" was running, leading to UAFs. Running
ResetAnimation synchronously stops the "wake-up timer" and prevents the
race.

This essentially reverts the workaround added by
r400934 (crbug.com/613709). After the change made by
r412798 (crbug.com/627418), the issue worked around - that the SMIL
animation engine could be re-entered via the ResetAnimation call during
GC - have been eliminated. (Now, after said CL, what
SVGImage::ResetAnimation does is to pause the animation, stopping all
timers, and set a flag that the animation state needs to be reset. The
resetting then happens later as needed.)

Bug: 924450
Change-Id: Ideef98f05c81d779950aac56506cbbe152762afa
Reviewed-on: https://chromium-review.googlesource.com/c/1445935
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#627431}

third_party/blink/renderer/core/loader/resource/image_resource.cc

diff --git a/third_party/blink/renderer/core/loader/resource/image_resource.cc b/third_party/blink/renderer/core/loader/resource/image_resource.cc
index 651e85b..889d2d65 100644
--- a/third_party/blink/renderer/core/loader/resource/image_resource.cc
+++ b/third_party/blink/renderer/core/loader/resource/image_resource.cc
@@ -313,16 +313,7 @@
   // TODO(hiroshige): Make the CHECK condition cleaner.
   CHECK(is_during_finish_as_error_ || !GetContent()->HasImage() ||
         !ErrorOccurred());
-  // If possible, delay the resetting until back at the event loop. Doing so
-  // after a conservative GC prevents resetAnimation() from upsetting ongoing
-  // animation updates (crbug.com/613709)
-  if (!ThreadHeap::WillObjectBeLazilySwept(this)) {
-    Thread::Current()->GetTaskRunner()->PostTask(
-        FROM_HERE, WTF::Bind(&ImageResourceContent::DoResetAnimation,
-                             WrapWeakPersistent(GetContent())));
-  } else {
-    GetContent()->DoResetAnimation();
-  }
+  GetContent()->DoResetAnimation();
   if (multipart_parser_)
     multipart_parser_->Cancel();
   Resource::AllClientsAndObserversRemoved();