Fix FontCacheKey-keyed HashMap usage in core/css

Fix FontCacheKey-keyed HashMap usage in core/css

HashMap to cache fonts in core/css associates FontCacheKey, however they use
hashed value of FontCacheKey. That causes incorrect cache hit or assertion
failure on hash collision case or on 0 or -1 hash value.

BUG=611669
Committed: https://crrev.com/cf88b677e190c6b306b9721a267ab55c4e869e14
Cr-Commit-Position: refs/heads/master@{#393744}

[tzik, 2016-05-13 04:39:46]

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

[commit-bot: I haz the power, 2016-05-13 04:39:55]

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

[tzik, 2016-05-13 04:58:35]

Description was changed from

==========
Fix FontCacheKey-keyed HashMap usage in core/css

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

to

==========
Fix FontCacheKey-keyed HashMap usage in core/css

HashMap to cache fonts in core/css associates FontCacheKey, however they use
hashed value of FontCacheKey. That causes incorrect cache hit or assertion
failure on hash collision case or on 0 or -1 hash value.
==========

[tzik, 2016-05-13 04:58:36]

tzik@chromium.org changed reviewers:
+ eae@chromium.org, kochi@chromium.org, kojii@chromium.org, kouhei@chromium.org

[tzik, 2016-05-13 04:59:01]

PTAL

[kouhei (in TOK), 2016-05-13 05:00:53]

On 2016/05/13 04:59:01, tzik wrote:
> PTAL

Would you create a bug and possibly a layout test?

[kochi, 2016-05-13 05:02:44]

On 2016/05/13 04:59:01, tzik wrote:
> PTAL

Sorry, I'm not familiar with font caching code in Blink... so I'm not a
good reviewer for the code :)

It seems this is a fix for wrong caching cases, so if there is a description
about the problem in crbug.com or a reproduction case, it would be nice.
Is it difficult to write a test for this?

[commit-bot: I haz the power, 2016-05-13 06:06:30]

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

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

Dry run: This issue passed the CQ dry run.

[tzik, 2016-05-13 07:12:54]

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

[commit-bot: I haz the power, 2016-05-13 07:13:10]

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

[tzik, 2016-05-13 07:25:09]

Description was changed from

==========
Fix FontCacheKey-keyed HashMap usage in core/css

HashMap to cache fonts in core/css associates FontCacheKey, however they use
hashed value of FontCacheKey. That causes incorrect cache hit or assertion
failure on hash collision case or on 0 or -1 hash value.
==========

to

==========
Fix FontCacheKey-keyed HashMap usage in core/css

HashMap to cache fonts in core/css associates FontCacheKey, however they use
hashed value of FontCacheKey. That causes incorrect cache hit or assertion
failure on hash collision case or on 0 or -1 hash value.

BUG=611669
==========

[tzik, 2016-05-13 07:28:19]

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

[commit-bot: I haz the power, 2016-05-13 07:28:43]

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

[tzik, 2016-05-13 07:31:26]

Filed a bug, and added a test for CSSFontFaceSource fix. PS2 should fail on the
new test, and PS3 should fix it.
CSSSegmentedFontFace is relatively hard to set up the test, though the fix
itself is almost identical. Can I skip that part if you don't mind?

[drott, 2016-05-13 08:40:06]

Not a core owner, but LGTM for the change and test, thanks for fixing this!

[commit-bot: I haz the power, 2016-05-13 09:02:33]

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

[commit-bot: I haz the power, 2016-05-13 09:02:34]

Dry run: This issue passed the CQ dry run.

[kouhei (in TOK), 2016-05-13 09:06:40]

lgtm

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

https://codereview.chromium.org/1973153004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSFontFaceSourceTest.cpp:47:
DummyFontFaceSource fontFaceSource;
// Even if the hash value collide, fontface cache should return different value
for different fonts.

[tzik, 2016-05-14 11:57:15]

tzik@chromium.org changed reviewers:
+ drott@chromium.org

[tzik, 2016-05-14 11:57:16]

The CQ bit was checked by tzik@chromium.org

[tzik, 2016-05-14 11:57:16]

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

https://codereview.chromium.org/1973153004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSFontFaceSourceTest.cpp:47:
DummyFontFaceSource fontFaceSource;
On 2016/05/13 at 09:06:40, kouhei wrote:
> // Even if the hash value collide, fontface cache should return different
value for different fonts.

Done

[tzik, 2016-05-14 11:57:17]

The patchset sent to the CQ was uploaded after l-g-t-m from drott@chromium.org,
kouhei@chromium.org
Link to the patchset: https://codereview.chromium.org/1973153004/#ps60001
(title: "+comment")

[commit-bot: I haz the power, 2016-05-14 11:57:33]

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

[commit-bot: I haz the power, 2016-05-14 20:30:56]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-14 20:33:03]

Description was changed from

==========
Fix FontCacheKey-keyed HashMap usage in core/css

HashMap to cache fonts in core/css associates FontCacheKey, however they use
hashed value of FontCacheKey. That causes incorrect cache hit or assertion
failure on hash collision case or on 0 or -1 hash value.

BUG=611669
==========

to

==========
Fix FontCacheKey-keyed HashMap usage in core/css

HashMap to cache fonts in core/css associates FontCacheKey, however they use
hashed value of FontCacheKey. That causes incorrect cache hit or assertion
failure on hash collision case or on 0 or -1 hash value.

BUG=611669

Committed: https://crrev.com/cf88b677e190c6b306b9721a267ab55c4e869e14
Cr-Commit-Position: refs/heads/master@{#393744}
==========

[commit-bot: I haz the power, 2016-05-14 20:33:04]

Patchset 4 (id:??) landed as
https://crrev.com/cf88b677e190c6b306b9721a267ab55c4e869e14
Cr-Commit-Position: refs/heads/master@{#393744}

[eae, 2016-05-16 16:30:51]

Awesome, thank you!