New issue
Advanced search Search tips

Issue 889864 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 918144

Blocking:
issue 828317



Sign in to add a comment

Implement font unique name matching based on DWrite

Project Member Reported by drott@chromium.org, Sep 27

Issue description

After implementations for the other OSes are coming along, we need to implement font unique name matching on Windows.

Details in the design document: https://docs.google.com/document/d/1Fe0FvHXqKatXfwbFici6aydSvCjsRTIQONZPR0Sb46Q/edit#

It seems we can perform the matching in the sandboxed process, but need Skia API to be able to create an SkTypeface just from a IDWriteFontFace.


 
Blockedon: skia:8418
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f176173706f1ee0432ecd443e9b7966eede5f6c9

commit f176173706f1ee0432ecd443e9b7966eede5f6c9
Author: Dominik Röttsches <drott@chromium.org>
Date: Tue Dec 11 17:56:44 2018

Remove deprecated loading mode switches

These switches were used in an trial to figure out whether they help
with the Windows empty font handle issues. These trials have now been
expired without the intended results and are no longer used.

Bug:  889864 
Change-Id: I9ed1376d989b59a71e325cd54604faeaae71a5b1
Reviewed-on: https://chromium-review.googlesource.com/c/1371807
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615576}
[modify] https://crrev.com/f176173706f1ee0432ecd443e9b7966eede5f6c9/content/browser/renderer_host/OWNERS
[modify] https://crrev.com/f176173706f1ee0432ecd443e9b7966eede5f6c9/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc
[modify] https://crrev.com/f176173706f1ee0432ecd443e9b7966eede5f6c9/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.h

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e598fc4441c22995aacff7da88934fa9f82f3ce

commit 6e598fc4441c22995aacff7da88934fa9f82f3ce
Author: Dominik Röttsches <drott@chromium.org>
Date: Tue Dec 11 18:03:50 2018

Add method to retrieve path and ttc index, refactor AddFontFiles

DirectWrite will only ever give us one file - we do not need an
implementation for retrieving multiple files per IDWriteFont. In
upcoming changes for unique local font matching for src: local(...)
@font-face declarations, we need a way to retrieve the ttc index of a
matched font. Taking those two aspects into account, refactor
AddFontFiles to rely on a new method FontFilePathAndTtcIndex which
retrieves path and ttc index for a IDWriteFont. Also move helper
methods to anonymous namespace.

No functional changes to the DWriteFontProxy mojo interface, which
this DWriteFontProxyImpl implements. Implementation mostly moved
and restructured, but mostly unchanged and taken from previous
code.

Bug:  889864 
Change-Id: Ida13667533eb338bb62083026ddbb386b8dcf0b2
Reviewed-on: https://chromium-review.googlesource.com/c/1371884
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615582}
[modify] https://crrev.com/6e598fc4441c22995aacff7da88934fa9f82f3ce/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc
[modify] https://crrev.com/6e598fc4441c22995aacff7da88934fa9f82f3ce/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.h
[modify] https://crrev.com/6e598fc4441c22995aacff7da88934fa9f82f3ce/tools/metrics/histograms/enums.xml

Blockedon: 918144
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24f9d163550d0de7d8e843fe37f92908bc6a1372

commit 24f9d163550d0de7d8e843fe37f92908bc6a1372
Author: Dominik Röttsches <drott@chromium.org>
Date: Fri Jan 04 10:43:56 2019

Move DWriteFontProxy mojo file to blink/common

For enabling font unique local name matching, Blink needs to access the
DWriteFontProxy mojo service directly, not only through Skia. For Blink
to have access to the Mojo interface, move the file to Blink Common, the
location for shared files between content on the browser side and Blink
side.

Bug:  889864 
Change-Id: I438d6659f3fda7140051a57824e8f3554703d2e3
Reviewed-on: https://chromium-review.googlesource.com/c/1394584
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619907}
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/browser/renderer_host/dwrite_font_proxy_impl_win.cc
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/browser/renderer_host/dwrite_font_proxy_impl_win.h
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/browser/renderer_host/dwrite_font_proxy_impl_win_unittest.cc
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/child/dwrite_font_proxy/dwrite_font_proxy_init_impl_win.cc
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/child/dwrite_font_proxy/dwrite_font_proxy_init_impl_win.h
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/child/dwrite_font_proxy/dwrite_font_proxy_win.cc
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/child/dwrite_font_proxy/dwrite_font_proxy_win.h
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/child/dwrite_font_proxy/font_fallback_win.cc
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/child/dwrite_font_proxy/font_fallback_win.h
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/common/BUILD.gn
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/test/dwrite_font_fake_sender_win.cc
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/content/test/dwrite_font_fake_sender_win.h
[modify] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/third_party/blink/public/mojom/BUILD.gn
[add] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/third_party/blink/public/mojom/dwrite_font_proxy/OWNERS
[rename] https://crrev.com/24f9d163550d0de7d8e843fe37f92908bc6a1372/third_party/blink/public/mojom/dwrite_font_proxy/dwrite_font_proxy.mojom

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d41b79cf99c34be4c1894e7d673002b7562d6d4

commit 0d41b79cf99c34be4c1894e7d673002b7562d6d4
Author: Dominik Röttsches <drott@chromium.org>
Date: Fri Jan 04 17:16:16 2019

Move unique font instantiation out to FontCache

As this helper function will be used by font_cache_skia_win.cc as well,
move it out to FontCache and share the implementation between backends.

Bug:  889864 
Change-Id: Ic0d6c3eec1cd7689af7f2e394b9f83c8487dd22a
Reviewed-on: https://chromium-review.googlesource.com/c/1396020
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619981}
[modify] https://crrev.com/0d41b79cf99c34be4c1894e7d673002b7562d6d4/third_party/blink/renderer/platform/fonts/font_cache.cc
[modify] https://crrev.com/0d41b79cf99c34be4c1894e7d673002b7562d6d4/third_party/blink/renderer/platform/fonts/font_cache.h
[modify] https://crrev.com/0d41b79cf99c34be4c1894e7d673002b7562d6d4/third_party/blink/renderer/platform/fonts/skia/font_cache_skia.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fde3083af29476e8b8abd261d260ac801a5e18d4

commit fde3083af29476e8b8abd261d260ac801a5e18d4
Author: Dominik Röttsches <drott@chromium.org>
Date: Mon Jan 07 21:08:05 2019

Add a unique font name lookup table to DWriteFontProxy Mojo Service

Analogously to the implementation on Android [1], on Windows build a
font unique name lookup table for font unique name matching used in
@font-face { src: local(<name>) situations.

While we cache this lookup structure on Android, as it only changes when
the firmware is updated, on Windows we rebuild it after every Chrome
startup. UMA will be added to measure table construction time and
sizes. On the developer machine, Windows 10, the table construction
takes about 26ms, and is about 50kb in size.

Using newer Windows 10 DirectWrite API proved problematic, as the
results from matching unique font names using that resulted in hitting
sandbox restrictions. Also, usage of Windows 10 only API would not
provide a solution for our users with older Windows versions. That's
the reason we're using only DirectWrite API compatible with older
versions of Windows - enabling font unique name matching for all Windows
users.

Design document linked from https://crbug.com/828317.

[1] https://crrev.com/7e00fed99a45512b17c974482c73ddcb72181850

Bug:  889864 
Change-Id: Ie4cf33990d1fc87d3745abbc214c84449d5a13e2
Reviewed-on: https://chromium-review.googlesource.com/c/1394531
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620468}
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/content/browser/BUILD.gn
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/content/browser/renderer_host/dwrite_font_proxy_impl_win.cc
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/content/browser/renderer_host/dwrite_font_proxy_impl_win.h
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/content/browser/renderer_host/dwrite_font_proxy_impl_win_unittest.cc
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/content/test/BUILD.gn
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/content/test/dwrite_font_fake_sender_win.cc
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/content/test/dwrite_font_fake_sender_win.h
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/third_party/blink/common/BUILD.gn
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/third_party/blink/public/common/BUILD.gn
[modify] https://crrev.com/fde3083af29476e8b8abd261d260ac801a5e18d4/third_party/blink/public/mojom/dwrite_font_proxy/dwrite_font_proxy.mojom

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1c865468f7ba485248b63cccd4607514d809828

commit f1c865468f7ba485248b63cccd4607514d809828
Author: Dominik Röttsches <drott@chromium.org>
Date: Mon Jan 07 21:17:59 2019

Add UMA for font table size construction time and size

For figuring out how font unique name lookup table construction time and
size scale across a wider spectrum of machine specs, track these two
values. These will help us decide whether the way the table is built is
readonably fast and just copied from cached information in DWrite or
whether it is slowed down by hitting the disk or similar.

Bug:  889864 
Change-Id: I3e5e9fb49f1bbad30ffae69cc1118b60ff965aa5
Reviewed-on: https://chromium-review.googlesource.com/c/1396018
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620473}
[modify] https://crrev.com/f1c865468f7ba485248b63cccd4607514d809828/content/browser/renderer_host/dwrite_font_proxy_impl_win.cc
[modify] https://crrev.com/f1c865468f7ba485248b63cccd4607514d809828/tools/metrics/histograms/histograms.xml

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fded3f60bac6300501d3b41defdb032a1a2d7eaf

commit fded3f60bac6300501d3b41defdb032a1a2d7eaf
Author: Dominik Röttsches <drott@chromium.org>
Date: Tue Jan 08 14:25:48 2019

Revert "Add UMA for font table size construction time and size"

This reverts commit f1c865468f7ba485248b63cccd4607514d809828.

Reason for revert: Base CL introducing unique font matching needs to be reverted.

Original change's description:
> Add UMA for font table size construction time and size
> 
> For figuring out how font unique name lookup table construction time and
> size scale across a wider spectrum of machine specs, track these two
> values. These will help us decide whether the way the table is built is
> readonably fast and just copied from cached information in DWrite or
> whether it is slowed down by hitting the disk or similar.
> 
> Bug:  889864 
> Change-Id: I3e5e9fb49f1bbad30ffae69cc1118b60ff965aa5
> Reviewed-on: https://chromium-review.googlesource.com/c/1396018
> Commit-Queue: Dominik Röttsches <drott@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620473}

TBR=asvitkine@chromium.org,eae@chromium.org,drott@chromium.org

Change-Id: Icdcd7dfd6e07c7879f1f54d452d9d972cf7f230d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  889864 
Reviewed-on: https://chromium-review.googlesource.com/c/1400801
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620707}
[modify] https://crrev.com/fded3f60bac6300501d3b41defdb032a1a2d7eaf/content/browser/renderer_host/dwrite_font_proxy_impl_win.cc
[modify] https://crrev.com/fded3f60bac6300501d3b41defdb032a1a2d7eaf/tools/metrics/histograms/histograms.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09398ed918cd83a385c8867e6098b17d3b80e5ab

commit 09398ed918cd83a385c8867e6098b17d3b80e5ab
Author: Dominik Röttsches <drott@chromium.org>
Date: Tue Jan 08 14:27:12 2019

Revert "Add a unique font name lookup table to DWriteFontProxy Mojo Service"

This reverts commit fde3083af29476e8b8abd261d260ac801a5e18d4.

Reason for revert: Hits an access violation on official builders, as documented in issue:919827

Original change's description:
> Add a unique font name lookup table to DWriteFontProxy Mojo Service
> 
> Analogously to the implementation on Android [1], on Windows build a
> font unique name lookup table for font unique name matching used in
> @font-face { src: local(<name>) situations.
> 
> While we cache this lookup structure on Android, as it only changes when
> the firmware is updated, on Windows we rebuild it after every Chrome
> startup. UMA will be added to measure table construction time and
> sizes. On the developer machine, Windows 10, the table construction
> takes about 26ms, and is about 50kb in size.
> 
> Using newer Windows 10 DirectWrite API proved problematic, as the
> results from matching unique font names using that resulted in hitting
> sandbox restrictions. Also, usage of Windows 10 only API would not
> provide a solution for our users with older Windows versions. That's
> the reason we're using only DirectWrite API compatible with older
> versions of Windows - enabling font unique name matching for all Windows
> users.
> 
> Design document linked from https://crbug.com/828317.
> 
> [1] https://crrev.com/7e00fed99a45512b17c974482c73ddcb72181850
> 
> Bug:  889864 
> Change-Id: Ie4cf33990d1fc87d3745abbc214c84449d5a13e2
> Reviewed-on: https://chromium-review.googlesource.com/c/1394531
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Commit-Queue: Dominik Röttsches <drott@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620468}

TBR=digit@chromium.org,chrishtr@chromium.org,eae@chromium.org,drott@chromium.org,rsesek@chromium.org,foolip@chromium.org

Change-Id: Ie12699d9c315940a0f9cd3e0170ab91688df8ef6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  889864 , 919827
Reviewed-on: https://chromium-review.googlesource.com/c/1400802
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620709}
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/content/browser/BUILD.gn
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/content/browser/renderer_host/dwrite_font_proxy_impl_win.cc
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/content/browser/renderer_host/dwrite_font_proxy_impl_win.h
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/content/browser/renderer_host/dwrite_font_proxy_impl_win_unittest.cc
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/content/test/BUILD.gn
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/content/test/dwrite_font_fake_sender_win.cc
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/content/test/dwrite_font_fake_sender_win.h
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/third_party/blink/common/BUILD.gn
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/third_party/blink/public/common/BUILD.gn
[modify] https://crrev.com/09398ed918cd83a385c8867e6098b17d3b80e5ab/third_party/blink/public/mojom/dwrite_font_proxy/dwrite_font_proxy.mojom

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/421609e7af38ba79558a2ec822d6823e57a571a7

commit 421609e7af38ba79558a2ec822d6823e57a571a7
Author: Dominik Röttsches <drott@chromium.org>
Date: Tue Jan 08 15:56:03 2019

Move FontTableMatcher FontUniqueNameLookup implementation to base class

Preparation for reusing this implementation on Windows.
No functional changes for the existing Android implementation.
End to end Blink to Browser tests in FontUniqueNameBrowserTest.

Bug:  889864 
Change-Id: I5a42e2ed50b2d5a772643e8818981041c744b047
Reviewed-on: https://chromium-review.googlesource.com/c/1400683
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620737}
[modify] https://crrev.com/421609e7af38ba79558a2ec822d6823e57a571a7/third_party/blink/renderer/platform/fonts/android/font_unique_name_lookup_android.cc
[modify] https://crrev.com/421609e7af38ba79558a2ec822d6823e57a571a7/third_party/blink/renderer/platform/fonts/android/font_unique_name_lookup_android.h
[modify] https://crrev.com/421609e7af38ba79558a2ec822d6823e57a571a7/third_party/blink/renderer/platform/fonts/font_unique_name_lookup.cc
[modify] https://crrev.com/421609e7af38ba79558a2ec822d6823e57a571a7/third_party/blink/renderer/platform/fonts/font_unique_name_lookup.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/839641e398645f238aba1beb8d624ea7d4f23b8a

commit 839641e398645f238aba1beb8d624ea7d4f23b8a
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Tue Jan 08 16:27:04 2019

Revert "Move FontTableMatcher FontUniqueNameLookup implementation to base class"

This reverts commit 421609e7af38ba79558a2ec822d6823e57a571a7.

Reason for revert: Looks like it broke the build

https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Win/42907

Original change's description:
> Move FontTableMatcher FontUniqueNameLookup implementation to base class
> 
> Preparation for reusing this implementation on Windows.
> No functional changes for the existing Android implementation.
> End to end Blink to Browser tests in FontUniqueNameBrowserTest.
> 
> Bug:  889864 
> Change-Id: I5a42e2ed50b2d5a772643e8818981041c744b047
> Reviewed-on: https://chromium-review.googlesource.com/c/1400683
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Commit-Queue: Dominik Röttsches <drott@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620737}

TBR=eae@chromium.org,kojii@chromium.org,drott@chromium.org

Change-Id: Idafde7e7999bffe06c3c802ffaace348e82c8bbc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  889864 
Reviewed-on: https://chromium-review.googlesource.com/c/1400808
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620747}
[modify] https://crrev.com/839641e398645f238aba1beb8d624ea7d4f23b8a/third_party/blink/renderer/platform/fonts/android/font_unique_name_lookup_android.cc
[modify] https://crrev.com/839641e398645f238aba1beb8d624ea7d4f23b8a/third_party/blink/renderer/platform/fonts/android/font_unique_name_lookup_android.h
[modify] https://crrev.com/839641e398645f238aba1beb8d624ea7d4f23b8a/third_party/blink/renderer/platform/fonts/font_unique_name_lookup.cc
[modify] https://crrev.com/839641e398645f238aba1beb8d624ea7d4f23b8a/third_party/blink/renderer/platform/fonts/font_unique_name_lookup.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46e7ed4ef9381e32c04e9afef9766e655e4f87dc

commit 46e7ed4ef9381e32c04e9afef9766e655e4f87dc
Author: Dominik Röttsches <drott@chromium.org>
Date: Wed Jan 09 13:44:54 2019

Reland: Move FontTableMatcher FontUniqueNameLookup implementation to base class

Previously reviewed in [1], reverted in [2],
now contains Windows build fix.

Preparation for reusing this implementation on Windows.
No functional changes for the existing Android implementation.
End to end Blink to Browser tests in FontUniqueNameBrowserTest.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1400683
[2] https://chromium-review.googlesource.com/c/chromium/src/+/1400808

Bug:  889864 
Change-Id: I6a8f7b820615ae3f223660b0dcd9d83aeffd7dcc
Tbr: eae
Reviewed-on: https://chromium-review.googlesource.com/c/1402761
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621131}
[modify] https://crrev.com/46e7ed4ef9381e32c04e9afef9766e655e4f87dc/third_party/blink/public/common/BUILD.gn
[modify] https://crrev.com/46e7ed4ef9381e32c04e9afef9766e655e4f87dc/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/46e7ed4ef9381e32c04e9afef9766e655e4f87dc/third_party/blink/renderer/platform/fonts/android/font_unique_name_lookup_android.cc
[modify] https://crrev.com/46e7ed4ef9381e32c04e9afef9766e655e4f87dc/third_party/blink/renderer/platform/fonts/android/font_unique_name_lookup_android.h
[modify] https://crrev.com/46e7ed4ef9381e32c04e9afef9766e655e4f87dc/third_party/blink/renderer/platform/fonts/font_unique_name_lookup.cc
[modify] https://crrev.com/46e7ed4ef9381e32c04e9afef9766e655e4f87dc/third_party/blink/renderer/platform/fonts/font_unique_name_lookup.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0c644654c7ba8a20d2e137dcaf908415aea465f

commit b0c644654c7ba8a20d2e137dcaf908415aea465f
Author: Dominik Röttsches <drott@chromium.org>
Date: Wed Jan 09 15:18:37 2019

Reland: Add a unique font name lookup table to DWriteFontProxy Mojo Service

Reland after revert in [2]. It previously hit an issue on the official
builders due to fonts being installed in custom locations outside of the
Windows Fonts folder which caused and invalid path name to be
copied. This is now fixed and those path names are passed to Blink. We
still cannot open them on the Blink side due to sandboxing but UMA is
added on the Blink side to see if this is actually a frequent issue or
can be ignored.

Analogously to the implementation on Android [1], on Windows build a
font unique name lookup table for font unique name matching used in
@font-face { src: local(<name>) situations.

While we cache this lookup structure on Android, as it only changes when
the firmware is updated, on Windows we rebuild it after every Chrome
startup. UMA will be added to measure table construction time and
sizes. On the developer machine, Windows 10, the table construction
takes about 26ms, and is about 50kb in size.

Using newer Windows 10 DirectWrite API proved problematic, as the
results from matching unique font names using that resulted in hitting
sandbox restrictions. Also, usage of Windows 10 only API would not
provide a solution for our users with older Windows versions. That's
the reason we're using only DirectWrite API compatible with older
versions of Windows - enabling font unique name matching for all Windows
users.

Design document linked from https://crbug.com/828317.

[1] https://crrev.com/7e00fed99a45512b17c974482c73ddcb72181850
[2] https://chromium-review.googlesource.com/c/chromium/src/+/1400669

Bug:  889864 , 919827
Tbr: eae, foolip, rsesek, chrishtr
Change-Id: I8833fb6653b749083d03d8b277cf19184e05a40a
Reviewed-on: https://chromium-review.googlesource.com/c/1403115
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621144}
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/content/browser/BUILD.gn
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/content/browser/renderer_host/dwrite_font_proxy_impl_win.cc
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/content/browser/renderer_host/dwrite_font_proxy_impl_win.h
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/content/browser/renderer_host/dwrite_font_proxy_impl_win_unittest.cc
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/content/test/BUILD.gn
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/content/test/dwrite_font_fake_sender_win.cc
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/content/test/dwrite_font_fake_sender_win.h
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/third_party/blink/common/BUILD.gn
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/third_party/blink/public/common/BUILD.gn
[modify] https://crrev.com/b0c644654c7ba8a20d2e137dcaf908415aea465f/third_party/blink/public/mojom/dwrite_font_proxy/dwrite_font_proxy.mojom

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d55bfbc8d15e46da4d08806687f00bb77569603b

commit d55bfbc8d15e46da4d08806687f00bb77569603b
Author: Dominik Röttsches <drott@chromium.org>
Date: Wed Jan 09 20:03:05 2019

Reland: Add UMA for font table size construction time and size

Previously reverted in [1] to make way for reverting its parent commit.

For figuring out how font unique name lookup table construction time and
size scale across a wider spectrum of machine specs, track these two
values. These will help us decide whether the way the table is built is
readonably fast and just copied from cached information in DWrite or
whether it is slowed down by hitting the disk or similar.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1400801

Bug:  889864 
Tbr: asvitkine, eae
Change-Id: I95760447ef902040fec2574d215b80a4e014ea53
Reviewed-on: https://chromium-review.googlesource.com/c/1402763
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621280}
[modify] https://crrev.com/d55bfbc8d15e46da4d08806687f00bb77569603b/content/browser/renderer_host/dwrite_font_proxy_impl_win.cc
[modify] https://crrev.com/d55bfbc8d15e46da4d08806687f00bb77569603b/tools/metrics/histograms/histograms.xml

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29fa93958937e4324a35ec332f3ee8d06c3df2ff

commit 29fa93958937e4324a35ec332f3ee8d06c3df2ff
Author: Dominik Röttsches <drott@chromium.org>
Date: Wed Jan 09 23:48:16 2019

Hook up Blink's Windows font backend to font unique name matching

Reuse the Android implementation, except instead of connecting to the
FontUniqueNameLookup Mojo service, we connect to the
DWriteFontProxy. Retrieve the ReadOnlySharedMemoryRegion from there,
initialize a FontTableMatcher with this to perform matching.

Bug:  889864 
Change-Id: I59bbe59ba55510b37d6ff43f152947eebe72f821
Reviewed-on: https://chromium-review.googlesource.com/c/1400665
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621373}
[modify] https://crrev.com/29fa93958937e4324a35ec332f3ee8d06c3df2ff/content/browser/font_unique_name_lookup/font_unique_name_browsertest.cc
[modify] https://crrev.com/29fa93958937e4324a35ec332f3ee8d06c3df2ff/content/test/data/font_src_local_matching.html
[modify] https://crrev.com/29fa93958937e4324a35ec332f3ee8d06c3df2ff/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/29fa93958937e4324a35ec332f3ee8d06c3df2ff/third_party/blink/renderer/platform/fonts/font_unique_name_lookup.cc
[modify] https://crrev.com/29fa93958937e4324a35ec332f3ee8d06c3df2ff/third_party/blink/renderer/platform/fonts/win/font_cache_skia_win.cc
[add] https://crrev.com/29fa93958937e4324a35ec332f3ee8d06c3df2ff/third_party/blink/renderer/platform/fonts/win/font_unique_name_lookup_win.cc
[add] https://crrev.com/29fa93958937e4324a35ec332f3ee8d06c3df2ff/third_party/blink/renderer/platform/fonts/win/font_unique_name_lookup_win.h

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c093321cee234053ffdf18958def34219a0f6467

commit c093321cee234053ffdf18958def34219a0f6467
Author: Dominik Röttsches <drott@chromium.org>
Date: Thu Jan 10 09:13:15 2019

UMA metrics for detecting uniquely matched fonts outside system font dir

Add UMA metrics for detecting whether a uniquely matched font can be
instantiated, or whether access to it is restricted by the sandbox or
other reasons. The expectation is that this is a highly rare case. If a
font cannot be instantiated, Blink automatically falls back to the next
font in the CSS font family list, or performs system fallback.

With this metric, we would like to find out whether we need to cover the
case of uniquely matched fonts outside of system locations, or not. For
example, by opening those files on the browser process side and passing
the handle to the renderer.

Bug:  889864 
Change-Id: I27a242a1b2fca39be740897a5f270780f2c91a5a
Reviewed-on: https://chromium-review.googlesource.com/c/1400694
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621513}
[modify] https://crrev.com/c093321cee234053ffdf18958def34219a0f6467/third_party/blink/renderer/platform/fonts/win/font_unique_name_lookup_win.cc
[modify] https://crrev.com/c093321cee234053ffdf18958def34219a0f6467/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/c093321cee234053ffdf18958def34219a0f6467/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)
Blockedon: -skia:8418
Project Member

Comment 20 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4641ae7ad64f0972109a66989a90217a1d51a9fa

commit 4641ae7ad64f0972109a66989a90217a1d51a9fa
Author: Dominik Röttsches <drott@chromium.org>
Date: Fri Jan 11 09:58:56 2019

Fix unique local matching for sideloaded fonts

In Windows layout tests we populate a set of sideloaded fonts to the
FontCache. These were not covered by local unique matching, even though
some tests rely on for example matching src: local(Ahem). Fix this with
a test-only implementation based on retrieving the PostScript name from
FreeType.

This fixes
    http/tests/css/css-resources-referrer-srcdoc.html
    http/tests/css/css-resources-referrer.html
    http/tests/webfont/fontface-style-change.html
when enabling local unique font name matching.

Bug:  889864 
Change-Id: I5284b2fc186cff03d2da9475ccd081cb3632dbc6
Reviewed-on: https://chromium-review.googlesource.com/c/1404089
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621957}
[modify] https://crrev.com/4641ae7ad64f0972109a66989a90217a1d51a9fa/third_party/blink/renderer/platform/fonts/win/font_cache_skia_win.cc

Sign in to add a comment