Revert of Separate repaint and layout requirements of StyleDifference (Step 1)

Revert of Separate repaint and layout requirements of StyleDifference (Step 1) (https://codereview.chromium.org/236203020/)

Reason for revert:
Reason for revert:
Depends on https://codereview.chromium.org/203463007/ which broke ChromeOS login window: https://code.google.com/p/chromium/issues/detail?id=365507

Original issue's description:
> Separate repaint and layout requirements of StyleDifference (Step 1)
> 
> Previously StyleDifference was an enum that proximately bigger values
> imply smaller values (e.g. StyleDifferenceLayout implies
> StyleDifferenceRepaint). This causes unnecessary repaints in some cases
> on layout change.
> 
> Convert StyleDifference to a structure containing relatively independent
> flags.
> 
> This change doesn't directly improve the result, but can make further
> repaint optimizations possible.
> 
> Step 1 doesn't change any functionality. RenderStyle still generate the
> legacy StyleDifference enum when comparing styles and convert the result
> to the new StyleDifference. Implicit requirements are not handled during
> the conversion.
> 
> Converted call sites to use the new StyleDifference according to the
> following conversion rules:
> - diff == StyleDifferenceEqual (&& !context) => diff.hasNoChange()
> - diff == StyleDifferenceRepaint => diff.needsRepaintObjectOnly()
> - diff == StyleDifferenceRepaintLayer => diff.needsRepaintLayer()
> - diff == StyleDifferenceRepaint || diff == StyleDifferenceRepaintLayer => diff.needsRepaintLayer()
> - diff >= StyleDifferenceRepaint => diff.needsRepaint() || diff.needsLayout()
> - diff >= StyleDifferenceRepaintLayer => diff.needsRepaintLayer() || diff.needsLayout()
> - diff > StyleDifferenceRepaintLayer => diff.needsLayout()
> - diff == StyleDifferencePositionedMovementLayoutOnly => diff.needsPositionedMovementLayoutOnly()
> - diff == StyleDifferenceLayout => diff.needsFullLayout()
> 
> BUG=358460
> TEST=All existing layout tests.
> R=eseidel@chromium.org, esprehn@chromium.org, jchaffraix@chromium.org
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171983

TBR=esprehn@chromium.org,jchaffraix@chromium.org,ojan@chromium.org,eseidel@chromium.org,wangxianzhu@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=358460
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=172173

[aboxhall, 2014-04-22 20:14:42]

Created Revert of Separate repaint and layout requirements of StyleDifference
(Step 1)

[commit-bot: I haz the power, 2014-04-22 20:15:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aboxhall@chromium.org/247583003/1

[commit-bot: I haz the power, 2014-04-22 20:15:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-22 20:15:35]

Failed to apply patch for Source/core/rendering/RenderObject.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/rendering/RenderObject.cpp
  Hunk #1 FAILED at 1856.
  Hunk #2 succeeded at 1967 (offset 1 line).
  Hunk #3 succeeded at 2002 (offset 1 line).
  Hunk #4 succeeded at 2016 (offset 1 line).
  Hunk #5 succeeded at 2050 (offset 1 line).
  Hunk #6 succeeded at 2073 (offset 1 line).
  Hunk #7 succeeded at 2146 (offset 1 line).
  Hunk #8 succeeded at 2161 (offset 1 line).
  1 out of 8 hunks FAILED -- saving rejects to file
Source/core/rendering/RenderObject.cpp.rej

Patch:       Source/core/rendering/RenderObject.cpp
Index: Source/core/rendering/RenderObject.cpp
diff --git a/Source/core/rendering/RenderObject.cpp
b/Source/core/rendering/RenderObject.cpp
index
04b0ea30c5637bf7b9dfd7855cf90fc73d273b7e..63346d4f89457c81cfbbebc152ad939841139841
100644
--- a/Source/core/rendering/RenderObject.cpp
+++ b/Source/core/rendering/RenderObject.cpp
@@ -1856,53 +1856,51 @@
     DisableCompositingQueryAsserts disabler;
 
     if (contextSensitiveProperties & ContextSensitivePropertyTransform &&
isSVG())
-        diff.setNeedsFullLayout();
+        diff = StyleDifferenceLayout;
 
     // If transform changed, and the layer does not paint into its own separate
backing, then we need to repaint.
-    if (contextSensitiveProperties & ContextSensitivePropertyTransform &&
!diff.needsLayout()) {
+    if (contextSensitiveProperties & ContextSensitivePropertyTransform  && diff
<= StyleDifferenceRepaintLayer) {
         // Text nodes share style with their parents but transforms don't apply
to them,
         // hence the !isText() check.
         if (!isText() && (!hasLayer() ||
!toRenderLayerModelObject(this)->layer()->hasDirectReasonsForCompositing()))
-            diff.setNeedsRepaintLayer();
-        else
-            diff.setNeedsRecompositeLayer();
+            diff = StyleDifferenceRepaintLayer;
+        else if (diff < StyleDifferenceRecompositeLayer)
+            diff = StyleDifferenceRecompositeLayer;
     }
 
     // If opacity or filters changed, and the layer does not paint into its own
separate backing, then we need to repaint (also
     // ignoring text nodes)
-    if (contextSensitiveProperties & ContextSensitivePropertyOpacity &&
!diff.needsLayout()) {
+    if (contextSensitiveProperties & ContextSensitivePropertyOpacity && diff <=
StyleDifferenceRepaintLayer) {
         if (!isText() && (!hasLayer() ||
!toRenderLayerModelObject(this)->layer()->hasDirectReasonsForCompositing()))
-            diff.setNeedsRepaintLayer();
-        else
-            diff.setNeedsRecompositeLayer();
-    }
-
-    if ((contextSensitiveProperties & ContextSensitivePropertyFilter) &&
hasLayer() && !diff.needsLayout()) {
+            diff = StyleDifferenceRepaintLayer;
+        else if (diff < StyleDifferenceRecompositeLayer)
+            diff = StyleDifferenceRecompositeLayer;
+    }
+
+    if ((contextSensitiveProperties & ContextSensitivePropertyFilter) &&
hasLayer() && diff <= StyleDifferenceRepaintLayer) {
         RenderLayer* layer = toRenderLayerModelObject(this)->layer();
         if (!layer->hasDirectReasonsForCompositing() ||
layer->paintsWithFilters())
-            diff.setNeedsRepaintLayer();
-        else
-            diff.setNeedsRecompositeLayer();
-    }
-
-    if ((contextSensitiveProperties & ContextSensitivePropertyTextOrColor) &&
!diff.needsRepaint() && !diff.needsLayout()
+            diff = StyleDifferenceRepaintLayer;
+        else if (diff < StyleDifferenceRecompositeLayer)
+            diff = StyleDifferenceRecompositeLayer;
+    }
+
+    if ((contextSensitiveProperties & ContextSensitivePropertyTextOrColor) &&
diff < StyleDifferenceRepaint
         && hasImmediateNonWhitespaceTextChildOrPropertiesDependentOnColor())
-        diff.setNeedsRepaintObject();
+        diff = StyleDifferenceRepaint;
 
     // The answer to layerTypeRequired() for plugins, iframes, and canvas can
change without the actual
     // style changing, since it depends on whether we decide to composite these
elements. When the
     // layer status of one of these elements changes, we need to force a
layout.
-    if (diff.hasNoChange() && style() && isLayerModelObject()) {
+    if (diff == StyleDifferenceEqual && style() && isLayerModelObject()) {
         bool requiresLayer =
toRenderLayerModelObject(this)->layerTypeRequired() != NoLayer;
         if (hasLayer() != requiresLayer)
-            diff.setNeedsFullLayout();
+            diff = StyleDifferenceLayout;
     }
 
     // If we have no layer(), just treat a RepaintLayer hint as a normal
Repaint.
-    if (diff.needsRepaintLayer() && !hasLayer()) {
-        diff.clearNeedsRepaint();
-        diff.setNeedsRepaintObject();
-    }
+    if (diff == StyleDifferenceRepaintLayer && !hasLayer())
+        diff = StyleDifferenceRepaint;
 
     return diff;
 }
@@ -1966,7 +1964,7 @@
         return;
     }
 
-    StyleDifference diff;
+    StyleDifference diff = StyleDifferenceEqual;
     unsigned contextSensitiveProperties = ContextSensitivePropertyNone;
     if (m_style)
         diff = m_style->visualInvalidationDiff(*style,
contextSensitiveProperties);
@@ -2001,10 +1999,10 @@
     // check whether we should layout now, and decide if we need to repaint.
     StyleDifference updatedDiff = adjustStyleDifference(diff,
contextSensitiveProperties);
 
-    if (!diff.needsFullLayout()) {
-        if (updatedDiff.needsFullLayout())
+    if (diff <= StyleDifferenceLayoutPositionedMovementOnly) {
+        if (updatedDiff == StyleDifferenceLayout)
             setNeedsLayoutAndPrefWidthsRecalc();
-        else if (updatedDiff.needsPositionedMovementLayoutOnly())
+        else if (updatedDiff == StyleDifferenceLayoutPositionedMovementOnly)
             setNeedsPositionedMovementLayout();
     }
 
@@ -2015,7 +2013,7 @@
             toRenderBox(this)->updateLayerTransform();
     }
 
-    if (updatedDiff.needsRepaint()) {
+    if (updatedDiff == StyleDifferenceRepaint || updatedDiff ==
StyleDifferenceRepaintLayer) {
         // Do a repaint with the new style now, e.g., for example if we go from
         // not having an outline to having an outline.
         repaint();
@@ -2049,13 +2047,13 @@
                     layer->setHasVisibleContent();
                 } else if (layer->hasVisibleContent() && (this ==
layer->renderer() || layer->renderer()->style()->visibility() != VISIBLE)) {
                     layer->dirtyVisibleContentStatus();
-                    if (diff.needsLayout())
+                    if (diff > StyleDifferenceRepaintLayer)
                         repaint();
                 }
             }
         }
 
-        if (m_parent && diff.needsRepaintObjectOnly())
+        if (m_parent && diff == StyleDifferenceRepaint)
             repaint();
         if (isFloating() && (m_style->floating() != newStyle.floating()))
             // For changes in float styles, we need to conceivably remove
ourselves
@@ -2072,13 +2070,12 @@
 
         // Clearing these bits is required to avoid leaving stale renderers.
         // FIXME: We shouldn't need that hack if our logic was totally correct.
-        if (diff.needsFullLayout() || diff.needsPositionedMovementLayoutOnly())
{
+        if (diff == StyleDifferenceLayout || diff ==
StyleDifferenceLayoutPositionedMovementOnly) {
             setFloating(false);
             clearPositionedState();
         }
-    } else {
+    } else
         s_affectsParentBlock = false;
-    }
 
     if (view()->frameView()) {
         bool shouldBlitOnFixedBackgroundImage = false;
@@ -2146,7 +2143,7 @@
     if (!m_parent)
         return;
 
-    if (diff.needsFullLayout()) {
+    if (diff == StyleDifferenceLayout) {
         RenderCounter::rendererStyleChanged(*this, oldStyle, m_style.get());
 
         // If the object already needs layout, then setNeedsLayout won't do
@@ -2161,10 +2158,10 @@
         if (needsOverflowRecalcAfterStyleChange() && oldStyle->position() !=
m_style->position())
             markContainingBlocksForOverflowRecalc();
 
-        setNeedsLayoutAndPrefWidthsRecalc();
-    } else if (diff.needsPositionedMovementLayoutOnly()) {
+        if (diff == StyleDifferenceLayout)
+            setNeedsLayoutAndPrefWidthsRecalc();
+    } else if (diff == StyleDifferenceLayoutPositionedMovementOnly)
         setNeedsPositionedMovementLayout();
-    }
 
     // Don't check for repaint here; we need to wait until the layer has been
     // updated by subclasses before we know if we have to repaint (in
setStyle()).

[aboxhall, 2014-04-22 21:01:05]

The CQ bit was checked by aboxhall@chromium.org

[commit-bot: I haz the power, 2014-04-22 21:02:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aboxhall@chromium.org/247583003/150001

[commit-bot: I haz the power, 2014-04-22 21:04:17]

Change committed as 172173