Move special code for 'shape-outside' out of StyleBuilderFunctions.cpp.tmpl.

Move special code for 'shape-outside' out of StyleBuilderFunctions.cpp.tmpl.

There should not be special code generation for a single property, at
least not in cases where a converter can easily be used instead.

Changes:

 * Assume that 'value' is either 'none', an image value, or a list of
   values.
 * Use primitive value mapping for CSSBoxType, since parser ensures
   that only valid values go though.

R=timloh@chromium.org

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

[Timothy Loh, 2014-10-20 12:29:57]

lgtm

https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
File Source/core/css/resolver/StyleBuilderConverter.cpp (right):

https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
Source/core/css/resolver/StyleBuilderConverter.cpp:732:
ASSERT(value->isValueList());
I wouldn't bother with this since toCSSValueList checks for us.

https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
Source/core/css/resolver/StyleBuilderConverter.cpp:749: return nullptr;
I think we never reach this?

[andersr, 2014-10-20 12:36:03]

https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
File Source/core/css/resolver/StyleBuilderConverter.cpp (right):

https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
Source/core/css/resolver/StyleBuilderConverter.cpp:749: return nullptr;
On 2014/10/20 12:29:57, Timothy Loh wrote:
> I think we never reach this?

Can the parser deliver an empty CSSValueList?

[Timothy Loh, 2014-10-20 12:44:10]

On 2014/10/20 12:36:03, andersr wrote:
>
https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
> File Source/core/css/resolver/StyleBuilderConverter.cpp (right):
> 
>
https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
> Source/core/css/resolver/StyleBuilderConverter.cpp:749: return nullptr;
> On 2014/10/20 12:29:57, Timothy Loh wrote:
> > I think we never reach this?
> 
> Can the parser deliver an empty CSSValueList?

It doesn't look like it. My reasoning was: This would only occur from
parseBasicShapeAndOrBox(), by breaking out of the loop on the first iteration,
which means m_valueList->current() is null upon entering the function. But in
parseShapeProperty we access m_valueList->current()->id, which would crash if
the value was null (anyway, we null-check this at the top of parseValue, around
line 447).

[andersr, 2014-10-20 13:07:48]

On 2014/10/20 12:44:10, Timothy Loh wrote:
> On 2014/10/20 12:36:03, andersr wrote:
> >
>
https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
> > File Source/core/css/resolver/StyleBuilderConverter.cpp (right):
> > 
> >
>
https://codereview.chromium.org/645073003/diff/1/Source/core/css/resolver/Sty...
> > Source/core/css/resolver/StyleBuilderConverter.cpp:749: return nullptr;
> > On 2014/10/20 12:29:57, Timothy Loh wrote:
> > > I think we never reach this?
> > 
> > Can the parser deliver an empty CSSValueList?
> 
> It doesn't look like it. My reasoning was: This would only occur from
> parseBasicShapeAndOrBox(), by breaking out of the loop on the first iteration,
> which means m_valueList->current() is null upon entering the function. But in
> parseShapeProperty we access m_valueList->current()->id, which would crash if
> the value was null (anyway, we null-check this at the top of parseValue,
around
> line 447).

Ah, yes. Missed that.

Removed the ASSERT and the unreachable code in Patch #2.

[Timothy Loh, 2014-10-20 13:10:46]

still lgtm :)

[andersr, 2014-10-20 13:14:52]

The CQ bit was checked by andersr@opera.com

[commit-bot: I haz the power, 2014-10-20 13:15:09]

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

[commit-bot: I haz the power, 2014-10-20 14:17:37]

Committed patchset #2 (id:20001) as 183982