Bug 117277 – [CSS Shapes] Shape methods and member variables should be guarded with the CSS_SHAPES flag

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Bear Travis]

See RenderStyle and RareNonInheritedData

[Bear Travis]

Created attachment 204856 [details]
Initial Patch

[Build Bot]

Comment on attachment 204856 [details]
Initial Patch

Attachment 204856 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/884145

[Brent Fulgham]

This patch seems to break the Windows build.  Do we need to touch one of the *.in files to get a full regeneration for this patch to work?

[Brent Fulgham]

This patch seems to break the Windows build.  Do we need to touch one of the *.in files to get a full regeneration for this patch to work?

[Bear Travis]

The cause looks to be a bit more mundane. I've missed a couple segments of code that must also use the compile guard.

[Bear Travis]

Created attachment 204930 [details]
Updated patch

[Build Bot]

Comment on attachment 204930 [details]
Updated patch

Attachment 204930 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/934040

[Bear Travis]

Created attachment 205029 [details]
Modifying one of the .in files

[Alexandru Chiculita]

Comment on attachment 205029 [details]
Modifying one of the .in files

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

> Source/WebCore/rendering/RenderBlock.cpp:1471
>  static inline bool shapeInfoRequiresRelayout(const RenderBlock* block)
>  {

I think we could fix RenderBlock::updateRegionsAndShapesBeforeChildLayout to not call this function when CSS_SHAPES is disabled. For example, the first lines in RenderBlock::updateRegionsAndShapesBeforeChildLayout are very hard to read.

> Source/WebCore/rendering/RenderBlock.cpp:4457
> +#else
> +        UNUSED_PARAM(offsetMode);

You already have an unused_param for offset mode in that function. Should you remove the other one?

> Source/WebCore/rendering/RenderBlock.cpp:4520
> +        UNUSED_PARAM(offsetMode);

ditto

> Source/WebCore/rendering/style/RenderStyle.cpp:403
>          if (rareNonInheritedData->m_wrapFlow != other->rareNonInheritedData->m_wrapFlow

Is there a patch for adding these lines to the exclusions ifdef?

> Source/WebCore/rendering/style/RenderStyle.cpp:707
> +#if ENABLE(CSS_SHAPES)

The comment before this line seems related to the code inside the ifdef. I would move the comment inside the ifdef.

[Bear Travis]

Created attachment 207348 [details]
Updated

Making requested changes, filed bug 119019 to track exclusions compile guard

[Alexandru Chiculita]

Comment on attachment 207348 [details]
Updated

Thanks! r=me

[WebKit Commit Bot]

Comment on attachment 207348 [details]
Updated

Clearing flags on attachment: 207348

Committed r153330: <http://trac.webkit.org/changeset/153330>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.