Use PositionedMovementLayout for changes to positioned elements' margins

Source/core/rendering/style/RenderStyle.cpp

 /*
  * Copyright (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 367 matching lines @@
         diff = m_svgStyle->diff(other.m_svgStyle.get());
 
     if ((!diff.needsFullLayout() || !diff.needsPaintInvalidation()) && diffNeedsFullLayoutAndPaintInvalidation(other)) {
         diff.setNeedsFullLayout();
         diff.setNeedsPaintInvalidationObject();
     }
 
     if (!diff.needsFullLayout() && diffNeedsFullLayout(other))
         diff.setNeedsFullLayout();
 
+    if (!diff.needsFullLayout() && surround->margin != other.surround->margin) {
+        if (position() == AbsolutePosition || position() == FixedPosition)

[Julien - ping for review, 2014/09/23 19:53:51]

Shouldn't we use position() != StaticPosition here like we do below?

Also wouldn't it make sense to refactor the code to handle this case right below
too as the conditions are pretty similar (surround->offset vs surround->margin)?

[rhogan, 2014/09/23 20:08:27]

We can't include RelativePosition here because their margins collapse - they do
need a layout. I should add a comment.
Yes it looked tempting to me too, and I tried it, but it got messy because of
the consideration above - and then I thought 'Maybe the next optimization we
find to add here will suggest a good refactoring, but for now I think I would be
making two straightforward cases look like one big unclear one.'

+            diff.setNeedsPositionedMovementLayout();
+        else
+            diff.setNeedsFullLayout();
+    }
+
     if (!diff.needsFullLayout() && position() != StaticPosition && surround->offset != other.surround->offset) {
         // Optimize for the case where a positioned layer is moving but not changing size.
         if (positionedObjectMovedOnly(surround->offset, other.surround->offset, m_box->width()))
             diff.setNeedsPositionedMovementLayout();
         else
             diff.setNeedsFullLayout();
     }
 
     if (diffNeedsPaintInvalidationLayer(other))
         diff.setNeedsPaintInvalidationLayer();
@@ skipping 198 matching lines @@
 
         if (m_box->boxSizing() != other.m_box->boxSizing())
             return true;
     }
 
     if (noninherited_flags.verticalAlign != other.noninherited_flags.verticalAlign
         || noninherited_flags.position != other.noninherited_flags.position)
         return true;
 
     if (surround.get() != other.surround.get()) {
-        if (surround->margin != other.surround->margin)
-            return true;
-
         if (surround->padding != other.surround->padding)
             return true;
     }
 
     if (rareNonInheritedData.get() != other.rareNonInheritedData.get()) {
         if (rareNonInheritedData->m_alignContent != other.rareNonInheritedData->m_alignContent
             || rareNonInheritedData->m_alignItems != other.rareNonInheritedData->m_alignItems
             || rareNonInheritedData->m_alignSelf != other.rareNonInheritedData->m_alignSelf)
             return true;
     }
@@ skipping 1121 matching lines @@
         horizontal || includeLogicalRightEdge);
 
     edges[BSLeft] = BorderEdge(borderLeftWidth(),
         visitedDependentColor(CSSPropertyBorderLeftColor),
         borderLeftStyle(),
         borderLeftIsTransparent(),
         !horizontal || includeLogicalLeftEdge);
 }
 
 } // namespace blink