Move animation update to follow paint phase for SPv2.

third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp

 /*
  * Copyright (C) 2006 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
  * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
  * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
  * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "core/svg/graphics/SVGImage.h"
 
+#include "core/animation/DocumentAnimations.h"
 #include "core/animation/DocumentTimeline.h"
 #include "core/dom/NodeTraversal.h"
 #include "core/dom/shadow/FlatTreeTraversal.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
+#include "core/layout/LayoutView.h"
 #include "core/layout/svg/LayoutSVGRoot.h"
 #include "core/loader/FrameLoadRequest.h"
 #include "core/paint/FloatClipRecorder.h"
 #include "core/paint/TransformRecorder.h"
 #include "core/style/ComputedStyle.h"
 #include "core/svg/SVGDocumentExtensions.h"
 #include "core/svg/SVGFEImageElement.h"
 #include "core/svg/SVGImageElement.h"
 #include "core/svg/SVGSVGElement.h"
 #include "core/svg/animation/SMILTimeContainer.h"
@@ skipping 470 matching lines @@
   // The calls below may trigger GCs, so set up the required persistent
   // reference on the ImageResourceContent which owns this SVGImage. By
   // transitivity, that will keep the associated SVGImageChromeClient object
   // alive.
   Persistent<ImageObserver> protect(getImageObserver());
   m_page->animator().serviceScriptedAnimations(monotonicAnimationStartTime);
   // Do *not* update the paint phase. It's critical to paint only when
   // actually generating painted output, not only for performance reasons,
   // but to preserve correct coherence of the cache of the output with
   // the needsRepaint bits of the PaintLayers in the image.
-  toLocalFrame(m_page->mainFrame())
-      ->view()
-      ->updateAllLifecyclePhasesExceptPaint();
+  FrameView* frameView = toLocalFrame(m_page->mainFrame())->view();
+  frameView->updateAllLifecyclePhasesExceptPaint();
+
+  // For SPv2 we run updateAnimations after the paint phase, but per above
+  // comment we don't want to run lifecycle through to paint for SVG images.
+  // Since we know that SVG images never have composited animations, we can just
+  // update animations directly here without worrying about including
+  // PaintArtifactCompositor analysis of whether animations should be
+  // composited.
+  // TODO(wkorman): I'm not actually sure whether this is kosher. Is it possible

[wkorman, 2017/02/16 23:42:08]

This patch now fixes the SVG image test that was failing, but see my TODO
comment here.

To gain confidence and remove this TODO I believe we'd need to know that all
code paths that involve adding an animation are assured to have moved document
lifecycle past paint before this method in SVGImage is called.

I will look into that, your feedback as add'l checkpoint helpful.

[wkorman, 2017/02/17 19:35:14]

I looked through things and I think this is ok too.

Lingering concern is that if we have multiple SVGImages I believe they each have
their own timer, and so depending on when they fire we could call
updateAnimations() multiple times. But I think that will just cause performance
overhead, which hopefully we'd see in perf benchmarks or UMA or otherwise
realize if it were problematic, and we could find a way to aggregate or
otherwise resolve then.

+  // that a potentially-compositable-animation, call it 'A', is added, but
+  // FrameView hasn't yet run updateAnimations(), and then our SVG animation
+  // timer fires and we run updateAnimations() below, but without the
+  // PAC-provided info we need to potentially decide not to composite 'A'. Then
+  // updateAnimations() will analyze and start all animations, and could
+  // potentially end up compositing something we'd want not to composite.
+  if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
+    DocumentAnimations::updateAnimations(frameView->layoutView()->document(),
+                                         DocumentLifecycle::LayoutClean);
+  }
 }
 
 void SVGImage::advanceAnimationForTesting() {
   if (SVGSVGElement* rootElement = svgRootElement(m_page.get())) {
     rootElement->timeContainer()->advanceFrameForTesting();
 
     // The following triggers animation updates which can issue a new draw
     // but will not permanently change the animation timeline.
     // TODO(pdr): Actually advance the document timeline so CSS animations
     // can be properly tested.
@@ skipping 113 matching lines @@
   }
 
   return m_page ? SizeAvailable : SizeUnavailable;
 }
 
 String SVGImage::filenameExtension() const {
   return "svg";
 }
 
 }  // namespace blink