Shape unicode-range: font faces in only one iteration

Shape unicode-range: font faces in only one iteration

Previously, we incorrectly split CSS composite faces into multiple faces
for shaping, one for each comma-separated entry of unicode-range:. This
breaks shaping of ligatures and other features when the characters that
ought to be shaped combined were in different unicode-range entries. It
is also inefficient for subsetted web fonts that use unicode-range:
extensively, for example Google Fonts and Adobe TypeKit.

The fix is to transfer the UnicodeRangeSet information from CSSFontFace
to HarfBuzzFace and only restrict the glyph lookup function to the whole
unicode-range information, instead of restricting it to a single entry
and shaping multiple times with the same face. This should have a slight
performance benefit as well.

BUG=583450
TEST=fast/css/font-face-unicode-range-ligatures.html
R=eae, behdad

Committed: https://crrev.com/9694005f93116f9c9cc73fa99132fa4475a0cdab
Cr-Commit-Position: refs/heads/master@{#381683}

[drott, 2016-03-16 08:41:57]

cleanup, fixing GlyphPageTreeNode tests, test added

[drott, 2016-03-16 10:47:24]

Temporary lifecycle issue fix

[drott, 2016-03-16 19:00:50]

Moving UnicodeRangeSet to RefPtr for passing it between CSSFontFace and
HarfBuzzFace

[drott, 2016-03-16 20:28:36]

Description was changed from

==========
WiP: Shaping across unicode-range entries

BUG=
==========

to

==========
Previously, we incorrectly split CSS composite faces into multiple faces
for shaping, one for each comma-separated entry of unicode-range:. This
breaks shaping of ligatures and other features when the characters that
ought to be shaped combined were in different unicode-range entries. It
is also inefficient for subsetted web fonts that use unicode-range:
extensively, for example Google Fonts and Adobe TypeKit.

The fix is to transfer the UnicodeRangeSet information from CSSFontFace
to HarfBuzzFace and only restrict the glyph lookup function to the whole
unicode-range information, instead of restricting it to a single entry
and shaping multiple times with the same face. This should have a slight
performance benefit as well.

BUG=583450
TEST=fast/css/font-face-unicode-range-ligatures.html
R=eae, behdad
==========

[drott, 2016-03-16 20:28:36]

drott@chromium.org changed reviewers:
+ behdad@chromium.org, eae@chromium.org

[drott, 2016-03-16 20:29:01]

Description was changed from

==========
Previously, we incorrectly split CSS composite faces into multiple faces
for shaping, one for each comma-separated entry of unicode-range:. This
breaks shaping of ligatures and other features when the characters that
ought to be shaped combined were in different unicode-range entries. It
is also inefficient for subsetted web fonts that use unicode-range:
extensively, for example Google Fonts and Adobe TypeKit.

The fix is to transfer the UnicodeRangeSet information from CSSFontFace
to HarfBuzzFace and only restrict the glyph lookup function to the whole
unicode-range information, instead of restricting it to a single entry
and shaping multiple times with the same face. This should have a slight
performance benefit as well.

BUG=583450
TEST=fast/css/font-face-unicode-range-ligatures.html
R=eae, behdad
==========

to

==========
Shape unicode-range: font faces in only one iteration

Previously, we incorrectly split CSS composite faces into multiple faces
for shaping, one for each comma-separated entry of unicode-range:. This
breaks shaping of ligatures and other features when the characters that
ought to be shaped combined were in different unicode-range entries. It
is also inefficient for subsetted web fonts that use unicode-range:
extensively, for example Google Fonts and Adobe TypeKit.

The fix is to transfer the UnicodeRangeSet information from CSSFontFace
to HarfBuzzFace and only restrict the glyph lookup function to the whole
unicode-range information, instead of restricting it to a single entry
and shaping multiple times with the same face. This should have a slight
performance benefit as well.

BUG=583450
TEST=fast/css/font-face-unicode-range-ligatures.html
R=eae, behdad
==========

[drott, 2016-03-16 20:30:06]

Depends on https://codereview.chromium.org/1808853002, but this was green before
and is now ready for review.

[drott, 2016-03-16 21:53:22]

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

[commit-bot: I haz the power, 2016-03-16 21:53:49]

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

[commit-bot: I haz the power, 2016-03-16 21:56:26]

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

[commit-bot: I haz the power, 2016-03-16 21:56:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[eae, 2016-03-17 00:12:37]

LGTM

[behdad, 2016-03-17 00:57:22]

LGTM.  Thanks!

[drott, 2016-03-17 06:32:01]

The CQ bit was checked by drott@chromium.org

[drott, 2016-03-17 06:32:02]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
behdad@chromium.org
Link to the patchset: https://codereview.chromium.org/1806653002/#ps80001
(title: "Rebased on UnicodeRangeSet move")

[commit-bot: I haz the power, 2016-03-17 06:32:07]

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

[commit-bot: I haz the power, 2016-03-17 07:33:46]

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

[commit-bot: I haz the power, 2016-03-17 07:33:47]

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

[drott, 2016-03-17 08:53:17]

Update UnicodeRangeSetTests to RefPtr, remove copy test, and fix test
expectation

[drott, 2016-03-17 08:54:07]

Patchset #6 (id:100001) has been deleted

[drott, 2016-03-17 08:55:11]

Update UnicodeRangeSetTests to RefPtr, remove copy test, and fix test
expectation

[drott, 2016-03-17 08:55:33]

The CQ bit was checked by drott@chromium.org

[drott, 2016-03-17 08:55:34]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
behdad@chromium.org
Link to the patchset: https://codereview.chromium.org/1806653002/#ps120001
(title: "Update UnicodeRangeSetTests to RefPtr, remove copy test, and fix test
expectation")

[commit-bot: I haz the power, 2016-03-17 08:55:41]

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

[commit-bot: I haz the power, 2016-03-17 08:55:43]

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

[commit-bot: I haz the power, 2016-03-17 08:55:44]

CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true
and NOPRESUBMIT=true in order for the CQ to process them

[drott, 2016-03-17 08:56:02]

Patchset #6 (id:120001) has been deleted

[drott, 2016-03-17 08:58:00]

Update UnicodeRangeSetTests to RefPtr, remove copy test, and fix test
expectation

[drott, 2016-03-17 08:58:14]

The CQ bit was checked by drott@chromium.org

[drott, 2016-03-17 08:58:14]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
behdad@chromium.org
Link to the patchset: https://codereview.chromium.org/1806653002/#ps140001
(title: "Update UnicodeRangeSetTests to RefPtr, remove copy test, and fix test
expectation")

[commit-bot: I haz the power, 2016-03-17 08:58:21]

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

[commit-bot: I haz the power, 2016-03-17 08:58:23]

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

[commit-bot: I haz the power, 2016-03-17 08:58:25]

CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true
and NOPRESUBMIT=true in order for the CQ to process them

[drott, 2016-03-17 09:00:26]

The CQ bit was unchecked by drott@chromium.org

[drott, 2016-03-17 09:07:39]

Update UnicodeRangeSetTests to RefPtrtr, rm copy constructor and test

[drott, 2016-03-17 09:08:13]

eae@, I plan to land the latest test fix TBR, perhaps you can take a look at the
UnicodeRangeTest file. I changed it to use RefPtr's and remove the copy test and
copy constructor from UnicodeRangeSet. This approach was from an earlier version
of this CL, where I copied the UnicodeRangeSets instead of passing them as
RefPtrs.

[drott, 2016-03-17 09:08:34]

The CQ bit was checked by drott@chromium.org

[drott, 2016-03-17 09:08:34]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
behdad@chromium.org
Link to the patchset: https://codereview.chromium.org/1806653002/#ps160001
(title: "Update UnicodeRangeSetTests to RefPtrtr, rm copy constructor and test")

[commit-bot: I haz the power, 2016-03-17 09:08:45]

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

[commit-bot: I haz the power, 2016-03-17 09:08:47]

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

[commit-bot: I haz the power, 2016-03-17 09:08:49]

CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true
and NOPRESUBMIT=true in order for the CQ to process them

[drott, 2016-03-17 09:10:10]

Patchset #7 (id:160001) has been deleted

[drott, 2016-03-17 09:10:47]

Update UnicodeRangeSetTests to RefPtrtr, rm copy constructor and test

[drott, 2016-03-17 09:11:08]

The CQ bit was checked by drott@chromium.org

[drott, 2016-03-17 09:11:08]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
behdad@chromium.org
Link to the patchset: https://codereview.chromium.org/1806653002/#ps180001
(title: "Update UnicodeRangeSetTests to RefPtrtr, rm copy constructor and test")

[commit-bot: I haz the power, 2016-03-17 09:11:16]

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

[commit-bot: I haz the power, 2016-03-17 10:17:08]

Committed patchset #7 (id:180001)

[commit-bot: I haz the power, 2016-03-17 10:18:07]

Description was changed from

==========
Shape unicode-range: font faces in only one iteration

Previously, we incorrectly split CSS composite faces into multiple faces
for shaping, one for each comma-separated entry of unicode-range:. This
breaks shaping of ligatures and other features when the characters that
ought to be shaped combined were in different unicode-range entries. It
is also inefficient for subsetted web fonts that use unicode-range:
extensively, for example Google Fonts and Adobe TypeKit.

The fix is to transfer the UnicodeRangeSet information from CSSFontFace
to HarfBuzzFace and only restrict the glyph lookup function to the whole
unicode-range information, instead of restricting it to a single entry
and shaping multiple times with the same face. This should have a slight
performance benefit as well.

BUG=583450
TEST=fast/css/font-face-unicode-range-ligatures.html
R=eae, behdad
==========

to

==========
Shape unicode-range: font faces in only one iteration

Previously, we incorrectly split CSS composite faces into multiple faces
for shaping, one for each comma-separated entry of unicode-range:. This
breaks shaping of ligatures and other features when the characters that
ought to be shaped combined were in different unicode-range entries. It
is also inefficient for subsetted web fonts that use unicode-range:
extensively, for example Google Fonts and Adobe TypeKit.

The fix is to transfer the UnicodeRangeSet information from CSSFontFace
to HarfBuzzFace and only restrict the glyph lookup function to the whole
unicode-range information, instead of restricting it to a single entry
and shaping multiple times with the same face. This should have a slight
performance benefit as well.

BUG=583450
TEST=fast/css/font-face-unicode-range-ligatures.html
R=eae, behdad

Committed: https://crrev.com/9694005f93116f9c9cc73fa99132fa4475a0cdab
Cr-Commit-Position: refs/heads/master@{#381683}
==========

[commit-bot: I haz the power, 2016-03-17 10:18:09]

Patchset 7 (id:??) landed as
https://crrev.com/9694005f93116f9c9cc73fa99132fa4475a0cdab
Cr-Commit-Position: refs/heads/master@{#381683}

[fgorski, 2016-03-17 17:16:48]

A revert of this CL (patchset #7 id:180001) has been created in
https://codereview.chromium.org/1806363002/ by fgorski@chromium.org.

The reason for reverting is: Reverting on a suspicion that this is a root cause
to the failures of webkit_unit_tests on Webkit Android (Nexus4) bots, e.g.
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Android%20%28N....