My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 247920: CSS unicode-range should be used to prevent unnecessary downloads
16 people starred this issue and may be notified of changes. Back to list
 
Reported by chris@improbable.org, Jun 8, 2013
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.110 Safari/537.36

Example URL:
http://chris.improbable.org/experiments/browser/webfonts/universal-noto-sans/

Steps to reproduce the problem:
http://chris.improbable.org/experiments/browser/webfonts/universal-noto-sans/all-in-one.html has multiple @font-face declarations which different font-family names and unicode-range declarations

http://chris.improbable.org/experiments/browser/webfonts/universal-noto-sans/single-name.html has multiple @font-face declarations with a single font-family name and different unicode-range declarations

What is the expected behavior?
Chrome should not download fonts when at least one character in the unicode-range declaration is not present on the page

What went wrong?
In either case, Chrome downloads all of the fonts if they're specified in any font-family rule. See http://chris.improbable.org/experiments/browser/webfonts/universal-noto-sans/ for test cases and webpagetest.org sessions

Did this work before? Yes 

Chrome version: 27.0.1453.110  Channel: n/a
OS Version: OS X 10.8.4
Jun 9, 2013
#1 tkent@chromium.org
(No comment was entered for this change.)
Labels: -Cr-Internals-Network Cr-Blink-WebFonts
Jul 2, 2013
#2 ksakamoto@chromium.org
Previous attempt to fix this: https://bugs.webkit.org/show_bug.cgi?id=42154
Status: Untriaged
Cc: ksakamoto@chromium.org
Labels: -OS-Mac OS-All
Jul 3, 2013
#3 kenjibaheux@chromium.org
(No comment was entered for this change.)
Status: Available
Labels: Hotlist-Fixit
Jul 16, 2013
#4 ksakamoto@chromium.org
(No comment was entered for this change.)
Status: Assigned
Owner: ksakamoto@chromium.org
Sep 17, 2013
#6 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=157893

------------------------------------------------------------------------
r157893 | ksakamoto@chromium.org | 2013-09-17T10:44:29.698687Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFaceSource.cpp?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/platform/graphics/GlyphPageTreeNode.cpp?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.h?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/platform/graphics/FontFallbackList.h?r1=157893&r2=157892&pathrev=157893
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-load.html?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFaceSource.h?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/platform/graphics/SimpleFontData.cpp?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFace.cpp?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/platform/graphics/SimpleFontData.h?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/platform/graphics/SegmentedFontData.h?r1=157893&r2=157892&pathrev=157893
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-load-expected.txt?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.cpp?r1=157893&r2=157892&pathrev=157893
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/platform/graphics/FontFallbackList.cpp?r1=157893&r2=157892&pathrev=157893

Use unicode-range to prevent unnecessary @font-face donwnloads

This patch delays font loading until creation of GlyphPage whose
codepoint range intersects with the @font-face's unicode-range value.
GlyphPage is created when glyph data for a character in its codepoint
range is requested, so web fonts are not donwloaded if its glyph data
is not used.

Even if any glyph is not used, font metrics may be used (for example,
<input> uses font metrics to calculate its size). These metrics are
accessed via SimpleFontData::primarySimpleFontData, so we start loading
there too.

Caveat:
Since GlyphPage has glyph data for contiguous 256 Unicode codepoints,
this patch does not 100% prevent unnecessary downloads. Use of codepoint
*near* the unicode-range can trigger the font load. To fix this, we need
to delay font loading until a codepoint in unicode-range is requested.
That may require changing the GlyphPage data structure.

BUG=247920
TEST=fast/css/font-face-unicode-range-load.html

Review URL: https://chromiumcodereview.appspot.com/23446007
------------------------------------------------------------------------
Sep 17, 2013
#7 ksakamoto@chromium.org
This patch greatly improves, but does not 100% prevent unnecessary downloads. Blink manages font data for each contiguous 256 codepoints (U+00-FF, U+100-1FF, U+200-2FF, ...). This patch delays font download until this data structure is created, so if the document uses Armenian characters (U+0530-058F), it triggers load of Hebrew font (U+0590-05FF), because it creates a data structure for U+05??.

Let me mark this bug as fixed. Please reopen if it doesn't work for your use case.
Status: Fixed
Labels: M-31
Oct 10, 2013
#9 jakearch...@chromium.org
http://jsbin.com/icawUKu/1/quiet - I'm still seeing requests in basic cases. There shouldn't be a font request on this page because the unicode-range is only lowercase chars. Unless this patch hasn't landed in Canary?
Status: Untriaged
Oct 10, 2013
#10 jakearch...@chromium.org
Actually, I just read your patch a bit closer, I guess that case isn't supposed to work because the uppercase chars are in the same 255 group as lowercase. What about http://jsbin.com/icawUKu/4/quiet - I'm still seeing requests for that font.
Oct 10, 2013
#11 ksakamoto@chromium.org
Thank you for testing this feature!

> I guess that case isn't supposed to work because the uppercase chars are in the same 255 group as lowercase.

Right.

> What about http://jsbin.com/icawUKu/4/quiet - I'm still seeing requests for that font.

In this case, font metrics of the @font-face is used to calculate layout of <p> element because "myfont" is the first in the font-family list and myfont has only one @font-face. (If there is a @font-face that covers U+20 (space character), that face will be used.)

We should be able to use fallback font (sans-serif in this case) for layout. I will look into it later. In the meantime, This workaround works: http://jsbin.com/OHEJIbO/1/quiet

Jan 8, 2014
#12 kenjibaheux@chromium.org
It appears that the current implementation might be sub-optimal for the most common use case.

We always download the fallback font which is handy in the worst case but is impacting performance if the @font-face unicode ranges and subset fonts are properly set (which should be the common case).

We expect that developers will specify a catch all fallback (a large font covering a wide unicode range) and then do their best at defining a set of unicode-range to minimize the likelihood of resorting to downloading said fallback.

=> Re-opening + confirming with spec writers that this is a sound behavior. 
Status: Assigned
Labels: -Pri-2 -M-31 Pri-1 M-34
Jan 13, 2014
#13 jakearch...@chromium.org
Sounds better to me. The unicode-range check should be down to the code point, so a special webfont would be included to provide a fancy ampersand, and wouldn't be downloaded if the content didn't contain an ampersand.
Feb 13, 2014
#14 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=167130

------------------------------------------------------------------------
r167130 | ksakamoto@chromium.org | 2014-02-14T01:43:39.605513Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-load-expected.txt?r1=167130&r2=167129&pathrev=167130
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/SegmentedFontData.h?r1=167130&r2=167129&pathrev=167130
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontFallbackList.cpp?r1=167130&r2=167129&pathrev=167130
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-character-fallback.html?r1=167130&r2=167129&pathrev=167130
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-load.html?r1=167130&r2=167129&pathrev=167130

Primary FontData of FontFallbackList should have space character

This patch makes FontFallbackList::primaryFontData() skips
SegmentedFontData that does not have space character.
FontFallbackList::primarySimpleFontData() expects primaryFontData has
FontData for space character.

This prevents @font-face with unicode-range outside space character
(for example, overrides only an ampersand) from being downloaded
unnecessarily, and used as primary font.

BUG=247920
TEST=fast/css/font-face-unicode-range-load.html

Review URL: https://codereview.chromium.org/162883002
------------------------------------------------------------------------
Feb 13, 2014
#15 ksakamoto@chromium.org
This patch (#14) addresses the issue of #10. Now Chromium does not load font.woff on http://jsbin.com/icawUKu/4/quiet .

Feb 24, 2014
#16 kenjibaheux@chromium.org
Let us know if you hit any roadblocks. Thanks!
Status: Started
Labels: -M-34 M-35
Apr 7, 2014
#17 kar...@google.com
Moving all non essential bugs to the next Milestone.
Labels: -M-35 MovedFrom-35 M-36
Apr 23, 2014
#18 bugdro...@chromium.org
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=172443

------------------------------------------------------------------
r172443 | ksakamoto@chromium.org | 2014-04-24T03:05:35.366168Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/Font.cpp?r1=172443&r2=172442&pathrev=172443
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GlyphPageTreeNode.cpp?r1=172443&r2=172442&pathrev=172443
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GlyphPageTreeNodeTest.cpp?r1=172443&r2=172442&pathrev=172443
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load-expected.txt?r1=172443&r2=172442&pathrev=172443
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/svg/W3C-SVG-1.1-SE/text-intro-09-b.svg?r1=172443&r2=172442&pathrev=172443
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load.html?r1=172443&r2=172442&pathrev=172443
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GlyphPage.h?r1=172443&r2=172442&pathrev=172443

Proper unicode-range font loading behavior

Remote fonts defined by @font-face rules should be loaded only if any
characters in its unicode-range descriptor are used [1].

Before this patch, remote fonts are loaded when the first glyph page for
the font is created. It did not 100% prevent unnecessary downloads, as
(1) All remote fonts in the fallback list were loaded, and (2) Use of
codepoint near the unicode-range could trigger load.

This patch adds m_customFontToLoad field to GlyphPage. It contains
CustomFontData for the first unloaded remote font in the fallback list.
GlyphPage::glyphDataForCharacter() triggers the font load.

text-intro-09-b.svg had to be changed to wait loads because SVGHebrew
and 'Ezra SIL SR' are now loaded sequentially, not at once.

[1] http://www.w3.org/TR/css3-fonts/#composite-fonts

BUG=247920
TEST=blink_platform_unittests,fast/css/font-face-unicode-range-overlap-load.html

Review URL: https://codereview.chromium.org/243453003
-----------------------------------------------------------------
Apr 23, 2014
#19 kenjibaheux@chromium.org
Fixed? (=> Woohoo!)
Or still a few more things needed?
Apr 23, 2014
#20 ksakamoto@chromium.org
Still one more patch needed :)
Apr 29, 2014
#21 bugdro...@chromium.org
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=172943

------------------------------------------------------------------
r172943 | chrome-bot@google.com | 2014-04-30T05:20:20.792107Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.cpp?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.h?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/Font.cpp?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load-expected.txt?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.h?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/Font.h?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderText.cpp?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontSelector.h?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load.html?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFace.cpp?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.cpp?r1=172943&r2=172942&pathrev=172943
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFace.h?r1=172943&r2=172942&pathrev=172943

Make CSSFontFace::willUseFontData() load fonts with unicode-range

Before this patch CSSFontFace::willUseFontData() loads font faces that
have no unicode-range. Since font faces with no unicode-range tends to
be used as fallback font of segmented font family, this behavior leads
to unnecessary font downloads.

This patch makes willUseFontData() loads the first unloaded font face
whose unicode-range intersects with given text. That check does not
need to be 100% precise (false negative is ok), so it only checks the
first character of the text, for speed.

TEST=fast/css/font-face-unicode-range-overlap-load.html
BUG=247920,246492

Review URL: https://codereview.chromium.org/248473005
-----------------------------------------------------------------
Apr 29, 2014
#22 ksakamoto@chromium.org
Fixed!
Status: Fixed
Apr 30, 2014
#23 kenjibaheux@chromium.org
Thanks!!!
Apr 30, 2014
#24 jakearch...@chromium.org
Oh wow, excellent!
May 5, 2014
#25 bugdro...@chromium.org
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=173234

------------------------------------------------------------------
r173234 | enne@chromium.org | 2014-05-02T21:43:28.924173Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/Font.h?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderText.cpp?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontSelector.h?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load.html?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFace.cpp?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.cpp?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFace.h?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.cpp?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.h?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/Font.cpp?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load-expected.txt?r1=173234&r2=173233&pathrev=173234
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.h?r1=173234&r2=173233&pathrev=173234

Revert of Make CSSFontFace::willUseFontData() load fonts with unicode-range (https://codereview.chromium.org/248473005/)

Reason for revert:
Causes off-by-one text errors, e.g. SHOP->RGNO

BUG=369633

Original issue's description:
> Make CSSFontFace::willUseFontData() load fonts with unicode-range
> 
> Before this patch CSSFontFace::willUseFontData() loads font faces that
> have no unicode-range. Since font faces with no unicode-range tends to
> be used as fallback font of segmented font family, this behavior leads
> to unnecessary font downloads.
> 
> This patch makes willUseFontData() loads the first unloaded font face
> whose unicode-range intersects with given text. That check does not
> need to be 100% precise (false negative is ok), so it only checks the
> first character of the text, for speed.
> 
> TEST=fast/css/font-face-unicode-range-overlap-load.html
> BUG=247920,246492
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=172943

TBR=dglazkov@chromium.org,eae@chromium.org,ksakamoto@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=247920,246492

Review URL: https://codereview.chromium.org/261753011
-----------------------------------------------------------------
May 7, 2014
#26 kenjibaheux@chromium.org
(No comment was entered for this change.)
Status: Started
May 8, 2014
#27 bugdro...@chromium.org
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=173699

------------------------------------------------------------------
r173699 | ksakamoto@chromium.org | 2014-05-09T01:29:03.764436Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load-expected.txt?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.h?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/Font.h?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderText.cpp?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontSelector.h?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-unicode-range-overlap-load.html?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFace.cpp?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.cpp?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFace.h?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.cpp?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontFace.h?r1=173699&r2=173698&pathrev=173699
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/Font.cpp?r1=173699&r2=173698&pathrev=173699

Reland r172943 "Make CSSFontFace::willUseFontData() load fonts with unicode-range"

The original patch was reverted in r173234, because the chagne exposed
a hidden bug of SVG fonts (crbug.com/369633). That bug is being addressed
in https://codereview.chromium.org/271633002/ so I'm relanding this patch.

BUG=369633

> Make CSSFontFace::willUseFontData() load fonts with unicode-range
>
> Before this patch CSSFontFace::willUseFontData() loads font faces that
> have no unicode-range. Since font faces with no unicode-range tends to
> be used as fallback font of segmented font family, this behavior leads
> to unnecessary font downloads.
>
> This patch makes willUseFontData() loads the first unloaded font face
> whose unicode-range intersects with given text. That check does not
> need to be 100% precise (false negative is ok), so it only checks the
> first character of the text, for speed.
>
> TEST=fast/css/font-face-unicode-range-overlap-load.html
> BUG=247920, 246492
>
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=172943

Review URL: https://codereview.chromium.org/270813003
-----------------------------------------------------------------
May 14, 2014
#28 ksakamoto@chromium.org
(No comment was entered for this change.)
Status: Fixed
Sign in to add a comment

Powered by Google Project Hosting