Summary: | [CSS Shapes] Shape methods and member variables should be guarded with the CSS_SHAPES flag | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Bear Travis <betravis> | ||||||||||
Component: | CSS | Assignee: | Bear Travis <betravis> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, esprehn+autocc, glenn, kondapallykalyan, macpherson, menard | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 89256, 119019 | ||||||||||||
Attachments: |
|
Description
Bear Travis
2013-06-05 14:52:27 PDT
Created attachment 204856 [details]
Initial Patch
Comment on attachment 204856 [details] Initial Patch Attachment 204856 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/884145 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? 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? The cause looks to be a bit more mundane. I've missed a couple segments of code that must also use the compile guard. Created attachment 204930 [details]
Updated patch
Comment on attachment 204930 [details] Updated patch Attachment 204930 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/934040 Created attachment 205029 [details]
Modifying one of the .in files
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. Created attachment 207348 [details] Updated Making requested changes, filed bug 119019 to track exclusions compile guard Comment on attachment 207348 [details]
Updated
Thanks! r=me
Comment on attachment 207348 [details] Updated Clearing flags on attachment: 207348 Committed r153330: <http://trac.webkit.org/changeset/153330> All reviewed patches have been landed. Closing bug. |