Bug 117277

Summary: [CSS Shapes] Shape methods and member variables should be guarded with the CSS_SHAPES flag
Product: WebKit Reporter: Bear Travis <betravis>
Component: CSSAssignee: 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 Flags
Initial Patch
none
Updated patch
none
Modifying one of the .in files
none
Updated none

Bear Travis
Reported 2013-06-05 14:52:27 PDT
See RenderStyle and RareNonInheritedData
Attachments
Initial Patch (6.25 KB, patch)
2013-06-17 14:45 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Updated patch (12.77 KB, patch)
2013-06-18 12:37 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Modifying one of the .in files (13.25 KB, patch)
2013-06-19 13:01 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Updated (14.54 KB, patch)
2013-07-23 14:04 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Bear Travis
Comment 1 2013-06-17 14:45:26 PDT
Created attachment 204856 [details] Initial Patch
Build Bot
Comment 2 2013-06-17 18:20:48 PDT
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
Comment 3 2013-06-18 09:32:49 PDT
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
Comment 4 2013-06-18 09:32:49 PDT
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
Comment 5 2013-06-18 10:38:48 PDT
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
Comment 6 2013-06-18 12:37:08 PDT
Created attachment 204930 [details] Updated patch
Build Bot
Comment 7 2013-06-18 17:17:05 PDT
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
Comment 8 2013-06-19 13:01:05 PDT
Created attachment 205029 [details] Modifying one of the .in files
Alexandru Chiculita
Comment 9 2013-07-17 00:45:56 PDT
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
Comment 10 2013-07-23 14:04:51 PDT
Created attachment 207348 [details] Updated Making requested changes, filed bug 119019 to track exclusions compile guard
Alexandru Chiculita
Comment 11 2013-07-25 00:20:08 PDT
Comment on attachment 207348 [details] Updated Thanks! r=me
WebKit Commit Bot
Comment 12 2013-07-25 09:39:27 PDT
Comment on attachment 207348 [details] Updated Clearing flags on attachment: 207348 Committed r153330: <http://trac.webkit.org/changeset/153330>
WebKit Commit Bot
Comment 13 2013-07-25 09:39:29 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.