Don't force full invalidation on padding or margin change

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 404 matching lines @@
 }
 
 bool RenderStyle::diffNeedsFullLayoutAndRepaint(const RenderStyle& other) const
 {
     // FIXME: Not all cases in this method need both full layout and repaint.
     // Should move cases into diffNeedsFullLayout() if
     // - don't need repaint at all;
     // - or the renderer knows how to exactly repaint caused by the layout change
     //   instead of forced full repaint.
 
     if (surround.get() != other.surround.get()) {

[Julien - ping for review, 2014/06/29 01:20:32]

There is only border checks in this branch so I would just remove this check.

[Xianzhu, 2014/06/30 22:30:34]

If 'surround' object is shared between the old style and the new style, the
above check will save 2 to 8 border width getters (which contain logic) and 1 to
4 border width comparisons.

-        if (surround->margin != other.surround->margin)
-            return true;
-
-        if (surround->padding != other.surround->padding)
-            return true;
-
         // If our border widths change, then we need to layout. Other changes to borders only necessitate a repaint.
         if (borderLeftWidth() != other.borderLeftWidth()
             || borderTopWidth() != other.borderTopWidth()
             || borderBottomWidth() != other.borderBottomWidth()
             || borderRightWidth() != other.borderRightWidth())
             return true;
     }
 
     if (rareNonInheritedData.get() != other.rareNonInheritedData.get()) {
         if (rareNonInheritedData->m_appearance != other.rareNonInheritedData->m_appearance
@@ skipping 167 matching lines @@
         if (m_box->verticalAlign() != other.m_box->verticalAlign())
             return true;
 
         if (m_box->boxSizing() != other.m_box->boxSizing())
             return true;
     }
 
     if (noninherited_flags._vertical_align != other.noninherited_flags._vertical_align)
         return true;
 
+    if (surround.get() != other.surround.get()) {

[Julien - ping for review, 2014/06/29 01:20:32]

Will this work correctly if we have a background-clip: padding-box (or anything
bigger) and we resize the box without changing the overall size (by shrinking
the padding but increasing border so that the content box and border box are the
same)?

[Xianzhu, 2014/06/30 22:30:34]

Yes. Border size change will still trigger a full repaint.

Added a test case.

+        if (surround->margin != other.surround->margin)
+            return true;
+
+        if (surround->padding != other.surround->padding)
+            return true;
+    }
+
     return false;
 }
 
 bool RenderStyle::diffNeedsRepaintLayer(const RenderStyle& other) const
 {
     if (position() != StaticPosition && (visual->clip != other.visual->clip || visual->hasClip != other.visual->hasClip))
         return true;
 
     if (rareNonInheritedData.get() != other.rareNonInheritedData.get()) {
         if (RuntimeEnabledFeatures::cssCompositingEnabled()
@@ skipping 1078 matching lines @@
     // right
     radiiSum = radii.topRight().height() + radii.bottomRight().height();
     if (radiiSum > rect.height())
         factor = std::min(rect.height() / radiiSum, factor);
 
     ASSERT(factor <= 1);
     return factor;
 }
 
 } // namespace WebCore