Add CSS parsing support for the scroll-blocks-on property (in place of touch-action-delay)

Add CSS parsing support for the scroll-blocks-on property

Replaces the old (not-yet-used) CSS parsing support for
"touch-action-delay" with its successor "scroll-blocks-on."  Still
experimental and not yet wired up to anything.

Design-doc: https://docs.google.com/a/chromium.org/document/d/1aOQRw76C0enLBd0mCG_-IM6bso7DxXwvqTiRWgNdTn8/edit
Intent to implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/N3YrsrHus-k

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

[Rick Byers, 2014-11-14 21:45:25]

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

[Rick Byers, 2014-11-14 21:45:25]

Hey Tim,
It looks like you're probably a good person to review CSS parsing changes these
days, right?

Thanks,
   Rick

[Rick Byers, 2014-11-14 21:46:13]

rbyers@chromium.org changed reviewers:
+ vollick@chromium.org

[Rick Byers, 2014-11-14 21:46:42]

On 2014/11/14 21:45:25, Rick Byers wrote:
> Hey Tim,
> It looks like you're probably a good person to review CSS parsing changes
these
> days, right?
> 
> Thanks,
>    Rick

+vollick for OWNERS in Source/platform (trivial change to
RuntimeEnabledFeatures).

[Ian Vollick, 2014-11-14 21:47:30]

On 2014/11/14 21:46:42, Rick Byers wrote:
> On 2014/11/14 21:45:25, Rick Byers wrote:
> > Hey Tim,
> > It looks like you're probably a good person to review CSS parsing changes
> these
> > days, right?
> > 
> > Thanks,
> >    Rick
> 
> +vollick for OWNERS in Source/platform (trivial change to
> RuntimeEnabledFeatures).

lgtm

[Timothy Loh, 2014-11-14 22:06:41]

lgtm

https://codereview.chromium.org/723373006/diff/1/Source/core/css/CSSComputedS...
File Source/core/css/CSSComputedStyleDeclaration.cpp (right):

https://codereview.chromium.org/723373006/diff/1/Source/core/css/CSSComputedS...
Source/core/css/CSSComputedStyleDeclaration.cpp:1481: ASSERT(scrollBlocksOn ==
ScrollBlocksOnNone);
This might look cleaner if you tested this case first, and then just ASSERT
we're not empty here.

https://codereview.chromium.org/723373006/diff/1/Source/core/css/parser/CSSPr...
File Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/723373006/diff/1/Source/core/css/parser/CSSPr...
Source/core/css/parser/CSSPropertyParser.cpp:7396: if (m_valueList->size() == 1
&& value && (value->id == CSSValueNone)) {
I'm pretty sure we don't need to check the size (checked after the gigantic
switch) or that value is non-null (checked before the gigantic switch). Also I
think we can just return a CSSPrimitiveValue in this case instead of wrapping in
a list.

https://codereview.chromium.org/723373006/diff/1/Source/core/css/parser/CSSPr...
Source/core/css/parser/CSSPropertyParser.cpp:7419: if (list->length())
Since value is never null when we get into this function, the list will never be
empty. Maybe better to ASSERT we're not empty.

https://codereview.chromium.org/723373006/diff/1/Source/core/frame/UseCounter...
File Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/723373006/diff/1/Source/core/frame/UseCounter...
Source/core/frame/UseCounter.cpp:487: case CSSPropertyScrollBlocksOn: return
442;
Probably best to have a new return value here, even if touch-action-delay was
only enabled in tests.

[Rick Byers, 2014-11-18 03:03:08]

Thanks Tim!

https://codereview.chromium.org/723373006/diff/1/Source/core/css/CSSComputedS...
File Source/core/css/CSSComputedStyleDeclaration.cpp (right):

https://codereview.chromium.org/723373006/diff/1/Source/core/css/CSSComputedS...
Source/core/css/CSSComputedStyleDeclaration.cpp:1481: ASSERT(scrollBlocksOn ==
ScrollBlocksOnNone);
On 2014/11/14 22:06:41, Timothy Loh (OOO until 24th) wrote:
> This might look cleaner if you tested this case first, and then just ASSERT
> we're not empty here.

Done.

https://codereview.chromium.org/723373006/diff/1/Source/core/css/parser/CSSPr...
File Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/723373006/diff/1/Source/core/css/parser/CSSPr...
Source/core/css/parser/CSSPropertyParser.cpp:7396: if (m_valueList->size() == 1
&& value && (value->id == CSSValueNone)) {
On 2014/11/14 22:06:41, Timothy Loh (OOO until 24th) wrote:
> I'm pretty sure we don't need to check the size (checked after the gigantic
> switch) or that value is non-null (checked before the gigantic switch). Also I
> think we can just return a CSSPrimitiveValue in this case instead of wrapping
in
> a list.

Done.

https://codereview.chromium.org/723373006/diff/1/Source/core/css/parser/CSSPr...
Source/core/css/parser/CSSPropertyParser.cpp:7419: if (list->length())
On 2014/11/14 22:06:41, Timothy Loh (OOO until 24th) wrote:
> Since value is never null when we get into this function, the list will never
be
> empty. Maybe better to ASSERT we're not empty.

Done.

https://codereview.chromium.org/723373006/diff/1/Source/core/frame/UseCounter...
File Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/723373006/diff/1/Source/core/frame/UseCounter...
Source/core/frame/UseCounter.cpp:487: case CSSPropertyScrollBlocksOn: return
442;
On 2014/11/14 22:06:41, Timothy Loh (OOO until 24th) wrote:
> Probably best to have a new return value here, even if touch-action-delay was
> only enabled in tests.

Ok, done.  scroll-blocks-on is really just a rename of touch-action-delay (in
addition to becoming more general), but doesn't hurt.  I expect the name may
change again before shipping.  In general we don't need to retain old values
when we just rename, right?

[Rick Byers, 2014-11-18 03:03:28]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-11-18 03:04:09]

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

[commit-bot: I haz the power, 2014-11-18 04:15:47]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185495