Unicode Variation Selectors support in collectCandidateRuns

Unicode Variation Selectors support in collectCandidateRuns

This patch supports Unicode Variation Selectors in collectCandidateRuns.

This change alone does not fix the SVS/IVS bug crbug.com/383580. It has
to wait for another change to read CMAP 14 to get the correct glyphs
for SVS/IVS.

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

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

[kojii, 2015-05-20 16:50:35]

kojii@chromium.org changed reviewers:
+ bashi@chromium.org, dominik.rottsches@intel.com, eae@chromium.org,
ksakamoto@chromium.org

[kojii, 2015-05-20 16:50:35]

PTAL.

[Kunihiko Sakamoto, 2015-05-25 01:38:26]

non-owner lgtm.

[eae, 2015-05-26 17:45:29]

I'd like to see a test for this.

[kojii, 2015-05-27 01:07:17]

On 2015/05/26 17:45:29, eae wrote:
> I'd like to see a test for this.

There's a test for UVS bashi@ created: unicode-variation-selector.html, which
has a failure expectation image today.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

I confirmed by applying 3 patches (this, hb-ot-font, and the other one) this
test needs rebaseline and passes.
hb-ot-font: https://codereview.chromium.org/1143213003/
the other one: https://codereview.chromium.org/1149693002/

Can I confirm, are you suggesting more unit-test-like ones using gtest just for 
isUnicodeVariationSelector?

[eae, 2015-05-27 16:26:25]

On 2015/05/27 01:07:17, koji wrote:
> On 2015/05/26 17:45:29, eae wrote:
> > I'd like to see a test for this.
> 
> There's a test for UVS bashi@ created: unicode-variation-selector.html, which
> has a failure expectation image today.
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> I confirmed by applying 3 patches (this, hb-ot-font, and the other one) this
> test needs rebaseline and passes.
> hb-ot-font: https://codereview.chromium.org/1143213003/
> the other one: https://codereview.chromium.org/1149693002/
> 
> Can I confirm, are you suggesting more unit-test-like ones using gtest just
for 
> isUnicodeVariationSelector?

A unit test for that would be great, see HarfBuzzShaperTest.cpp for examples of
how our font code can be tested.

[kojii, 2015-05-28 18:10:48]

PTAL. Tests added, and a good finding is that this fix is really needed; we
split UVS in 2 of 5 cases without this fix. Updated the description accordingly.

[eae, 2015-05-28 18:13:01]

LGTM

[kojii, 2015-05-28 23:33:54]

The CQ bit was checked by kojii@chromium.org

[kojii, 2015-05-28 23:33:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
ksakamoto@chromium.org
Link to the patchset: https://codereview.chromium.org/1144033004/#ps60001
(title: "Fix compile warning on Linux/Mac")

[commit-bot: I haz the power, 2015-05-28 23:34:09]

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

[commit-bot: I haz the power, 2015-05-28 23:37:38]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196095

[dmurph, 2015-05-29 02:16:57]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1156763003/ by dmurph@chromium.org.

The reason for reverting is: Causing failures, tracking here:
https://code.google.com/p/chromium/issues/detail?id=493512.

[kojii, 2015-05-29 13:22:19]

I can't repro. I found linux_chromium_trusty32_rel bot but it passed.

4 runs against 3 characters look really weird. I'm suspecting wcslen() doing
something wrong, googling hits Ubuntu bug in wcslen(), maybe we're hitting this.

[kojii, 2015-05-29 13:47:26]

Contacted trooper.

[kojii, 2015-06-04 06:35:48]

The CQ bit was checked by kojii@chromium.org

[kojii, 2015-06-04 06:35:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
ksakamoto@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/1144033004/#ps80001
(title: "Avoid using wcslen() in tests")

[commit-bot: I haz the power, 2015-06-04 06:36:07]

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

[kojii, 2015-06-04 06:38:24]

After discussing with tkent@, I'll go to try to land, and see if the build bot
complains.

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

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196485

[kojii, 2015-06-04 09:32:12]

Looks like passed this time.
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%2032/buil...

# didn't know wcslen() is that unstable...