Remove "isImplicit" checks on fonts.

Remove some "isImplicit" checks on fonts.

Replace the check in appendFontLonghandValueIfExplicit with a check for
"normal", which is equivalent because the parser fills in all "implicit"
properties in the font longhand with "normal". Removing it would result in
things like:
"italic 20px sans-serif" -> "italic normal normal normal 20px/normal sans-serif".

These condition in fontValue() never matches because the parser always fills in
fontSizeProperty and fontFamilyProperty.
Rename appendFontLonghandValueIfExplicit to appendFontLonghandValueIfNotNormal.

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

[meade_UTC10, 2015-06-29 07:12:33]

meade@chromium.org changed reviewers:
+ timloh@chromium.org

[davve, 2015-06-29 08:12:47]

davve@opera.com changed reviewers:
+ davve@opera.com

[davve, 2015-06-29 08:12:47]

https://codereview.chromium.org/1219643002/diff/50001/LayoutTests/fast/css/ge...
File LayoutTests/fast/css/getPropertyValue-columns.html (right):

https://codereview.chromium.org/1219643002/diff/50001/LayoutTests/fast/css/ge...
LayoutTests/fast/css/getPropertyValue-columns.html:28: description("<a
href=\"https://bugs.webkit.org/show_bug.cgi?id=111011\">Bug 111011:
getPropertyValue for -webkit-columns returns null, should compute the shorthand
value<\/a>");
Unintentional escaping?

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
File Source/core/css/StylePropertySerializer.cpp (left):

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
Source/core/css/StylePropertySerializer.cpp:735: if
(!m_propertySet.isPropertyImplicit(shorthand.properties()[i])) {
Hm, what has this to do with fonts? From what I can see getShorthandValue is
never used on the font property?

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
File Source/core/css/StylePropertySerializer.cpp (right):

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
Source/core/css/StylePropertySerializer.cpp:488: &&
toCSSPrimitiveValue(m_propertySet.propertyAt(foundPropertyIndex).value())->getValueID()
== CSSValueNormal) {
This is not a removal but a replacement. You replace a never-matching
isImplicit-check with a check for normal. Perhaps something to add to the
description (along with any change in behavior)?

[meade_UTC10, 2015-06-30 08:03:17]

https://codereview.chromium.org/1219643002/diff/50001/LayoutTests/fast/css/ge...
File LayoutTests/fast/css/getPropertyValue-columns.html (right):

https://codereview.chromium.org/1219643002/diff/50001/LayoutTests/fast/css/ge...
LayoutTests/fast/css/getPropertyValue-columns.html:28: description("<a
href=\"https://bugs.webkit.org/show_bug.cgi?id=111011\">Bug 111011:
getPropertyValue for -webkit-columns returns null, should compute the shorthand
value<\/a>");
On 2015/06/29 at 08:12:47, David Vest wrote:
> Unintentional escaping?

My syntax highlighter was complaining...

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
File Source/core/css/StylePropertySerializer.cpp (left):

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
Source/core/css/StylePropertySerializer.cpp:735: if
(!m_propertySet.isPropertyImplicit(shorthand.properties()[i])) {
On 2015/06/29 at 08:12:47, David Vest wrote:
> Hm, what has this to do with fonts? From what I can see getShorthandValue is
never used on the font property?

...unintentional semi-related change. Moved it to
https://codereview.chromium.org/1215503009

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
File Source/core/css/StylePropertySerializer.cpp (right):

https://codereview.chromium.org/1219643002/diff/50001/Source/core/css/StylePr...
Source/core/css/StylePropertySerializer.cpp:488: &&
toCSSPrimitiveValue(m_propertySet.propertyAt(foundPropertyIndex).value())->getValueID()
== CSSValueNormal) {
On 2015/06/29 at 08:12:47, David Vest wrote:
> This is not a removal but a replacement. You replace a never-matching
isImplicit-check with a check for normal. Perhaps something to add to the
description (along with any change in behavior)?

Done.

[Timothy Loh, 2015-06-30 08:11:42]

mini nit: first line of description doesn't match patch title

lgtm

https://codereview.chromium.org/1219643002/diff/90001/Source/core/css/StylePr...
File Source/core/css/StylePropertySerializer.cpp (right):

https://codereview.chromium.org/1219643002/diff/90001/Source/core/css/StylePr...
Source/core/css/StylePropertySerializer.cpp:487: if
(m_propertySet.propertyAt(foundPropertyIndex).value()->isPrimitiveValue()
Maybe store the value as a temporary instead of having this long call chain here
and below

[meade_UTC10, 2015-06-30 08:26:47]

https://codereview.chromium.org/1219643002/diff/90001/Source/core/css/StylePr...
File Source/core/css/StylePropertySerializer.cpp (right):

https://codereview.chromium.org/1219643002/diff/90001/Source/core/css/StylePr...
Source/core/css/StylePropertySerializer.cpp:487: if
(m_propertySet.propertyAt(foundPropertyIndex).value()->isPrimitiveValue()
On 2015/06/30 at 08:11:42, Timothy Loh wrote:
> Maybe store the value as a temporary instead of having this long call chain
here and below

Done.

[davve, 2015-06-30 09:04:27]

fwiw, non-owner lgtm from me.

[meade_UTC10, 2015-07-01 03:10:07]

The CQ bit was checked by meade@chromium.org

[meade_UTC10, 2015-07-01 03:10:07]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org,
davve@opera.com
Link to the patchset: https://codereview.chromium.org/1219643002/#ps170001
(title: " ")

[commit-bot: I haz the power, 2015-07-01 03:10:18]

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

[commit-bot: I haz the power, 2015-07-01 04:01:07]

Committed patchset #10 (id:170001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198111