Order bounds update correctly for LayoutSVGShape and LayoutSVGImage

third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp

 /*
  * Copyright (C) 2006 Alexander Kellett <lypanov@kde.org>
  * Copyright (C) 2006 Apple Computer, Inc.
  * Copyright (C) 2007 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2007, 2008, 2009 Rob Buis <buis@kde.org>
  * Copyright (C) 2009 Google, Inc.
  * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
  * Copyright (C) 2010 Patrick Gansterer <paroga@paroga.com>
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 63 matching lines @@
     m_needsBoundariesUpdate |= oldBoundaries != m_objectBoundingBox;
 }
 
 void LayoutSVGImage::layout()
 {
     ASSERT(needsLayout());
     LayoutAnalyzer::Scope analyzer(*this);
 
     updateBoundingBox();
 
-    bool transformOrBoundariesUpdate = m_needsTransformUpdate || m_needsBoundariesUpdate;
+    bool updateParentBoundaries = false;
     if (m_needsTransformUpdate) {
         m_localTransform = toSVGImageElement(element())->calculateAnimatedLocalTransform();
         m_needsTransformUpdate = false;
-    }
-
-    if (m_needsBoundariesUpdate) {
-        m_paintInvalidationBoundingBox = m_objectBoundingBox;
-        SVGLayoutSupport::intersectPaintInvalidationRectWithResources(this, m_paintInvalidationBoundingBox);
-
-        m_needsBoundariesUpdate = false;
+        updateParentBoundaries = true;
     }
 
     // Invalidate all resources of this client if our layout changed.

[pdr., 2016/04/22 22:33:33]

It's a little confusing to have this in different places in different svg layout
functions. Can we just move it to the very top in all of them?

[fs, 2016/04/22 22:53:07]

I hope we can... (This is pretty side-effecty - as was found out here - but
reasonably shouldn't touch/rely on state from the client.)

     if (everHadLayout() && selfNeedsLayout())
         SVGResourcesCache::clientLayoutChanged(this);
 
+    if (m_needsBoundariesUpdate) {
+        m_paintInvalidationBoundingBox = m_objectBoundingBox;
+        SVGLayoutSupport::intersectPaintInvalidationRectWithResources(this, m_paintInvalidationBoundingBox);
+        m_needsBoundariesUpdate = false;
+        updateParentBoundaries = true;
+    }
+
     // If our bounds changed, notify the parents.
-    if (transformOrBoundariesUpdate)
+    if (updateParentBoundaries)
         LayoutSVGModelObject::setNeedsBoundariesUpdate();
 
     clearNeedsLayout();

[pdr., 2016/04/22 22:33:33]

Possibly assert here (and in the other functions) that all the m_needs* are
false?

[fs, 2016/04/22 22:53:07]

Yeah, that's probably a good idea.

 }
 
 void LayoutSVGImage::paint(const PaintInfo& paintInfo, const LayoutPoint&) const
 {
     SVGImagePainter(*this).paint(paintInfo);
 }
 
 bool LayoutSVGImage::nodeAtFloatPoint(HitTestResult& result, const FloatPoint& pointInParent, HitTestAction hitTestAction)
 {
     // We only draw in the forground phase, so we only hit-test then.
@@ skipping 30 matching lines @@
     setShouldDoFullPaintInvalidation();
 }
 
 void LayoutSVGImage::addOutlineRects(Vector<LayoutRect>& rects, const LayoutPoint&, IncludeBlockVisualOverflowOrNot) const
 {
     // this is called from paint() after the localTransform has already been applied
     rects.append(LayoutRect(paintInvalidationRectInLocalSVGCoordinates()));
 }
 
 } // namespace blink