[webkit-reviews] review granted: [Bug 215942] Make StyleRareNonInheritedData::mask and StyleBackgroundData::background DataRefs : [Attachment 407497] For EWS (and fix GTK build)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 215942: Make StyleRareNonInheritedData::mask and
StyleBackgroundData::background DataRefs
https://bugs.webkit.org/show_bug.cgi?id=215942

Attachment 407497: For EWS (and fix GTK build)

https://bugs.webkit.org/attachment.cgi?id=407497&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 407497
  --> https://bugs.webkit.org/attachment.cgi?id=407497
For EWS (and fix GTK build)

View in context: https://bugs.webkit.org/attachment.cgi?id=407497&action=review

> Source/WebCore/rendering/style/FillLayer.cpp:30
> +struct SameSizeAsFillLayer : public RefCounted<SameSizeAsFillLayer> {

No need for "public" here.

> Source/WebCore/rendering/style/FillLayer.cpp:31
> +    RefPtr<FillLayer> next;

SameSize structures don’t really need to use smart pointers instead of raw
pointers.

> Source/WebCore/rendering/style/FillLayer.cpp:110
> +	   m_next = FillLayer::create(*o.m_next);

Should just call create here, not FillLayer::create.

> Source/WebCore/rendering/style/FillLayer.h:74
> +    static Ref<FillLayer> create(FillLayerType type)
> +    {
> +	   return adoptRef(*new FillLayer(type));
> +    }
> +
> +    static Ref<FillLayer> create(const FillLayer& layer)
> +    {
> +	   return adoptRef(*new FillLayer(layer));
> +    }

I suggest not inlining these. We’d rather inline the constructor inside the
create function rather than inlining the allocation and then paying function
call overhead to call the constructor.

> Source/WebCore/rendering/style/FillLayer.h:79
> +    Ref<FillLayer> copy() const
> +    {
> +	   return FillLayer::create(*this);
> +    }

Should call just "create" and should write as a one-liner:

    Ref<FillLayer> copy() const { return create(*this); }