Add CSS support for Containment

Add CSS support for Containment

This patch adds a RuntimeEnabledFeature (experimental, of course)
as well as the properties, keywords, parsing, and storing of
containment values.

Intent to implement:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/9W80Kw-z3ss
BUG=561713
Committed: https://crrev.com/8b73b08d57cf6aa217e050144947e899ff40c3aa
Cr-Commit-Position: refs/heads/master@{#363178}

[leviw_travelin_and_unemployed, 2015-12-02 00:03:36]

leviw@chromium.org changed reviewers:
+ alancutter@chromium.org, eae@chromium.org, ojan@chromium.org,
timloh@chromium.org

[leviw_travelin_and_unemployed, 2015-12-02 00:03:37]

Round 1!

[alancutter (OOO until 2018), 2015-12-02 02:48:17]

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/containment/contain-parse-and-apply.html
(right):

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/containment/contain-parse-and-apply.html:7:
#strict { contain: strict; }
There should be tests that use combinations of style/layout/paint keywords
including invalid combinations like "layout layout layout".

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyleConstants.h (right):

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyleConstants.h:494:
ContainsStrict = 0x7
I wonder if we're allowed to use 0b111.

[Timothy Loh, 2015-12-02 02:49:48]

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/LegacyCSSPropertyParser.cpp
(right):

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/LegacyCSSPropertyParser.cpp:1476:
PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseContain()
This parsing code is wrong (accepts "strict layout", "layout layout layout
layout", etc.). We should do this in the non-legacy file
(CSSPropertyParser.cpp), it's similar to consumeTextDecorationLine. Also there
should be tests for combinations of layout/style/paint, since it doesn't look
like the getComputedStyle code works for it.

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyleConstants.h (right):

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyleConstants.h:488: static const
size_t EContainBits = 3;
I don't think the E prefix is the current style.

[leviw_travelin_and_unemployed, 2015-12-03 00:02:00]

PTAL

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/containment/contain-parse-and-apply.html
(right):

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/containment/contain-parse-and-apply.html:7:
#strict { contain: strict; }
On 2015/12/02 at 02:48:17, alancutter wrote:
> There should be tests that use combinations of style/layout/paint keywords
including invalid combinations like "layout layout layout".

Added a bunch of cases below.

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/LegacyCSSPropertyParser.cpp
(right):

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/LegacyCSSPropertyParser.cpp:1476:
PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseContain()
On 2015/12/02 at 02:49:47, Timothy Loh wrote:
> This parsing code is wrong (accepts "strict layout", "layout layout layout
layout", etc.). We should do this in the non-legacy file
(CSSPropertyParser.cpp), it's similar to consumeTextDecorationLine. Also there
should be tests for combinations of layout/style/paint, since it doesn't look
like the getComputedStyle code works for it.

Changed and moved.

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyleConstants.h (right):

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyleConstants.h:488: static const
size_t EContainBits = 3;
On 2015/12/02 at 02:49:47, Timothy Loh wrote:
> I don't think the E prefix is the current style.

Changed. The style code is sadly quite inconsistent on this particular issue :(

https://codereview.chromium.org/1485973005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyleConstants.h:494:
ContainsStrict = 0x7
On 2015/12/02 at 02:48:17, alancutter wrote:
> I wonder if we're allowed to use 0b111.

I didn't try, but I changed it to ContainsStrict = ContainsLayout |
ContainsStyle | ContainsPaint, which I think is better.

[Timothy Loh, 2015-12-03 05:23:45]

lgtm

https://codereview.chromium.org/1485973005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1485973005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1787:
RefPtrWillBeRawPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
Can we just handle strict separately? e.g.

if (id == CSSValueStrict) {
   list->append(consumeIdent(range))
   return list.release();
}

[eae, 2015-12-03 05:31:06]

LGTM

[leviw_travelin_and_unemployed, 2015-12-03 22:37:53]

On 2015/12/03 at 05:23:45, timloh wrote:
> lgtm
> 
>
https://codereview.chromium.org/1485973005/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):
> 
>
https://codereview.chromium.org/1485973005/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1787:
RefPtrWillBeRawPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
> Can we just handle strict separately? e.g.
> 
> if (id == CSSValueStrict) {
>    list->append(consumeIdent(range))
>    return list.release();
> }

Done. Originally the special handling for None in
StyleBuilderConverter::convertFlags tripped me up, but we could alternatively
have something like convertLineWidth uses if it was important to avoid the list.
Anyways, this is a better solution.

[leviw_travelin_and_unemployed, 2015-12-03 22:37:55]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-12-03 22:37:55]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org,
eae@chromium.org
Link to the patchset: https://codereview.chromium.org/1485973005/#ps60001
(title: "Address comments")

[commit-bot: I haz the power, 2015-12-03 22:39:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1485973005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1485973005/60001

[commit-bot: I haz the power, 2015-12-04 00:39:22]

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

[commit-bot: I haz the power, 2015-12-04 00:39:23]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[leviw_travelin_and_unemployed, 2015-12-04 07:18:44]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-12-04 07:18:45]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org,
eae@chromium.org
Link to the patchset: https://codereview.chromium.org/1485973005/#ps80001
(title: "Add test expectation")

[commit-bot: I haz the power, 2015-12-04 07:18:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1485973005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1485973005/80001

[commit-bot: I haz the power, 2015-12-04 09:52:11]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-04 09:53:11]

Description was changed from

==========
Add CSS support for Containment

This patch adds a RuntimeEnabledFeature (experimental, of course)
as well as the properties, keywords, parsing, and storing of
containment values.

Intent to implement:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/9W80Kw-z3ss
BUG=561713
==========

to

==========
Add CSS support for Containment

This patch adds a RuntimeEnabledFeature (experimental, of course)
as well as the properties, keywords, parsing, and storing of
containment values.

Intent to implement:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/9W80Kw-z3ss
BUG=561713

Committed: https://crrev.com/8b73b08d57cf6aa217e050144947e899ff40c3aa
Cr-Commit-Position: refs/heads/master@{#363178}
==========

[commit-bot: I haz the power, 2015-12-04 09:53:12]

Patchset 5 (id:??) landed as
https://crrev.com/8b73b08d57cf6aa217e050144947e899ff40c3aa
Cr-Commit-Position: refs/heads/master@{#363178}