Commit b529f87d authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

Font::ShouldSkipDrawing() should use valid FontFallbackList

When two text nodes are shaped together (e.g., <span>foo</span><span>bar</span>),
we only use the first node's Font. If they have different Font objects,
then the FontFallbackList on the second node becomes stale.

This patch makes sure that when calling Font::ShouldSkipDrawing() on the
second node, we always use the up-to-date FontFallbackList, so that we
don't hit a DCHECK about stale FontFallbackList.

Bug: 1050564
Change-Id: I92db1875faebb7311a179d70512b413b43f408aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2231905
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarDominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775738}
parent 6faf39f5
......@@ -237,10 +237,14 @@ class PLATFORM_EXPORT Font {
bool IsFallbackValid() const;
bool ShouldSkipDrawing() const {
return font_fallback_list_ && font_fallback_list_->ShouldSkipDrawing();
if (!font_fallback_list_)
return false;
return EnsureFontFallbackList()->ShouldSkipDrawing();
}
private:
// TODO(xiaochengh): The function not only initializes null FontFallbackList,
// but also syncs invalid FontFallbackList. Rename it for better readability.
FontFallbackList* EnsureFontFallbackList() const;
void RevalidateFontFallbackList() const;
void ReleaseFontFallbackListRef() const;
......
<!DOCTYPE html>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<style>
.container {
font: 20px/1 custom-font, sans-serif;
}
.toggle {
outline: 1px solid red;
}
</style>
<div class=container><b id="foo" style="color: orange">foo</b><b id="bar" style="color: purple">bar</b></div>
<script>
promise_test(async () => {
const face = new FontFace('custom-font', 'url(../../../resources/Ahem.ttf)');
face.load();
document.fonts.add(face);
document.body.offsetLeft; // force style and layout
document.querySelectorAll('b').forEach(b => b.classList.add('toggle'));
await document.fonts.ready;
document.body.offsetLeft; // force style and layout
assert_equals(foo.getBoundingClientRect().width, 60);
assert_equals(bar.getBoundingClientRect().width, 60);
}, 'Text nodes shaped together should still have valid Font objects after font loading and do not cause crash');
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment