Don't always repaint on zIndex 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 582 matching lines @@
         return StyleDifferenceLayout;
     }
 
     StyleDifference repaintDifference = repaintOnlyDiff(other, changedContextSensitiveProperties);
     ASSERT(repaintDifference <= StyleDifferenceRepaintLayer);
     return repaintDifference;
 }
 
 StyleDifference RenderStyle::repaintOnlyDiff(const RenderStyle& other, unsigned& changedContextSensitiveProperties) const
 {
-    if (position() != StaticPosition && (m_box->zIndex() != other.m_box->zIndex() || m_box->hasAutoZIndex() != other.m_box->hasAutoZIndex()
-        || visual->clip != other.visual->clip || visual->hasClip != other.visual->hasClip))
-        return StyleDifferenceRepaintLayer;
+    if (position() != StaticPosition) {

[ojan, 2014/04/18 01:46:23]

This isn't the fault of your patch, but now that I look at this code, it isn't
quite right. There are other conditions where zIndex applies than just the
position. See the following code in StyleAdjuster::adjustRenderStyle:
    if (style->position() == StaticPosition &&
!parentStyleForcesZIndexToCreateStackingContext(parentStyle))
        style->setHasAutoZIndex();

I don't think fixing this is very high priority, but mind putting a FIXME in
this code while you're here?

[Xianzhu, 2014/04/18 16:05:41]

My understanding about the code is that it forces autoZIndex when non-auto
zIndex is not applicable. Auto noticed that in other cases zIndex will be forced
to 0 in StyleAdjuster.cpp. So StyleAdjuster seems to already handle the
applicable conditions of ZIndex. I'm wondering if it'd be correct to just remove
the condition at line 603 for zIndex.

[ojan, 2014/04/18 17:20:03]

I think you're right. In either case, that should be a separate patch with a
test-case to go with it. So, a FIXME for now?

+        if (m_box->zIndex() != other.m_box->zIndex() || m_box->hasAutoZIndex() != other.m_box->hasAutoZIndex())
+            changedContextSensitiveProperties |= ContextSensitivePropertyZIndex;
+        if (visual->clip != other.visual->clip || visual->hasClip != other.visual->hasClip)
+            return StyleDifferenceRepaintLayer;
+    }
 
     if (RuntimeEnabledFeatures::cssCompositingEnabled() && (rareNonInheritedData->m_effectiveBlendMode != other.rareNonInheritedData->m_effectiveBlendMode
         || rareNonInheritedData->m_isolation != other.rareNonInheritedData->m_isolation))
         return StyleDifferenceRepaintLayer;
 
     if (rareNonInheritedData->opacity != other.rareNonInheritedData->opacity) {
         // Don't return early here; instead take note of the type of change,
         // and deal with it when looking at compositing.
         changedContextSensitiveProperties |= ContextSensitivePropertyOpacity;
     }
@@ skipping 1039 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