Empty @viewport should not override legacy viewport.

Empty @viewport should not override legacy viewport.

@viewport styles should not be considered present in author styles if there is
not a single descriptor present. This is not specified anywhere, though.

Marking the ViewportStyleResolver with author styles present when there were
no descriptors caused an assert to trigger (see mentioned BUG). This is fixed
by always require at least one descriptor to be present in order to set the
flag for author style presence, keeping the assert.

As part of this fix, collectViewportRules was refactored into
ViewportStyleResolver to resolve a FIXME in the StyleResolver.

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

[apavlov, 2013-09-30 13:04:44]

lgtm

Thanks for fixing!

[kenneth.r.christiansen, 2013-09-30 13:06:08]

https://codereview.chromium.org/25257003/diff/1/LayoutTests/fast/viewport/vie...
File LayoutTests/fast/viewport/viewport-legacy-ordering-10-expected.txt (right):

https://codereview.chromium.org/25257003/diff/1/LayoutTests/fast/viewport/vie...
LayoutTests/fast/viewport/viewport-legacy-ordering-10-expected.txt:2: PASS Check
that Meta viewport is not overridden by empty @viewport rule.
is an invalid @viewport rule also considered empty?

[rune, 2013-09-30 13:12:11]

On 2013/09/30 13:06:08, kenneth.r.christiansen wrote:
>
https://codereview.chromium.org/25257003/diff/1/LayoutTests/fast/viewport/vie...
> File LayoutTests/fast/viewport/viewport-legacy-ordering-10-expected.txt
(right):
> 
>
https://codereview.chromium.org/25257003/diff/1/LayoutTests/fast/viewport/vie...
> LayoutTests/fast/viewport/viewport-legacy-ordering-10-expected.txt:2: PASS
Check
> that Meta viewport is not overridden by empty @viewport rule.
> is an invalid @viewport rule also considered empty?

Yes, if there is no descriptor with a valid value present, the @viewport rule is
also effectively empty. If there is a syntax error in the "prelude" (like
"@viewport blah {}"), there's not even an @viewport rule present.

Do you think there should be a different rule for priority here?

[kenneth.r.christiansen, 2013-09-30 13:14:05]

If someone added a @viewport and they made a mistake, they might think it is
working because the viewport meta is taking over.

I am not sure what is the best solution. Do we report any console messages in
this case?

> Do you think there should be a different rule for priority here?

[rune, 2013-09-30 13:30:45]

On 2013/09/30 13:14:05, kenneth.r.christiansen wrote:
> If someone added a @viewport and they made a mistake, they might think it is
> working because the viewport meta is taking over.
> 
> I am not sure what is the best solution. Do we report any console messages in
> this case?

Per-descriptor warnings as you get for CSS property declarations.

[commit-bot: I haz the power, 2013-10-01 10:30:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/25257003/1

[commit-bot: I haz the power, 2013-10-01 11:25:44]

Change committed as 158625