Commit 6c893b83 authored by Fabrice de Gans-Riberi's avatar Fabrice de Gans-Riberi Committed by Commit Bot

Revert "Simplify SVGImage::ResetAnimation"

This reverts commit bcff0792.

Reason for revert: Speculative revert due to svg/as-image/animated-svg-wrong-timesource.html failure.
See https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20Leak/8508 for first failure.

Original change's description:
> Simplify SVGImage::ResetAnimation
> 
> Now that SVGSVGElement::setCurrentTime(...) (or more specifically
> SMILTimeContainer::SetElapsed) no longer trigger further updates when
> the time container is paused, we can remove the "pending timeline
> rewind" workaround.
> When doing this also take the opportunity to fold CanScheduleFrame(...)
> in SMILTimeContainer into its only caller and simplify a little bit.
> 
> Bug: 998526
> Change-Id: I9ff3de1e8c4b28284bbf3ed27d128206ea7ee857
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1976410
> Reviewed-by: Stephen Chenney <schenney@chromium.org>
> Commit-Queue: Fredrik Söderquist <fs@opera.com>
> Cr-Commit-Position: refs/heads/master@{#726808}

TBR=pdr@chromium.org,fs@opera.com,schenney@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 998526
Change-Id: Ia0e5739e3a33d2e3c875d2f600b5f2e035701294
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1980902Reviewed-by: default avatarFabrice de Gans-Riberi <fdegans@chromium.org>
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727236}
parent fe64b085
......@@ -397,6 +397,16 @@ void SMILTimeContainer::ServiceAnimations() {
UpdateAnimationsAndScheduleFrameIfNeeded(Elapsed());
}
bool SMILTimeContainer::CanScheduleFrame(SMILTime earliest_fire_time) const {
// If there's synchronization pending (most likely due to syncbases), then
// let that complete first before attempting to schedule a frame.
if (HasPendingSynchronization())
return false;
if (!IsTimelineRunning())
return false;
return earliest_fire_time.IsFinite();
}
void SMILTimeContainer::UpdateAnimationsAndScheduleFrameIfNeeded(
SMILTime elapsed) {
DCHECK(GetDocument().IsActive());
......@@ -404,13 +414,11 @@ void SMILTimeContainer::UpdateAnimationsAndScheduleFrameIfNeeded(
UpdateAnimationTimings(elapsed);
ApplyTimedEffects(elapsed);
DCHECK(!wakeup_timer_.IsActive());
DCHECK(!HasPendingSynchronization());
if (!IsTimelineRunning())
return;
SMILTime next_progress_time = NextProgressTime(elapsed);
if (!next_progress_time.IsFinite())
DCHECK(!wakeup_timer_.IsActive());
if (!CanScheduleFrame(next_progress_time))
return;
SMILTime delay_time = next_progress_time - elapsed;
DCHECK(delay_time.IsFinite());
......
......@@ -106,6 +106,7 @@ class SMILTimeContainer final : public GarbageCollected<SMILTimeContainer> {
void AnimationPolicyTimerFired(TimerBase*);
ImageAnimationPolicy AnimationPolicy() const;
bool HandleAnimationPolicy(AnimationPolicyOnceAction);
bool CanScheduleFrame(SMILTime earliest_fire_time) const;
void UpdateAnimationsAndScheduleFrameIfNeeded(SMILTime elapsed);
void ResetIntervals();
void UpdateIntervals(SMILTime presentation_time);
......
......@@ -99,7 +99,8 @@ class SVGImage::SVGImageLocalFrameClient : public EmptyLocalFrameClient {
SVGImage::SVGImage(ImageObserver* observer, bool is_multipart)
: Image(observer, is_multipart),
paint_controller_(std::make_unique<PaintController>()) {}
paint_controller_(std::make_unique<PaintController>()),
has_pending_timeline_rewind_(false) {}
SVGImage::~SVGImage() {
if (frame_client_)
......@@ -515,6 +516,13 @@ sk_sp<PaintRecord> SVGImage::PaintRecordForCurrentFrame(const KURL& url) {
// there may have been a previous url/fragment that needs to be reset.
view->ProcessUrlFragment(url, /*same_document_navigation=*/false);
// If the image was reset, we need to rewind the timeline back to 0. This
// needs to be done before painting, or else we wouldn't get the correct
// reset semantics (we'd paint the "last" frame rather than the one at
// time=0.) The reason we do this here and not in resetAnimation() is to
// avoid setting timers from the latter.
FlushPendingTimelineRewind();
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
view->UpdateAllLifecyclePhases(
DocumentLifecycle::LifecycleUpdateReason::kOther);
......@@ -557,6 +565,18 @@ void SVGImage::DrawInternal(cc::PaintCanvas* canvas,
StartAnimation();
}
void SVGImage::ScheduleTimelineRewind() {
has_pending_timeline_rewind_ = true;
}
void SVGImage::FlushPendingTimelineRewind() {
if (!has_pending_timeline_rewind_)
return;
if (SVGSVGElement* root_element = SvgRootElement(page_.Get()))
root_element->setCurrentTime(0);
has_pending_timeline_rewind_ = false;
}
void SVGImage::StartAnimation() {
SVGSVGElement* root_element = SvgRootElement(page_.Get());
if (!root_element)
......@@ -580,7 +600,7 @@ void SVGImage::ResetAnimation() {
return;
chrome_client_->SuspendAnimation();
root_element->pauseAnimations();
root_element->setCurrentTime(0);
ScheduleTimelineRewind();
}
void SVGImage::RestoreAnimation() {
......
......@@ -210,6 +210,8 @@ class CORE_EXPORT SVGImage final : public Image {
const KURL&);
void StopAnimation();
void ScheduleTimelineRewind();
void FlushPendingTimelineRewind();
Page* GetPageForTesting() { return page_; }
void LoadCompleted();
......@@ -225,8 +227,9 @@ class CORE_EXPORT SVGImage final : public Image {
// object size, which in turn depends on the container. One SVGImage may
// belong to multiple containers so the final image size can't be known in
// SVGImage. SVGImageForContainer carries the final image size, also called
// the "concrete object size". For more, see svg_image_for_container.h.
// the "concrete object size". For more, see: SVGImageForContainer.h
LayoutSize intrinsic_size_;
bool has_pending_timeline_rewind_;
enum LoadState {
kDataChangedNotStarted,
......
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