RenderElement::styleWillChange() should pass newStyle as reference.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Oct 2013 12:00:20 +0000 (12:00 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Oct 2013 12:00:20 +0000 (12:00 +0000)
<https://webkit.org/b/123332>

When styleWillChange() is called, there is always a new style getting
set so there's no need to handle the null style case.
This flushed out a couple of unnecessary checks.

Reviewed by Antti Koivisto.

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

15 files changed:
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderBlockFlow.h
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderButton.cpp
Source/WebCore/rendering/RenderButton.h
Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp
Source/WebCore/rendering/RenderDeprecatedFlexibleBox.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderLayerModelObject.cpp
Source/WebCore/rendering/RenderLayerModelObject.h

index 61d31ce..957eb5e 100644 (file)
@@ -1,3 +1,14 @@
+2013-10-25  Andreas Kling  <akling@apple.com>
+
+        RenderElement::styleWillChange() should pass newStyle as reference.
+        <https://webkit.org/b/123332>
+
+        When styleWillChange() is called, there is always a new style getting
+        set so there's no need to handle the null style case.
+        This flushed out a couple of unnecessary checks.
+
+        Reviewed by Antti Koivisto.
+
 2013-10-25  peavo@outlook.com  <peavo@outlook.com>
 
         [WinCairo] Compile fixes.
index 50a006f..a6e7d00 100644 (file)
@@ -237,14 +237,14 @@ void RenderBlock::willBeDestroyed()
     RenderBox::willBeDestroyed();
 }
 
-void RenderBlock::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
+void RenderBlock::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     RenderStyle* oldStyle = style();
 
-    setReplaced(newStyle->isDisplayInlineType());
+    setReplaced(newStyle.isDisplayInlineType());
     
-    if (oldStyle && parent() && diff == StyleDifferenceLayout && oldStyle->position() != newStyle->position()) {
-        if (newStyle->position() == StaticPosition)
+    if (oldStyle && parent() && diff == StyleDifferenceLayout && oldStyle->position() != newStyle.position()) {
+        if (newStyle.position() == StaticPosition)
             // Clear our positioned objects list. Our absolutely positioned descendants will be
             // inserted into our containing block's positioned objects list during layout.
             removePositionedObjects(0, NewContainingBlock);
index ecacb04..467a2b3 100644 (file)
@@ -471,7 +471,7 @@ protected:
     void updateScrollInfoAfterLayout();
     void removeFromDelayedUpdateScrollInfoSet();
 
-    virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle) OVERRIDE;
+    virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle) OVERRIDE;
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) OVERRIDE;
 
     virtual bool hasLineIfEmpty() const;
index b4fe9bd..f196533 100644 (file)
@@ -1634,13 +1634,13 @@ void RenderBlockFlow::styleDidChange(StyleDifference diff, const RenderStyle* ol
         invalidateLineLayoutPath();
 }
 
-void RenderBlockFlow::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
+void RenderBlockFlow::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     RenderStyle* oldStyle = style();
     s_canPropagateFloatIntoSibling = oldStyle ? !isFloatingOrOutOfFlowPositioned() && !avoidsFloats() : false;
 
-    if (oldStyle && parent() && diff == StyleDifferenceLayout && oldStyle->position() != newStyle->position()) {
-        if (containsFloats() && !isFloating() && !isOutOfFlowPositioned() && newStyle->hasOutOfFlowPosition())
+    if (oldStyle && parent() && diff == StyleDifferenceLayout && oldStyle->position() != newStyle.position()) {
+        if (containsFloats() && !isFloating() && !isOutOfFlowPositioned() && newStyle.hasOutOfFlowPosition())
             markAllDescendantsWithFloatsForLayout();
     }
 
index a01d1bd..9ab6218 100644 (file)
@@ -366,7 +366,7 @@ protected:
     LayoutUnit applyAfterBreak(RenderBox& child, LayoutUnit logicalOffset, MarginInfo&); // If the child has an after break, then return a new offset that shifts to the top of the next page/column.
     LayoutUnit adjustBlockChildForPagination(LayoutUnit logicalTopAfterClear, LayoutUnit estimateWithoutPagination, RenderBox& child, bool atBeforeSideOfBlock);
 
-    virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle) OVERRIDE;
+    virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle) OVERRIDE;
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) OVERRIDE;
 
     void createFloatingObjects();
index 455c8b3..a709031 100644 (file)
@@ -264,7 +264,7 @@ void RenderBox::removeFloatingOrPositionedChildFromBlockLists()
         RenderBlock::removePositionedObject(*this);
 }
 
-void RenderBox::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
+void RenderBox::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     s_hadOverflowClip = hasOverflowClip();
 
@@ -275,23 +275,23 @@ void RenderBox::styleWillChange(StyleDifference diff, const RenderStyle* newStyl
         if (diff >= StyleDifferenceRepaint && (isRoot() || isBody())) {
             view().repaintRootContents();
 #if USE(ACCELERATED_COMPOSITING)
-            if (oldStyle->hasEntirelyFixedBackground() != newStyle->hasEntirelyFixedBackground())
+            if (oldStyle->hasEntirelyFixedBackground() != newStyle.hasEntirelyFixedBackground())
                 view().compositor().rootFixedBackgroundsChanged();
 #endif
         }
         
         // When a layout hint happens and an object's position style changes, we have to do a layout
         // to dirty the render tree using the old position value now.
-        if (diff == StyleDifferenceLayout && parent() && oldStyle->position() != newStyle->position()) {
+        if (diff == StyleDifferenceLayout && parent() && oldStyle->position() != newStyle.position()) {
             markContainingBlocksForLayout();
             if (oldStyle->position() == StaticPosition)
                 repaint();
-            else if (newStyle->hasOutOfFlowPosition())
+            else if (newStyle.hasOutOfFlowPosition())
                 parent()->setChildNeedsLayout();
-            if (isFloating() && !isOutOfFlowPositioned() && newStyle->hasOutOfFlowPosition())
+            if (isFloating() && !isOutOfFlowPositioned() && newStyle.hasOutOfFlowPosition())
                 removeFloatingOrPositionedChildFromBlockLists();
         }
-    } else if (newStyle && isBody())
+    } else if (isBody())
         view().repaintRootContents();
 
     RenderBoxModelObject::styleWillChange(diff, newStyle);
index 35af07e..97fd813 100644 (file)
@@ -606,7 +606,7 @@ protected:
 
     virtual void willBeDestroyed() OVERRIDE;
 
-    virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle) OVERRIDE;
+    virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle) OVERRIDE;
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) OVERRIDE;
     virtual void updateFromStyle() OVERRIDE;
 
index a688ed3..33068cd 100644 (file)
@@ -86,7 +86,7 @@ void RenderButton::removeChild(RenderObject& oldChild)
         m_inner->removeChild(oldChild);
 }
 
-void RenderButton::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
+void RenderButton::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     if (m_inner) {
         // RenderBlock::setStyle is going to apply a new style to the inner block, which
@@ -94,9 +94,9 @@ void RenderButton::styleWillChange(StyleDifference diff, const RenderStyle* newS
         // it right below. Here we change it back to 0 to avoid getting a spurious layout hint
         // because of the difference. Same goes for the other properties.
         // FIXME: Make this hack unnecessary.
-        m_inner->style()->setFlexGrow(newStyle->initialFlexGrow());
-        m_inner->style()->setMarginTop(newStyle->initialMargin());
-        m_inner->style()->setMarginBottom(newStyle->initialMargin());
+        m_inner->style()->setFlexGrow(newStyle.initialFlexGrow());
+        m_inner->style()->setMarginTop(newStyle.initialMargin());
+        m_inner->style()->setMarginBottom(newStyle.initialMargin());
     }
     RenderBlock::styleWillChange(diff, newStyle);
 }
index 2b27c34..1f3391c 100644 (file)
@@ -63,7 +63,7 @@ private:
     virtual const char* renderName() const OVERRIDE { return "RenderButton"; }
     virtual bool isRenderButton() const OVERRIDE { return true; }
 
-    virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle) OVERRIDE;
+    virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle) OVERRIDE;
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) OVERRIDE;
 
     virtual bool hasLineIfEmpty() const OVERRIDE;
index 53fb420..75e014b 100644 (file)
@@ -168,10 +168,10 @@ static LayoutUnit contentHeightForChild(RenderBox* child)
     return child->logicalHeight() - child->borderAndPaddingLogicalHeight();
 }
 
-void RenderDeprecatedFlexibleBox::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
+void RenderDeprecatedFlexibleBox::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     RenderStyle* oldStyle = style();
-    if (oldStyle && !oldStyle->lineClamp().isNone() && newStyle->lineClamp().isNone())
+    if (oldStyle && !oldStyle->lineClamp().isNone() && newStyle.lineClamp().isNone())
         clearLineClamp();
 
     RenderBlock::styleWillChange(diff, newStyle);
index b4dcf98..1d2001d 100644 (file)
@@ -38,7 +38,7 @@ public:
 
     virtual const char* renderName() const;
 
-    virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle) OVERRIDE;
+    virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle) OVERRIDE;
 
     virtual void layoutBlock(bool relayoutChildren, LayoutUnit pageHeight = 0);
     void layoutHorizontalBox(bool relayoutChildren);
index 6dee372..3a756a5 100644 (file)
@@ -368,7 +368,7 @@ void RenderElement::setStyle(PassRef<RenderStyle> style)
 
     diff = adjustStyleDifference(diff, contextSensitiveProperties);
 
-    styleWillChange(diff, &style.get());
+    styleWillChange(diff, style.get());
     
     RefPtr<RenderStyle> oldStyle = m_style.release();
     m_style = std::move(style);
@@ -793,54 +793,52 @@ static inline bool rendererHasBackground(const RenderElement* renderer)
     return renderer && renderer->hasBackground();
 }
 
-void RenderElement::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
+void RenderElement::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     if (m_style) {
         // If our z-index changes value or our visibility changes,
         // we need to dirty our stacking context's z-order list.
-        if (newStyle) {
-            bool visibilityChanged = m_style->visibility() != newStyle->visibility() 
-                || m_style->zIndex() != newStyle->zIndex() 
-                || m_style->hasAutoZIndex() != newStyle->hasAutoZIndex();
+        bool visibilityChanged = m_style->visibility() != newStyle.visibility()
+            || m_style->zIndex() != newStyle.zIndex()
+            || m_style->hasAutoZIndex() != newStyle.hasAutoZIndex();
 #if ENABLE(DASHBOARD_SUPPORT) || ENABLE(DRAGGABLE_REGION)
-            if (visibilityChanged)
-                document().setAnnotatedRegionsDirty(true);
+        if (visibilityChanged)
+            document().setAnnotatedRegionsDirty(true);
 #endif
-            if (visibilityChanged) {
-                if (AXObjectCache* cache = document().existingAXObjectCache())
-                    cache->childrenChanged(parent());
-            }
+        if (visibilityChanged) {
+            if (AXObjectCache* cache = document().existingAXObjectCache())
+                cache->childrenChanged(parent());
+        }
 
-            // Keep layer hierarchy visibility bits up to date if visibility changes.
-            if (m_style->visibility() != newStyle->visibility()) {
-                if (RenderLayer* layer = enclosingLayer()) {
-                    if (newStyle->visibility() == VISIBLE)
-                        layer->setHasVisibleContent();
-                    else if (layer->hasVisibleContent() && (this == &layer->renderer() || layer->renderer().style()->visibility() != VISIBLE)) {
-                        layer->dirtyVisibleContentStatus();
-                        if (diff > StyleDifferenceRepaintLayer)
-                            repaint();
-                    }
+        // Keep layer hierarchy visibility bits up to date if visibility changes.
+        if (m_style->visibility() != newStyle.visibility()) {
+            if (RenderLayer* layer = enclosingLayer()) {
+                if (newStyle.visibility() == VISIBLE)
+                    layer->setHasVisibleContent();
+                else if (layer->hasVisibleContent() && (this == &layer->renderer() || layer->renderer().style()->visibility() != VISIBLE)) {
+                    layer->dirtyVisibleContentStatus();
+                    if (diff > StyleDifferenceRepaintLayer)
+                        repaint();
                 }
             }
         }
 
-        if (m_parent && (newStyle->outlineSize() < m_style->outlineSize() || shouldRepaintForStyleDifference(diff)))
+        if (m_parent && (newStyle.outlineSize() < m_style->outlineSize() || shouldRepaintForStyleDifference(diff)))
             repaint();
-        if (isFloating() && (m_style->floating() != newStyle->floating()))
+        if (isFloating() && (m_style->floating() != newStyle.floating()))
             // For changes in float styles, we need to conceivably remove ourselves
             // from the floating objects list.
             toRenderBox(this)->removeFloatingOrPositionedChildFromBlockLists();
-        else if (isOutOfFlowPositioned() && (m_style->position() != newStyle->position()))
+        else if (isOutOfFlowPositioned() && (m_style->position() != newStyle.position()))
             // For changes in positioning styles, we need to conceivably remove ourselves
             // from the positioned objects list.
             toRenderBox(this)->removeFloatingOrPositionedChildFromBlockLists();
 
         s_affectsParentBlock = isFloatingOrOutOfFlowPositioned()
-            && (!newStyle->isFloating() && !newStyle->hasOutOfFlowPosition())
+            && (!newStyle.isFloating() && !newStyle.hasOutOfFlowPosition())
             && parent() && (parent()->isRenderBlockFlow() || parent()->isRenderInline());
 
-        s_noLongerAffectsParentBlock = ((!isFloating() && newStyle->isFloating()) || (!isOutOfFlowPositioned() && newStyle->hasOutOfFlowPosition()))
+        s_noLongerAffectsParentBlock = ((!isFloating() && newStyle.isFloating()) || (!isOutOfFlowPositioned() && newStyle.hasOutOfFlowPosition()))
             && parent() && parent()->isRenderBlock();
 
         // reset style flags
@@ -860,14 +858,14 @@ void RenderElement::styleWillChange(StyleDifference diff, const RenderStyle* new
 
     bool repaintFixedBackgroundsOnScroll = shouldRepaintFixedBackgroundsOnScroll();
 
-    bool newStyleSlowScroll = newStyle && repaintFixedBackgroundsOnScroll && newStyle->hasFixedBackgroundImage();
+    bool newStyleSlowScroll = repaintFixedBackgroundsOnScroll && newStyle.hasFixedBackgroundImage();
     bool oldStyleSlowScroll = m_style && repaintFixedBackgroundsOnScroll && m_style->hasFixedBackgroundImage();
 
 #if USE(ACCELERATED_COMPOSITING)
     bool drawsRootBackground = isRoot() || (isBody() && !rendererHasBackground(document().documentElement()->renderer()));
     if (drawsRootBackground && repaintFixedBackgroundsOnScroll) {
         if (view().compositor().supportsFixedRootBackgroundCompositing()) {
-            if (newStyleSlowScroll && newStyle->hasEntirelyFixedBackground())
+            if (newStyleSlowScroll && newStyle.hasEntirelyFixedBackground())
                 newStyleSlowScroll = false;
 
             if (oldStyleSlowScroll && m_style->hasEntirelyFixedBackground())
index db4b545..628ea00 100644 (file)
@@ -131,8 +131,8 @@ protected:
     void setLastChild(RenderObject* child) { m_lastChild = child; }
     void destroyLeftoverChildren();
 
-    virtual void styleWillChange(StyleDifference, const RenderStyle*);
-    virtual void styleDidChange(StyleDifference, const RenderStyle*);
+    virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle);
+    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
 
     virtual void insertedIntoTree() OVERRIDE;
     virtual void willBeRemovedFromTree() OVERRIDE;
index 3790edc..aa6fb98 100644 (file)
@@ -85,7 +85,7 @@ void RenderLayerModelObject::willBeDestroyed()
     RenderElement::willBeDestroyed();
 }
 
-void RenderLayerModelObject::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
+void RenderLayerModelObject::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     s_wasFloating = isFloating();
     s_hadLayer = hasLayer();
@@ -96,15 +96,15 @@ void RenderLayerModelObject::styleWillChange(StyleDifference diff, const RenderS
     // If our z-index changes value or our visibility changes,
     // we need to dirty our stacking context's z-order list.
     RenderStyle* oldStyle = style();
-    if (oldStyle && newStyle) {
+    if (oldStyle) {
         if (parent()) {
             // Do a repaint with the old style first, e.g., for example if we go from
             // having an outline to not having an outline.
             if (diff == StyleDifferenceRepaintLayer) {
                 layer()->repaintIncludingDescendants();
-                if (!(oldStyle->clip() == newStyle->clip()))
+                if (!(oldStyle->clip() == newStyle.clip()))
                     layer()->clearClipRectsIncludingDescendants();
-            } else if (diff == StyleDifferenceRepaint || newStyle->outlineSize() < oldStyle->outlineSize())
+            } else if (diff == StyleDifferenceRepaint || newStyle.outlineSize() < oldStyle->outlineSize())
                 repaint();
         }
 
@@ -112,19 +112,19 @@ void RenderLayerModelObject::styleWillChange(StyleDifference diff, const RenderS
             // When a layout hint happens, we go ahead and do a repaint of the layer, since the layer could
             // end up being destroyed.
             if (hasLayer()) {
-                if (oldStyle->position() != newStyle->position()
-                    || oldStyle->zIndex() != newStyle->zIndex()
-                    || oldStyle->hasAutoZIndex() != newStyle->hasAutoZIndex()
-                    || !(oldStyle->clip() == newStyle->clip())
-                    || oldStyle->hasClip() != newStyle->hasClip()
-                    || oldStyle->opacity() != newStyle->opacity()
-                    || oldStyle->transform() != newStyle->transform()
+                if (oldStyle->position() != newStyle.position()
+                    || oldStyle->zIndex() != newStyle.zIndex()
+                    || oldStyle->hasAutoZIndex() != newStyle.hasAutoZIndex()
+                    || !(oldStyle->clip() == newStyle.clip())
+                    || oldStyle->hasClip() != newStyle.hasClip()
+                    || oldStyle->opacity() != newStyle.opacity()
+                    || oldStyle->transform() != newStyle.transform()
 #if ENABLE(CSS_FILTERS)
-                    || oldStyle->filter() != newStyle->filter()
+                    || oldStyle->filter() != newStyle.filter()
 #endif
                     )
                 layer()->repaintIncludingDescendants();
-            } else if (newStyle->hasTransform() || newStyle->opacity() < 1 || newStyle->hasFilter()) {
+            } else if (newStyle.hasTransform() || newStyle.opacity() < 1 || newStyle.hasFilter()) {
                 // If we don't have a layer yet, but we are going to get one because of transform or opacity,
                 //  then we need to repaint the old position of the object.
                 repaint();
index aff588f..9ac606f 100644 (file)
@@ -39,7 +39,7 @@ public:
     bool hasSelfPaintingLayer() const;
     RenderLayer* layer() const { return m_layer.get(); }
 
-    virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle) OVERRIDE;
+    virtual void styleWillChange(StyleDifference, const RenderStyle& newStyle) OVERRIDE;
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) OVERRIDE;
     virtual void updateFromStyle() { }