Expand system font values during property parsing.

Expand system font values during property parsing.

This is alancutter's patch, adjusted to work with the current master.

This patch removes special case handling of the font shorthand property.
Previously this property would be carried through to the style resolving
stage if system font values were used. This patch refactors this process
to happen in the CSS parser so that font is always expanded into longhand
values.

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

[andersr, 2014-10-24 19:00:01]

Patchset #3 (id:40001) has been deleted

[andersr, 2014-10-24 19:00:06]

Patchset #2 (id:20001) has been deleted

[andersr, 2014-10-25 13:46:10]

andersr@opera.com changed reviewers:
+ alancutter@chromium.org, timloh@chromium.org

[andersr, 2014-10-25 13:59:32]

Original patch: https://codereview.chromium.org/205743004/

I figured I should try to reproduce the problem with this patch related to
Mac::systemFont() calls in WebWorkers, but I don't get any fails on the trybots.

How was the problem discovered last time?

[alancutter (OOO until 2018), 2014-10-26 23:40:20]

lgtm

Thanks for reviving this change!
Previously all the web worker layout tests were failing only on the Mac trybots,
if they pass now then that's a green light for this patch to go ahead. :)

https://codereview.chromium.org/677103002/diff/60001/Source/core/rendering/Re...
File Source/core/rendering/RenderTheme.cpp (right):

https://codereview.chromium.org/677103002/diff/60001/Source/core/rendering/Re...
Source/core/rendering/RenderTheme.cpp:974: case CSSValueNone:
When would this case be reached?

[Timothy Loh, 2014-10-27 05:45:46]

If it works, then hooray lgtm :), remember to remove [wip] from the
subject/description though

https://codereview.chromium.org/677103002/diff/60001/Source/core/css/StylePro...
File Source/core/css/StylePropertyShorthandCustom.cpp (right):

https://codereview.chromium.org/677103002/diff/60001/Source/core/css/StylePro...
Source/core/css/StylePropertyShorthandCustom.cpp:138: bool
isExpandedShorthandForAll(CSSPropertyID propertyId)
I think we can remove this function and just call isExpandedShorthand. Probably
you'll want to wait for https://codereview.chromium.org/339713008/ to land and
rebase over.

https://codereview.chromium.org/677103002/diff/60001/Source/core/rendering/Re...
File Source/core/rendering/RenderTheme.cpp (right):

https://codereview.chromium.org/677103002/diff/60001/Source/core/rendering/Re...
Source/core/rendering/RenderTheme.cpp:974: case CSSValueNone:
On 2014/10/26 23:40:19, alancutter wrote:
> When would this case be reached?

I think only in DragContoller.cpp/dragImageForLink. Seems a bit dodgy, but
probably OK.

[andersr, 2014-10-27 10:58:30]

On 2014/10/26 23:40:20, alancutter wrote:
> lgtm
> 
> Thanks for reviving this change!
> Previously all the web worker layout tests were failing only on the Mac
trybots,
> if they pass now then that's a green light for this patch to go ahead. :)

A land-and-pray, eh? This patch is going to be reverted so hard.

You say "all" web worker tests failed on Mac, I guess that means it reproduced
consistently on the trybots? (I'm contemplating whether I should try to delete a
system font NSFont would return, and see what happens).

https://codereview.chromium.org/677103002/diff/60001/Source/core/css/StylePro...
File Source/core/css/StylePropertyShorthandCustom.cpp (right):

https://codereview.chromium.org/677103002/diff/60001/Source/core/css/StylePro...
Source/core/css/StylePropertyShorthandCustom.cpp:138: bool
isExpandedShorthandForAll(CSSPropertyID propertyId)
On 2014/10/27 05:45:46, Timothy Loh wrote:
> I think we can remove this function and just call isExpandedShorthand.
Probably
> you'll want to wait for https://codereview.chromium.org/339713008/ to land and
> rebase over.

Done.

https://codereview.chromium.org/677103002/diff/60001/Source/core/rendering/Re...
File Source/core/rendering/RenderTheme.cpp (right):

https://codereview.chromium.org/677103002/diff/60001/Source/core/rendering/Re...
Source/core/rendering/RenderTheme.cpp:974: case CSSValueNone:
On 2014/10/27 05:45:46, Timothy Loh wrote:
> On 2014/10/26 23:40:19, alancutter wrote:
> > When would this case be reached?
> 
> I think only in DragContoller.cpp/dragImageForLink. Seems a bit dodgy, but
> probably OK.

Yes, that's the only place. By letting CSSValueNone mean "default system font"
here, that code should work approximately like before.

[andersr, 2014-10-27 10:59:00]

The CQ bit was checked by andersr@opera.com

[commit-bot: I haz the power, 2014-10-27 10:59:34]

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

[commit-bot: I haz the power, 2014-10-27 13:18:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-27 13:18:02]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/33738)

[andersr, 2014-10-27 13:31:37]

The CQ bit was checked by andersr@opera.com

[commit-bot: I haz the power, 2014-10-27 13:32:21]

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

[commit-bot: I haz the power, 2014-10-27 14:13:03]

Committed patchset #4 (id:100001) as 184449