Use unique_ptr for FillLayer::m_next
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Apr 2014 04:28:25 +0000 (04:28 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Apr 2014 04:28:25 +0000 (04:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=75222

Reviewed by Dan Bernstein.

* css/DeprecatedStyleBuilder.cpp:
(WebCore::ApplyPropertyFillLayer::applyInheritValue):
Renamed currChild to just child and prevChild to previousChild.
Changed code to pass ownership of the new FillLayer immediately.
Changed some loops to be for loops.
(WebCore::ApplyPropertyFillLayer::applyInitialValue): Ditto.
(WebCore::ApplyPropertyFillLayer::applyValue): Ditto.

* rendering/RenderBox.cpp:
(WebCore::RenderBox::backgroundHasOpaqueTopLayer): Use reference
instead of pointer.
(WebCore::RenderBox::paintFillLayers): Ditto.
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::paintFillLayerExtended): Ditto.

* rendering/style/FillLayer.cpp:
(WebCore::FillLayer::FillLayer): Removed m_next initializer since it is now an
OwnPtr and initializes automatically. In a couple other places, changed m_next
initializer to use make_unique.
(WebCore::FillLayer::~FillLayer): Wrote loop for deletion of m_next.
(WebCore::FillLayer::operator=): Removed unneeded explicit deletion of m_next.
(WebCore::FillLayer::cullEmptyLayers): Ditto.
(WebCore::clipMax): Marked inline.
(WebCore::FillLayer::computeClipMax): Rewrote to use a loop instead of recursion.
(WebCore::FillLayer::containsImage): Ditto.
(WebCore::FillLayer::imagesAreLoaded): Ditto.
(WebCore::FillLayer::hasOpaqueImage): Rewrote to use && instead of multiple if.
(WebCore::FillLayer::hasImage): Rewrote to use a loop instead of recursion.
(WebCore::FillLayer::hasFixedImage): Ditto.

* rendering/style/FillLayer.h: Changed m_next to be a unique_ptr.

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

Source/WebCore/ChangeLog
Source/WebCore/css/DeprecatedStyleBuilder.cpp
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/style/FillLayer.cpp
Source/WebCore/rendering/style/FillLayer.h

index 6642b9e..3bd6d46 100644 (file)
@@ -1,3 +1,42 @@
+2014-04-12  Darin Adler  <darin@apple.com>
+
+        Use unique_ptr for FillLayer::m_next
+        https://bugs.webkit.org/show_bug.cgi?id=75222
+
+        Reviewed by Dan Bernstein.
+
+        * css/DeprecatedStyleBuilder.cpp:
+        (WebCore::ApplyPropertyFillLayer::applyInheritValue):
+        Renamed currChild to just child and prevChild to previousChild.
+        Changed code to pass ownership of the new FillLayer immediately.
+        Changed some loops to be for loops.
+        (WebCore::ApplyPropertyFillLayer::applyInitialValue): Ditto.
+        (WebCore::ApplyPropertyFillLayer::applyValue): Ditto.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::backgroundHasOpaqueTopLayer): Use reference
+        instead of pointer.
+        (WebCore::RenderBox::paintFillLayers): Ditto.
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::paintFillLayerExtended): Ditto.
+
+        * rendering/style/FillLayer.cpp:
+        (WebCore::FillLayer::FillLayer): Removed m_next initializer since it is now an
+        OwnPtr and initializes automatically. In a couple other places, changed m_next
+        initializer to use make_unique.
+        (WebCore::FillLayer::~FillLayer): Wrote loop for deletion of m_next.
+        (WebCore::FillLayer::operator=): Removed unneeded explicit deletion of m_next.
+        (WebCore::FillLayer::cullEmptyLayers): Ditto.
+        (WebCore::clipMax): Marked inline.
+        (WebCore::FillLayer::computeClipMax): Rewrote to use a loop instead of recursion.
+        (WebCore::FillLayer::containsImage): Ditto.
+        (WebCore::FillLayer::imagesAreLoaded): Ditto.
+        (WebCore::FillLayer::hasOpaqueImage): Rewrote to use && instead of multiple if.
+        (WebCore::FillLayer::hasImage): Rewrote to use a loop instead of recursion.
+        (WebCore::FillLayer::hasFixedImage): Ditto.
+
+        * rendering/style/FillLayer.h: Changed m_next to be a unique_ptr.
+
 2014-04-13  Andy Estes  <aestes@apple.com>
 
         [QuickLook] Move file system-related code into WebKit
index 954b159..0ac39d1 100644 (file)
@@ -520,72 +520,60 @@ public:
         if (*(styleResolver->parentStyle()->*layersFunction)() == *(styleResolver->style()->*layersFunction)())
             return;
 
-        FillLayer* currChild = (styleResolver->style()->*accessLayersFunction)();
-        FillLayer* prevChild = 0;
-        const FillLayer* currParent = (styleResolver->parentStyle()->*layersFunction)();
-        while (currParent && (currParent->*testFunction)()) {
-            if (!currChild) {
-                /* Need to make a new layer.*/
-                currChild = new FillLayer(fillLayerType);
-                prevChild->setNext(currChild);
+        auto* child = (styleResolver->style()->*accessLayersFunction)();
+        FillLayer* previousChild = nullptr;
+        for (auto* parent = (styleResolver->parentStyle()->*layersFunction)(); parent && (parent->*testFunction)(); parent = parent->next()) {
+            if (!child) {
+                previousChild->setNext(std::make_unique<FillLayer>(fillLayerType));
+                child = previousChild->next();
             }
-            (currChild->*setFunction)((currParent->*getFunction)());
-            prevChild = currChild;
-            currChild = prevChild->next();
-            currParent = currParent->next();
-        }
-
-        while (currChild) {
-            /* Reset any remaining layers to not have the property set. */
-            (currChild->*clearFunction)();
-            currChild = currChild->next();
+            (child->*setFunction)((parent->*getFunction)());
+            previousChild = child;
+            child = previousChild->next();
         }
+        for (; child; child = child->next())
+            (child->*clearFunction)();
     }
 
     static void applyInitialValue(CSSPropertyID, StyleResolver* styleResolver)
     {
         // Check for (single-layer) no-op before clearing anything.
         const FillLayer& layers = *(styleResolver->style()->*layersFunction)();
-        bool firstLayerHasValue = (layers.*testFunction)();
-        if (!layers.next() && (!firstLayerHasValue || (layers.*getFunction)() == (*initialFunction)(fillLayerType)))
+        if (!layers.next() && (!(layers.*testFunction)() || (layers.*getFunction)() == (*initialFunction)(fillLayerType)))
             return;
 
-        FillLayer* currChild = (styleResolver->style()->*accessLayersFunction)();
-        (currChild->*setFunction)((*initialFunction)(fillLayerType));
-        for (currChild = currChild->next(); currChild; currChild = currChild->next())
-            (currChild->*clearFunction)();
+        FillLayer* child = (styleResolver->style()->*accessLayersFunction)();
+        (child->*setFunction)((*initialFunction)(fillLayerType));
+        for (child = child->next(); child; child = child->next())
+            (child->*clearFunction)();
     }
 
     static void applyValue(CSSPropertyID, StyleResolver* styleResolver, CSSValue* value)
     {
-        FillLayer* currChild = (styleResolver->style()->*accessLayersFunction)();
-        FillLayer* prevChild = 0;
+        FillLayer* child = (styleResolver->style()->*accessLayersFunction)();
+        FillLayer* previousChild = nullptr;
         if (value->isValueList()
 #if ENABLE(CSS_IMAGE_SET)
         && !value->isImageSetValue()
 #endif
         ) {
-            /* Walk each value and put it into a layer, creating new layers as needed. */
-            CSSValueList* valueList = toCSSValueList(value);
-            for (unsigned int i = 0; i < valueList->length(); i++) {
-                if (!currChild) {
-                    /* Need to make a new layer to hold this value */
-                    currChild = new FillLayer(fillLayerType);
-                    prevChild->setNext(currChild);
+            // Walk each value and put it into a layer, creating new layers as needed.
+            CSSValueList& valueList = toCSSValueList(*value);
+            for (unsigned i = 0; i < valueList.length(); i++) {
+                if (!child) {
+                    previousChild->setNext(std::make_unique<FillLayer>(fillLayerType));
+                    child = previousChild->next();
                 }
-                (styleResolver->styleMap()->*mapFillFunction)(propertyId, currChild, valueList->itemWithoutBoundsCheck(i));
-                prevChild = currChild;
-                currChild = currChild->next();
+                (styleResolver->styleMap()->*mapFillFunction)(propertyId, child, valueList.itemWithoutBoundsCheck(i));
+                previousChild = child;
+                child = child->next();
             }
         } else {
-            (styleResolver->styleMap()->*mapFillFunction)(propertyId, currChild, value);
-            currChild = currChild->next();
-        }
-        while (currChild) {
-            /* Reset all remaining layers to not have the property set. */
-            (currChild->*clearFunction)();
-            currChild = currChild->next();
+            (styleResolver->styleMap()->*mapFillFunction)(propertyId, child, value);
+            child = child->next();
         }
+        for (; child; child = child->next())
+            (child->*clearFunction)();
     }
 
     static PropertyHandler createHandler() { return PropertyHandler(&applyInheritValue, &applyInitialValue, &applyValue); }
@@ -2499,9 +2487,9 @@ DeprecatedStyleBuilder::DeprecatedStyleBuilder()
 #endif
     setPropertyHandler(CSSPropertyWebkitBoxDirection, ApplyPropertyDefault<EBoxDirection, &RenderStyle::boxDirection, EBoxDirection, &RenderStyle::setBoxDirection, EBoxDirection, &RenderStyle::initialBoxDirection>::createHandler());
     setPropertyHandler(CSSPropertyWebkitBoxFlex, ApplyPropertyDefault<float, &RenderStyle::boxFlex, float, &RenderStyle::setBoxFlex, float, &RenderStyle::initialBoxFlex>::createHandler());
-    setPropertyHandler(CSSPropertyWebkitBoxFlexGroup, ApplyPropertyDefault<unsigned int, &RenderStyle::boxFlexGroup, unsigned int, &RenderStyle::setBoxFlexGroup, unsigned int, &RenderStyle::initialBoxFlexGroup>::createHandler());
+    setPropertyHandler(CSSPropertyWebkitBoxFlexGroup, ApplyPropertyDefault<unsigned, &RenderStyle::boxFlexGroup, unsigned, &RenderStyle::setBoxFlexGroup, unsigned, &RenderStyle::initialBoxFlexGroup>::createHandler());
     setPropertyHandler(CSSPropertyWebkitBoxLines, ApplyPropertyDefault<EBoxLines, &RenderStyle::boxLines, EBoxLines, &RenderStyle::setBoxLines, EBoxLines, &RenderStyle::initialBoxLines>::createHandler());
-    setPropertyHandler(CSSPropertyWebkitBoxOrdinalGroup, ApplyPropertyDefault<unsigned int, &RenderStyle::boxOrdinalGroup, unsigned int, &RenderStyle::setBoxOrdinalGroup, unsigned int, &RenderStyle::initialBoxOrdinalGroup>::createHandler());
+    setPropertyHandler(CSSPropertyWebkitBoxOrdinalGroup, ApplyPropertyDefault<unsigned, &RenderStyle::boxOrdinalGroup, unsigned, &RenderStyle::setBoxOrdinalGroup, unsigned, &RenderStyle::initialBoxOrdinalGroup>::createHandler());
     setPropertyHandler(CSSPropertyWebkitBoxOrient, ApplyPropertyDefault<EBoxOrient, &RenderStyle::boxOrient, EBoxOrient, &RenderStyle::setBoxOrient, EBoxOrient, &RenderStyle::initialBoxOrient>::createHandler());
     setPropertyHandler(CSSPropertyWebkitBoxPack, ApplyPropertyDefault<EBoxPack, &RenderStyle::boxPack, EBoxPack, &RenderStyle::setBoxPack, EBoxPack, &RenderStyle::initialBoxPack>::createHandler());
     setPropertyHandler(CSSPropertyWebkitColorCorrection, ApplyPropertyDefault<ColorSpace, &RenderStyle::colorSpace, ColorSpace, &RenderStyle::setColorSpace, ColorSpace, &RenderStyle::initialColorSpace>::createHandler());
index 27e9177..4bf4891 100644 (file)
@@ -1409,7 +1409,7 @@ bool RenderBox::backgroundHasOpaqueTopLayer() const
     if (hasOverflowClip() && fillLayer->attachment() == LocalBackgroundAttachment)
         return false;
 
-    if (fillLayer->hasOpaqueImage(this) && fillLayer->hasRepeatXY() && fillLayer->image()->canRender(this, style().effectiveZoom()))
+    if (fillLayer->hasOpaqueImage(*this) && fillLayer->hasRepeatXY() && fillLayer->image()->canRender(this, style().effectiveZoom()))
         return true;
 
     // If there is only one layer and no image, check whether the background color is opaque
@@ -1511,7 +1511,7 @@ void RenderBox::paintFillLayers(const PaintInfo& paintInfo, const Color& c, cons
             shouldDrawBackgroundInSeparateBuffer = true;
 
         // The clipOccludesNextLayers condition must be evaluated first to avoid short-circuiting.
-        if (curLayer->clipOccludesNextLayers(curLayer == fillLayer) && curLayer->hasOpaqueImage(this) && curLayer->image()->canRender(this, style().effectiveZoom()) && curLayer->hasRepeatXY() && curLayer->blendMode() == BlendModeNormal)
+        if (curLayer->clipOccludesNextLayers(curLayer == fillLayer) && curLayer->hasOpaqueImage(*this) && curLayer->image()->canRender(this, style().effectiveZoom()) && curLayer->hasRepeatXY() && curLayer->blendMode() == BlendModeNormal)
             break;
         curLayer = curLayer->next();
     }
index 93068f1..d69c528 100644 (file)
@@ -774,7 +774,7 @@ void RenderBoxModelObject::paintFillLayerExtended(const PaintInfo& paintInfo, co
     if (!bgLayer->next()) {
         LayoutRect backgroundRect(scrolledPaintRect);
         bool boxShadowShouldBeAppliedToBackground = this->boxShadowShouldBeAppliedToBackground(bleedAvoidance, box);
-        if (boxShadowShouldBeAppliedToBackground || !shouldPaintBackgroundImage || !bgLayer->hasOpaqueImage(this) || !bgLayer->hasRepeatXY()) {
+        if (boxShadowShouldBeAppliedToBackground || !shouldPaintBackgroundImage || !bgLayer->hasOpaqueImage(*this) || !bgLayer->hasRepeatXY()) {
             if (!boxShadowShouldBeAppliedToBackground)
                 backgroundRect.intersect(paintInfo.rect);
 
index 3c563b2..7fba562 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 1999 Antti Koivisto (koivisto@kde.org)
- * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2014 Apple Inc. 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
 namespace WebCore {
 
 struct SameSizeAsFillLayer {
-    FillLayer* m_next;
+    FillLayer* next;
 
-    RefPtr<StyleImage> m_image;
+    RefPtr<StyleImage> image;
 
-    Length m_xPosition;
-    Length m_yPosition;
+    Length x;
+    Length y;
 
-    LengthSize m_sizeLength;
+    LengthSize sizeLength;
 
-    unsigned m_bitfields: 32;
-    unsigned m_bitfields2: 1;
+    unsigned bitfields : 32;
+    unsigned bitfields2 : 11;
 };
 
 COMPILE_ASSERT(sizeof(FillLayer) == sizeof(SameSizeAsFillLayer), FillLayer_should_stay_small);
 
 FillLayer::FillLayer(EFillLayerType type)
-    : m_next(0)
-    , m_image(FillLayer::initialFillImage(type))
+    : m_image(FillLayer::initialFillImage(type))
     , m_xPosition(FillLayer::initialFillXPosition(type))
     , m_yPosition(FillLayer::initialFillYPosition(type))
     , m_sizeLength(FillLayer::initialFillSizeLength(type))
@@ -74,7 +73,7 @@ FillLayer::FillLayer(EFillLayerType type)
 }
 
 FillLayer::FillLayer(const FillLayer& o)
-    : m_next(o.m_next ? new FillLayer(*o.m_next) : 0)
+    : m_next(o.m_next ? std::make_unique<FillLayer>(*o.m_next) : nullptr)
     , m_image(o.m_image)
     , m_xPosition(o.m_xPosition)
     , m_yPosition(o.m_yPosition)
@@ -108,15 +107,13 @@ FillLayer::FillLayer(const FillLayer& o)
 
 FillLayer::~FillLayer()
 {
-    delete m_next;
+    // Delete the layers in a loop rather than allowing recursive calls to the destructors.
+    for (std::unique_ptr<FillLayer> next = std::move(m_next); next; next = std::move(next->m_next)) { }
 }
 
 FillLayer& FillLayer::operator=(const FillLayer& o)
 {
-    if (m_next != o.m_next) {
-        delete m_next;
-        m_next = o.m_next ? new FillLayer(*o.m_next) : 0;
-    }
+    m_next = o.m_next ? std::make_unique<FillLayer>(*o.m_next) : nullptr;
 
     m_image = o.m_image;
     m_xPosition = o.m_xPosition;
@@ -155,14 +152,14 @@ FillLayer& FillLayer::operator=(const FillLayer& o)
 bool FillLayer::operator==(const FillLayer& o) const
 {
     // We do not check the "isSet" booleans for each property, since those are only used during initial construction
-    // to propagate patterns into layers.  All layer comparisons happen after values have all been filled in anyway.
+    // to propagate patterns into layers. All layer comparisons happen after values have all been filled in anyway.
     return StyleImage::imagesEquivalent(m_image.get(), o.m_image.get()) && m_xPosition == o.m_xPosition && m_yPosition == o.m_yPosition
-            && m_backgroundXOrigin == o.m_backgroundXOrigin && m_backgroundYOrigin == o.m_backgroundYOrigin
-            && m_attachment == o.m_attachment && m_clip == o.m_clip && m_composite == o.m_composite
-            && m_blendMode == o.m_blendMode && m_origin == o.m_origin && m_repeatX == o.m_repeatX
-            && m_repeatY == o.m_repeatY && m_sizeType == o.m_sizeType && m_maskSourceType == o.m_maskSourceType
-            && m_sizeLength == o.m_sizeLength && m_type == o.m_type
-            && ((m_next && o.m_next) ? *m_next == *o.m_next : m_next == o.m_next);
+        && m_backgroundXOrigin == o.m_backgroundXOrigin && m_backgroundYOrigin == o.m_backgroundYOrigin
+        && m_attachment == o.m_attachment && m_clip == o.m_clip && m_composite == o.m_composite
+        && m_blendMode == o.m_blendMode && m_origin == o.m_origin && m_repeatX == o.m_repeatX
+        && m_repeatY == o.m_repeatY && m_sizeType == o.m_sizeType && m_maskSourceType == o.m_maskSourceType
+        && m_sizeLength == o.m_sizeLength && m_type == o.m_type
+        && ((m_next && o.m_next) ? *m_next == *o.m_next : m_next == o.m_next);
 }
 
 void FillLayer::fillUnsetProperties()
@@ -290,18 +287,15 @@ void FillLayer::fillUnsetProperties()
 
 void FillLayer::cullEmptyLayers()
 {
-    FillLayer* next;
-    for (FillLayer* p = this; p; p = next) {
-        next = p->m_next;
-        if (next && !next->isImageSet()) {
-            delete next;
-            p->m_next = 0;
+    for (FillLayer* layer = this; layer; layer = layer->m_next.get()) {
+        if (layer->m_next && !layer->m_next->isImageSet()) {
+            layer->m_next = nullptr;
             break;
         }
     }
 }
 
-static EFillBox clipMax(EFillBox clipA, EFillBox clipB)
+static inline EFillBox clipMax(EFillBox clipA, EFillBox clipB)
 {
     if (clipA == BorderFillBox || clipB == BorderFillBox)
         return BorderFillBox;
@@ -314,11 +308,15 @@ static EFillBox clipMax(EFillBox clipA, EFillBox clipB)
 
 void FillLayer::computeClipMax() const
 {
-    if (m_next) {
-        m_next->computeClipMax();
-        m_clipMax = clipMax(clip(), m_next->clip());
-    } else
-        m_clipMax = m_clip;
+    Vector<const FillLayer*, 4> layers;
+    for (auto* layer = this; layer; layer = layer->m_next.get())
+        layers.append(layer);
+    EFillBox computedClipMax = TextFillBox;
+    for (unsigned i = layers.size(); i; --i) {
+        auto& layer = *layers[i - 1];
+        computedClipMax = clipMax(computedClipMax, layer.clip());
+        layer.m_clipMax = computedClipMax;
+    }
 }
 
 bool FillLayer::clipOccludesNextLayers(bool firstLayer) const
@@ -328,29 +326,25 @@ bool FillLayer::clipOccludesNextLayers(bool firstLayer) const
     return m_clip == m_clipMax;
 }
 
-bool FillLayer::containsImage(StyleImage* s) const
+bool FillLayer::containsImage(StyleImage& image) const
 {
-    if (!s)
-        return false;
-    if (m_image && *s == *m_image)
-        return true;
-    if (m_next)
-        return m_next->containsImage(s);
+    for (auto* layer = this; layer; layer = layer->m_next.get()) {
+        if (layer->m_image && image == *layer->m_image)
+            return true;
+    }
     return false;
 }
 
 bool FillLayer::imagesAreLoaded() const
 {
-    const FillLayer* curr;
-    for (curr = this; curr; curr = curr->next()) {
-        if (curr->m_image && !curr->m_image->isLoaded())
+    for (auto* layer = this; layer; layer = layer->m_next.get()) {
+        if (layer->m_image && !layer->m_image->isLoaded())
             return false;
     }
-
     return true;
 }
 
-bool FillLayer::hasOpaqueImage(const RenderElement* renderer) const
+bool FillLayer::hasOpaqueImage(const RenderElement& renderer) const
 {
     if (!m_image)
         return false;
@@ -358,18 +352,30 @@ bool FillLayer::hasOpaqueImage(const RenderElement* renderer) const
     if (m_composite == CompositeClear || m_composite == CompositeCopy)
         return true;
 
-    if (m_blendMode != BlendModeNormal)
-        return false;
+    return m_blendMode == BlendModeNormal && m_composite == CompositeSourceOver && m_image->knownToBeOpaque(&renderer);
+}
 
-    if (m_composite == CompositeSourceOver)
-        return m_image->knownToBeOpaque(renderer);
+bool FillLayer::hasRepeatXY() const
+{
+    return m_repeatX == RepeatFill && m_repeatY == RepeatFill;
+}
 
+bool FillLayer::hasImage() const
+{
+    for (auto* layer = this; layer; layer = layer->m_next.get()) {
+        if (layer->m_image)
+            return true;
+    }
     return false;
 }
 
-bool FillLayer::hasRepeatXY() const
+bool FillLayer::hasFixedImage() const
 {
-    return m_repeatX == RepeatFill && m_repeatY == RepeatFill;
+    for (auto* layer = this; layer; layer = layer->m_next.get()) {
+        if (layer->m_image && layer->m_attachment == FixedBackgroundAttachment)
+            return true;
+    }
+    return false;
 }
 
 } // namespace WebCore
index d49a5c2..ce4b4f3 100644 (file)
@@ -2,7 +2,7 @@
  * Copyright (C) 2000 Lars Knoll (knoll@kde.org)
  *           (C) 2000 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
- * Copyright (C) 2003, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2003, 2005, 2006, 2007, 2008, 2014 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *
  * This library is free software; you can redistribute it and/or
@@ -26,7 +26,6 @@
 #define FillLayer_h
 
 #include "GraphicsTypes.h"
-#include "Length.h"
 #include "LengthSize.h"
 #include "RenderStyleConstants.h"
 #include "StyleImage.h"
@@ -42,29 +41,30 @@ struct FillSize {
     {
     }
 
-    FillSize(EFillSizeType t, LengthSize size)
-        : type(t)
-        , size(std::move(size))
+    FillSize(EFillSizeType type, const LengthSize& size)
+        : type(type)
+        , size(size)
     {
     }
 
-    bool operator==(const FillSize& o) const
-    {
-        return type == o.type && size == o.size;
-    }
-    bool operator!=(const FillSize& o) const
-    {
-        return !(*this == o);
-    }
-
     EFillSizeType type;
     LengthSize size;
 };
 
+inline bool operator==(const FillSize& a, const FillSize& b)
+{
+    return a.type == b.type && a.size == b.size;
+}
+
+inline bool operator!=(const FillSize& a, const FillSize& b)
+{
+    return !(a == b);
+}
+
 class FillLayer {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    FillLayer(EFillLayerType);
+    explicit FillLayer(EFillLayerType);
     ~FillLayer();
 
     StyleImage* image() const { return m_image.get(); }
@@ -84,8 +84,8 @@ public:
     FillSize size() const { return FillSize(static_cast<EFillSizeType>(m_sizeType), m_sizeLength); }
     EMaskSourceType maskSourceType() const { return static_cast<EMaskSourceType>(m_maskSourceType); }
 
-    const FillLayer* next() const { return m_next; }
-    FillLayer* next() { return m_next; }
+    const FillLayer* next() const { return m_next.get(); }
+    FillLayer* next() { return m_next.get(); }
 
     bool isImageSet() const { return m_imageSet; }
     bool isXPositionSet() const { return m_xPosSet; }
@@ -101,7 +101,7 @@ public:
     bool isSizeSet() const { return m_sizeType != SizeNone; }
     bool isMaskSourceTypeSet() const { return m_maskSourceTypeSet; }
 
-    void setImage(PassRefPtr<StyleImage> i) { m_image = i; m_imageSet = true; }
+    void setImage(PassRefPtr<StyleImage> image) { m_image = image; m_imageSet = true; }
     void setXPosition(Length length) { m_xPosition = std::move(length); m_xPosSet = true; }
     void setYPosition(Length length) { m_yPosition = std::move(length); m_yPosSet = true; }
     void setBackgroundXOrigin(BackgroundEdgeOrigin o) { m_backgroundXOrigin = o; m_backgroundOriginSet = true; }
@@ -119,16 +119,8 @@ public:
     void setMaskSourceType(EMaskSourceType m) { m_maskSourceType = m; m_maskSourceTypeSet = true; }
 
     void clearImage() { m_image.clear(); m_imageSet = false; }
-    void clearXPosition()
-    {
-        m_xPosSet = false;
-        m_backgroundOriginSet = false;
-    }
-    void clearYPosition()
-    {
-        m_yPosSet = false;
-        m_backgroundOriginSet = false;
-    }
+    void clearXPosition() { m_xPosSet = false; m_backgroundOriginSet = false; }
+    void clearYPosition() { m_yPosSet = false; m_backgroundOriginSet = false; }
 
     void clearAttachment() { m_attachmentSet = false; }
     void clearClip() { m_clipSet = false; }
@@ -140,35 +132,19 @@ public:
     void clearSize() { m_sizeType = SizeNone; }
     void clearMaskSourceType() { m_maskSourceTypeSet = false; }
 
-    void setNext(FillLayer* n) { if (m_next != n) { delete m_next; m_next = n; } }
+    void setNext(std::unique_ptr<FillLayer> next) { m_next = std::move(next); }
 
-    FillLayer& operator=(const FillLayer& o);    
-    FillLayer(const FillLayer& o);
+    FillLayer& operator=(const FillLayer&);
+    FillLayer(const FillLayer&);
 
-    bool operator==(const FillLayer& o) const;
-    bool operator!=(const FillLayer& o) const
-    {
-        return !(*this == o);
-    }
+    bool operator==(const FillLayer&) const;
+    bool operator!=(const FillLayer& other) const { return !(*this == other); }
 
-    bool containsImage(StyleImage*) const;
+    bool containsImage(StyleImage&) const;
     bool imagesAreLoaded() const;
-
-    bool hasImage() const
-    {
-        if (m_image)
-            return true;
-        return m_next ? m_next->hasImage() : false;
-    }
-
-    bool hasFixedImage() const
-    {
-        if (m_image && m_attachment == FixedBackgroundAttachment)
-            return true;
-        return m_next ? m_next->hasFixedImage() : false;
-    }
-
-    bool hasOpaqueImage(const RenderElement*) const;
+    bool hasImage() const;
+    bool hasFixedImage() const;
+    bool hasOpaqueImage(const RenderElement&) const;
     bool hasRepeatXY() const;
     bool clipOccludesNextLayers(bool firstLayer) const;
 
@@ -197,9 +173,7 @@ private:
 
     void computeClipMax() const;
 
-    FillLayer() { }
-
-    FillLayer* m_next;
+    std::unique_ptr<FillLayer> m_next;
 
     RefPtr<StyleImage> m_image;