Bug 92927

Summary: Switch mapLocalToContainer to use a flag instead of boolean parameters
Product: WebKit Reporter: Levi Weintraub <leviw>
Component: Layout and RenderingAssignee: Levi Weintraub <leviw>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 89238    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Levi Weintraub 2012-08-01 17:49:53 PDT
I intend on adding another mode of operation for bug switching between pixel snapping which will further complicate the current contract.
Comment 1 Levi Weintraub 2012-08-02 10:12:17 PDT
Created attachment 156114 [details]
Patch
Comment 2 Build Bot 2012-08-02 10:34:07 PDT
Comment on attachment 156114 [details]
Patch

Attachment 156114 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13425302
Comment 3 Dirk Schulze 2012-08-02 10:41:07 PDT
Comment on attachment 156114 [details]
Patch

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

> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:193
> +void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const

You need to comment it out, or flag it unused.

> Source/WebCore/rendering/svg/RenderSVGInline.cpp:82
> +void RenderSVGInline::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const

Ditto.

> Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:58
> +void RenderSVGModelObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const

Ditto.
Comment 4 Levi Weintraub 2012-08-02 11:26:48 PDT
(In reply to comment #3)
> (From update of attachment 156114 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156114&action=review
> 
> > Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:193
> > +void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
> 
> You need to comment it out, or flag it unused.
> 
> > Source/WebCore/rendering/svg/RenderSVGInline.cpp:82
> > +void RenderSVGInline::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
> 
> Ditto.
> 
> > Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:58
> > +void RenderSVGModelObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
> 
> Ditto.

Of course, thank you :)
Comment 5 Levi Weintraub 2012-08-02 14:06:15 PDT
Created attachment 156161 [details]
Patch
Comment 6 Dirk Schulze 2012-08-02 20:21:52 PDT
Comment on attachment 156161 [details]
Patch

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

LGTM. Just some snippets, r=me.

> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:195
> +void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
>  {
> +    UNUSED_PARAM(mode);

Sorry for not mention it before. But isn't it easier to just not use 'mode'? I think MapLocalToContainerFlags is self descriptive.

> Source/WebCore/rendering/svg/RenderSVGInline.cpp:84
> +    UNUSED_PARAM(mode);

Ditto.

> Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:60
> +    UNUSED_PARAM(mode);

Ditto.

> Source/WebCore/rendering/svg/RenderSVGText.cpp:116
> +    UNUSED_PARAM(mode);

Ditto.
Comment 7 Levi Weintraub 2012-08-03 10:27:08 PDT
Created attachment 156410 [details]
Patch for landing
Comment 8 WebKit Review Bot 2012-08-03 15:23:58 PDT
Comment on attachment 156410 [details]
Patch for landing

Clearing flags on attachment: 156410

Committed r124662: <http://trac.webkit.org/changeset/124662>
Comment 9 WebKit Review Bot 2012-08-03 15:24:02 PDT
All reviewed patches have been landed.  Closing bug.