Clarify RenderElement::adjustStyleDifference()

https://bugs.webkit.org/show_bug.cgi?id=142256

Reviewed by David Hyatt.

Make RenderElement::adjustStyleDifference() clearer in two ways.

First, replace lots of if (diff < X) diff = X with
diff = std::max(diff, X). I did this even in cases where diff was
being set unconditionally, because it's never correct to change the diff
to a lesser value.

Second the "set at least SimplifiedLayout, but if we have PositionedMovementOnly
set it to SimplifiedLayoutAndPositionedMovement" was confusingly written.

* rendering/RenderElement.cpp:
(WebCore::RenderElement::adjustStyleDifference):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@181069 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 39434df..f4bbf72 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2015-03-04  Simon Fraser  <simon.fraser@apple.com>
+
+        Clarify RenderElement::adjustStyleDifference()
+        https://bugs.webkit.org/show_bug.cgi?id=142256
+
+        Reviewed by David Hyatt.
+
+        Make RenderElement::adjustStyleDifference() clearer in two ways.
+        
+        First, replace lots of if (diff < X) diff = X with
+        diff = std::max(diff, X). I did this even in cases where diff was
+        being set unconditionally, because it's never correct to change the diff
+        to a lesser value.
+        
+        Second the "set at least SimplifiedLayout, but if we have PositionedMovementOnly
+        set it to SimplifiedLayoutAndPositionedMovement" was confusingly written.
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::adjustStyleDifference):
+
 2015-03-04  David Kilzer  <ddkilzer@apple.com>
 
         Switch new soft-linking debug asserts to release asserts

diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
index 5b2f96d..0f9f6d2 100644 (file)
--- a/Source/WebCore/rendering/RenderElement.cpp
+++ b/Source/WebCore/rendering/RenderElement.cpp
@@ -263,37 +263,35 @@ StyleDifference RenderElement::adjustStyleDifference(StyleDifference diff, unsig
 {
     // If transform changed, and we are not composited, need to do a layout.
     if (contextSensitiveProperties & ContextSensitivePropertyTransform) {
-        // Text nodes share style with their parents but transforms don't apply to them,
-        // hence the !isText() check.
         // FIXME: when transforms are taken into account for overflow, we will need to do a layout.
         if (!hasLayer() || !downcast<RenderLayerModelObject>(*this).layer()->isComposited()) {
-            // We need to set at least SimplifiedLayout, but if PositionedMovementOnly is already set
-            // then we actually need SimplifiedLayoutAndPositionedMovement.
             if (!hasLayer())
-                diff = StyleDifferenceLayout; // FIXME: Do this for now since SimplifiedLayout cannot handle updating floating objects lists.
-            else if (diff < StyleDifferenceLayoutPositionedMovementOnly)
-                diff = StyleDifferenceSimplifiedLayout;
-            else if (diff < StyleDifferenceSimplifiedLayout)
-                diff = StyleDifferenceSimplifiedLayoutAndPositionedMovement;
-        } else if (diff < StyleDifferenceRecompositeLayer)
-            diff = StyleDifferenceRecompositeLayer;
+                diff = std::max(diff, StyleDifferenceLayout);
+            else {
+                // We need to set at least SimplifiedLayout, but if PositionedMovementOnly is already set
+                // then we actually need SimplifiedLayoutAndPositionedMovement.
+                diff = std::max(diff, (diff == StyleDifferenceLayoutPositionedMovementOnly) ? StyleDifferenceSimplifiedLayoutAndPositionedMovement : StyleDifferenceSimplifiedLayout);
+            }
+        
+        } else
+            diff = std::max(diff, StyleDifferenceRecompositeLayer);
     }
 
     // If opacity changed, and we are not composited, need to repaint (also
     // ignoring text nodes)
     if (contextSensitiveProperties & ContextSensitivePropertyOpacity) {
         if (!hasLayer() || !downcast<RenderLayerModelObject>(*this).layer()->isComposited())
-            diff = StyleDifferenceRepaintLayer;
-        else if (diff < StyleDifferenceRecompositeLayer)
-            diff = StyleDifferenceRecompositeLayer;
+            diff = std::max(diff, StyleDifferenceRepaintLayer);
+        else
+            diff = std::max(diff, StyleDifferenceRecompositeLayer);
     }
 
     if ((contextSensitiveProperties & ContextSensitivePropertyFilter) && hasLayer()) {
         RenderLayer* layer = downcast<RenderLayerModelObject>(*this).layer();
         if (!layer->isComposited() || layer->paintsWithFilters())
-            diff = StyleDifferenceRepaintLayer;
-        else if (diff < StyleDifferenceRecompositeLayer)
-            diff = StyleDifferenceRecompositeLayer;
+            diff = std::max(diff, StyleDifferenceRepaintLayer);
+        else
+            diff = std::max(diff, StyleDifferenceRecompositeLayer);
     }
     
     // The answer to requiresLayer() for plugins, iframes, and canvas can change without the actual