Implement the new text-size-adjust CSS property

Implement the new text-size-adjust CSS property

This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyKAQAJ

At a high level, this patch adds a new CSS property and plumbs it to
storage on the rare inherited style data. The new datatype for this is
"TextSizeAdjust" which just wraps a float with logic for storing both
a floating point adjustment value, and whether 'auto' should be used.
The text autosizer itself has been modified to read this new property.

A few changes were needed to allow the text autosizer to use
multipliers less than 1 (see: TextAutosizer::computeAutosizedFontSize
and callers to that function).

The simple-paragraph.html layouttest has been switched to a unit test
to prove the unit test infrastructure works, and many new tests have
been added for the text-size-adjust feature.

BUG=623158
Committed: https://crrev.com/5365b8ad85c2f5cee6034b8baea3c764ed78e1ea
Cr-Commit-Position: refs/heads/master@{#402916}

[pdr., 2016-06-27 03:38:13]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 03:38:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2016-06-27 04:31:57]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 04:32:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 06:01:59]

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

[commit-bot: I haz the power, 2016-06-27 06:02:00]

Dry run: This issue passed the CQ dry run.

[pdr., 2016-06-27 17:58:40]

Description was changed from

==========
Implement the new text-size-adjust CSS property

This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyK...

BUG=623158
==========

to

==========
Implement the new text-size-adjust CSS property

This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyK...

At a high level, this patch adds a new CSS property and plumbs it to
storage on the rare inherited style data. The new datatype for this is
"TextSizeAdjust" which just wraps a float with logic for storing both
a floating point adjustment value, and whether 'auto' should be used.
The text autosizer itself has been modified to read this new property.

The simple-paragraph.html layouttest has been switched to a unit test
to prove the unit test infrastructure works, and many new tests have
been added for the text-size-adjust feature.

BUG=623158
==========

[pdr., 2016-06-27 17:58:40]

pdr@chromium.org changed reviewers:
+ skobes@chromium.org, timloh@chromium.org

[pdr., 2016-06-27 17:59:49]

timloh, can you review the CSS property plumbing side of things?

skobes, can you review everything else?

[skobes, 2016-06-27 21:37:03]

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/TextAutosizer.cpp (right):

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/TextAutosizer.cpp:993: } else if
(multiplier < 1) {
Some callers of ComputedStyle::textAutosizingMultiplier() still check multiplier
> 1:

- FontBuilder::updateComputedSize
- FontBuilder::updateAdjustedSize
- ComputedStyle::lineHeight

I think those all need to be updated?

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/TextAutosizer.cpp:1197: // pleasantSize by
less and less, until huge fonts are not increased at all.
I'm not sure how much sense the linear backoff makes when the multiplier is < 1.
 Maybe we should apply it at all sizes in that case?

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp (right):

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp:10: class
TextAutosizerTest : public RenderingTest {
This test is awesome, very thorough coverage.

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1412:
desc.setComputedSize(std::max(std::min(maximumAllowedFontSize,
autosizedFontSize), 0.f));
I think TextAutosizer should avoid setting a multiplier < 0.  Then this can be
an ASSERT instead of using std::max.

[pdr., 2016-06-27 22:28:59]

Description was changed from

==========
Implement the new text-size-adjust CSS property

This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyK...

At a high level, this patch adds a new CSS property and plumbs it to
storage on the rare inherited style data. The new datatype for this is
"TextSizeAdjust" which just wraps a float with logic for storing both
a floating point adjustment value, and whether 'auto' should be used.
The text autosizer itself has been modified to read this new property.

The simple-paragraph.html layouttest has been switched to a unit test
to prove the unit test infrastructure works, and many new tests have
been added for the text-size-adjust feature.

BUG=623158
==========

to

==========
Implement the new text-size-adjust CSS property

This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyK...

At a high level, this patch adds a new CSS property and plumbs it to
storage on the rare inherited style data. The new datatype for this is
"TextSizeAdjust" which just wraps a float with logic for storing both
a floating point adjustment value, and whether 'auto' should be used.
The text autosizer itself has been modified to read this new property.

A few changes were needed to allow the text autosizer to use
multipliers less than 1 (see: TextAutosizer::computeAutosizedFontSize
and callers to that function).

The simple-paragraph.html layouttest has been switched to a unit test
to prove the unit test infrastructure works, and many new tests have
been added for the text-size-adjust feature.

BUG=623158
==========

[pdr., 2016-06-27 22:29:43]

Good catches! Code updated and change description updated to mention the
multiplier changes.

PTAL

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/TextAutosizer.cpp (right):

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/TextAutosizer.cpp:993: } else if
(multiplier < 1) {
On 2016/06/27 at 21:37:03, skobes wrote:
> Some callers of ComputedStyle::textAutosizingMultiplier() still check
multiplier > 1:
> 
> - FontBuilder::updateComputedSize
> - FontBuilder::updateAdjustedSize
> - ComputedStyle::lineHeight
> 
> I think those all need to be updated?

Good catch, done.

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2100013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1412:
desc.setComputedSize(std::max(std::min(maximumAllowedFontSize,
autosizedFontSize), 0.f));
On 2016/06/27 at 21:37:03, skobes wrote:
> I think TextAutosizer should avoid setting a multiplier < 0.  Then this can be
an ASSERT instead of using std::max.

Good idea.

I added a DCHECK in TextAutosizer::computeAutosizedFontSize and removed this
clamping.

[pdr., 2016-06-27 22:29:50]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 22:31:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[skobes, 2016-06-27 22:41:40]

lgtm

[pdr., 2016-06-27 22:43:49]

On 2016/06/27 at 22:41:40, skobes wrote:
> lgtm

Tanks!

timloh, can you review the CSS property plumbing side of things?

[commit-bot: I haz the power, 2016-06-28 00:25:57]

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

[commit-bot: I haz the power, 2016-06-28 00:25:58]

Dry run: This issue passed the CQ dry run.

[Timothy Loh, 2016-06-28 00:50:26]

css plumbing lgtm

https://codereview.chromium.org/2100013002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp
(right):

https://codereview.chromium.org/2100013002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp:943: if
(value.isPrimitiveValue()) {
I'd probably write

const CSSPrimitiveValue& primitiveValue = toCSSPrimitiveValue(value);
if (primitiveValue.getValueID() == CSSValueNone)
    return TextSizeAdjust::adjustNone();
if (primitiveValue.getValueID() == CSSValueAuto)
    return TextSizeAdjust::adjustAuto();
DCHECK(primitiveValue.isPercentage());
return TextSizeAdjust(primitiveValue.getFloatValue() / 100.0f);

[pdr., 2016-06-28 16:38:57]

Thanks all, off to the CQ

https://codereview.chromium.org/2100013002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp
(right):

https://codereview.chromium.org/2100013002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp:943: if
(value.isPrimitiveValue()) {
On 2016/06/28 at 00:50:25, Timothy Loh wrote:
> I'd probably write
> 
> const CSSPrimitiveValue& primitiveValue = toCSSPrimitiveValue(value);
> if (primitiveValue.getValueID() == CSSValueNone)
>     return TextSizeAdjust::adjustNone();
> if (primitiveValue.getValueID() == CSSValueAuto)
>     return TextSizeAdjust::adjustAuto();
> DCHECK(primitiveValue.isPercentage());
> return TextSizeAdjust(primitiveValue.getFloatValue() / 100.0f);

I wasn't sure this would work but it is better than mine where I was guarding
against non-percentage values unnecessarily. I've changed this to your
suggestion and added a test (TextAutosizerTest.TextSizeAdjustmentPixelUnits).

[pdr., 2016-06-28 16:45:06]

The CQ bit was checked by pdr@chromium.org

[pdr., 2016-06-28 16:45:07]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org,
skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/2100013002/#ps80001
(title: "Cleanup convertTextSizeAdjust and add a test that better covers it")

[commit-bot: I haz the power, 2016-06-28 16:45:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 16:55:41]

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

[commit-bot: I haz the power, 2016-06-28 16:55:43]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[pdr., 2016-06-28 16:57:42]

pdr@chromium.org changed reviewers:
+ chrishtr@chromium.org

[pdr., 2016-06-28 16:58:04]

+cc chrishtr for third_party/WebKit/LayoutTests/virtual/stable/webexposed
OWNERS.

Please CQ as well.

[pdr., 2016-06-29 17:03:42]

Ping chrishtr

[chrishtr, 2016-06-29 18:03:31]

lgtm

[pdr., 2016-06-29 18:17:45]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-06-29 18:18:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 20:06:56]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-29 20:07:33]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-29 20:09:58]

Description was changed from

==========
Implement the new text-size-adjust CSS property

This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyK...

At a high level, this patch adds a new CSS property and plumbs it to
storage on the rare inherited style data. The new datatype for this is
"TextSizeAdjust" which just wraps a float with logic for storing both
a floating point adjustment value, and whether 'auto' should be used.
The text autosizer itself has been modified to read this new property.

A few changes were needed to allow the text autosizer to use
multipliers less than 1 (see: TextAutosizer::computeAutosizedFontSize
and callers to that function).

The simple-paragraph.html layouttest has been switched to a unit test
to prove the unit test infrastructure works, and many new tests have
been added for the text-size-adjust feature.

BUG=623158
==========

to

==========
Implement the new text-size-adjust CSS property

This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyK...

At a high level, this patch adds a new CSS property and plumbs it to
storage on the rare inherited style data. The new datatype for this is
"TextSizeAdjust" which just wraps a float with logic for storing both
a floating point adjustment value, and whether 'auto' should be used.
The text autosizer itself has been modified to read this new property.

A few changes were needed to allow the text autosizer to use
multipliers less than 1 (see: TextAutosizer::computeAutosizedFontSize
and callers to that function).

The simple-paragraph.html layouttest has been switched to a unit test
to prove the unit test infrastructure works, and many new tests have
been added for the text-size-adjust feature.

BUG=623158

Committed: https://crrev.com/5365b8ad85c2f5cee6034b8baea3c764ed78e1ea
Cr-Commit-Position: refs/heads/master@{#402916}
==========

[commit-bot: I haz the power, 2016-06-29 20:10:00]

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

[ymalik, 2016-07-12 14:40:55]

ymalik@chromium.org changed reviewers:
+ ymalik@chromium.org

[ymalik, 2016-07-12 14:40:56]

https://codereview.chromium.org/2100013002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/2100013002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/UseCounter.cpp:567: case
CSSPropertyAliasWebkitTextSizeAdjust: return 537;
Did you miss running histograms.xml after? I was adding a use counter and
noticed update_use_counter_css.py add histograms for this. 

Fix here: https://codereview.chromium.org/2140173002/.