Constify and use reference for RenderStyle in LayoutTheme

Constify and use reference for RenderStyle in LayoutTheme

As part of this change, fixed some functions to take
a constant reference as they were not expected to modify
the RenderStyle.

Another step towards making RenderObject::style()
return a const RenderStyle.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189565

[Julien - ping for review, 2015-02-04 21:20:34]

jchaffraix@chromium.org changed reviewers:
+ dsinclair@chromium.org

[Julien - ping for review, 2015-02-04 21:20:34]

Another fun change :-)

[dsinclair, 2015-02-04 21:59:29]

lgtm. Pointed out a few more possible consts.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
File Source/core/layout/LayoutTheme.cpp (right):

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutTheme.cpp:563: bool LayoutTheme::isControlStyled(const
RenderStyle& style, const CachedUAStyle* uaStyle) const
Make uaStyle a ref as well? We immediately ASSERT(uaStyle)

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
File Source/core/layout/LayoutThemeChromiumMac.mm (right):

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:472: bool
LayoutThemeChromiumMac::isControlStyled(const RenderStyle& style, const
CachedUAStyle* uaStyle) const
Reference for uaStyle?

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:476: return style.border() !=
uastyle.border || style.boxShadow();
uastyle should be uaStyle and this should still be a pointer (unless you make
the ref change above)

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:588: NSControlSize
LayoutThemeChromiumMac::controlSizeForFont(RenderStyle& style) const
Looks like RenderStyle can be const here?

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:601: static NSControlSize
searchFieldControlSizeForFont(RenderStyle& style)
const RenderStyle?

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:628: IntSize
LayoutThemeChromiumMac::sizeForFont(const RenderStyle& style, const IntSize*
sizes) const
Can sizes be a ref? We access without checking if it's null already.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:637: IntSize
LayoutThemeChromiumMac::sizeForSystemFont(const RenderStyle& style, const
IntSize* sizes) const
sizes as ref?

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:675: NSControlSize
LayoutThemeChromiumMac::controlSizeForSystemFont(RenderStyle& style) const
const?

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:917: RenderStyle& style =
renderMeter->style();
const?

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:975: int
LayoutThemeChromiumMac::minimumProgressBarHeight(RenderStyle& style) const
Can't tell from the code below, can this be const?

[Julien - ping for review, 2015-02-04 22:32:18]

Thanks for the review. Your comments match the version 2 of the change that was
caught by breaking Mac.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
File Source/core/layout/LayoutTheme.cpp (right):

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutTheme.cpp:563: bool LayoutTheme::isControlStyled(const
RenderStyle& style, const CachedUAStyle* uaStyle) const
On 2015/02/04 21:59:29, dsinclair wrote:
> Make uaStyle a ref as well? We immediately ASSERT(uaStyle)

That's a good point. However it's unclear if uaStyle is non-NULL in the whole
code path so I would rather be consistent and stick to pointers.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
File Source/core/layout/LayoutThemeChromiumMac.mm (right):

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:588: NSControlSize
LayoutThemeChromiumMac::controlSizeForFont(RenderStyle& style) const
On 2015/02/04 21:59:29, dsinclair wrote:
> Looks like RenderStyle can be const here?

Done.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:601: static NSControlSize
searchFieldControlSizeForFont(RenderStyle& style)
On 2015/02/04 21:59:29, dsinclair wrote:
> const RenderStyle?

Done.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:628: IntSize
LayoutThemeChromiumMac::sizeForFont(const RenderStyle& style, const IntSize*
sizes) const
On 2015/02/04 21:59:29, dsinclair wrote:
> Can sizes be a ref? We access without checking if it's null already.

|sizes| is an array of IntSize so we can't remove the '*'. We could turn them
into const IntSize*& but I don't think that's a good change.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:637: IntSize
LayoutThemeChromiumMac::sizeForSystemFont(const RenderStyle& style, const
IntSize* sizes) const
On 2015/02/04 21:59:29, dsinclair wrote:
> sizes as ref?

Ditto.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:675: NSControlSize
LayoutThemeChromiumMac::controlSizeForSystemFont(RenderStyle& style) const
On 2015/02/04 21:59:29, dsinclair wrote:
> const?

Done.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:917: RenderStyle& style =
renderMeter->style();
On 2015/02/04 21:59:29, dsinclair wrote:
> const?

Done.

https://codereview.chromium.org/894393003/diff/1/Source/core/layout/LayoutThe...
Source/core/layout/LayoutThemeChromiumMac.mm:975: int
LayoutThemeChromiumMac::minimumProgressBarHeight(RenderStyle& style) const
On 2015/02/04 21:59:29, dsinclair wrote:
> Can't tell from the code below, can this be const?

It can. Done.

[Julien - ping for review, 2015-02-04 22:32:23]

The CQ bit was checked by jchaffraix@chromium.org

[commit-bot: I haz the power, 2015-02-04 22:32:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/894393003/20001

[commit-bot: I haz the power, 2015-02-05 15:27:56]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189565