New issue
Advanced search Search tips

Issue 881007 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

need to cleanup build dependencies on skia's FontConfigInterface

Project Member Reported by reed@google.com, Sep 5

Issue description

Chrome only links against Skia's impl files on linux, but has subclasses / instances of it on non-linux builds. Skia needs to do some refactoring, moving some base impl into SkFontConfigInterface.cpp, but this is breaking on non-linux builds.

quick-fix: always link SkFontConfigInterface.cpp

deeper-fix: clean-up dependencies / references to not instantiate when not on linux (if possible)
 
Inspired by trying to land this Skia CL:

https://skia-review.googlesource.com/c/skia/+/151880
font_service::FontLoader appears to built on Mac, but skia/BUILD.gn only pulls in the implementation of SkFontConfigInterface when is_linux is true. This used to work due to SkFontConfigInterface having only pure virtual and inline functions.

It appears that any uses of this can be found by

https://cs.chromium.org/search/?q=file:BUILD.gn+font/public/cpp&sq=package:chromium&type=cs

and it appears there are two users which do not protect this behind an is_linux flag. It appears these users should be updated and src/components/services/font/public/cpp/BUILD.gn be modified to assert that is_linux is true.

The Skia build file could provide the SkFontConfigInterface implementation on non-linux platforms, but since this is an unfortunate interface it would be nice to isolate it.
Agree that SkFontConfigInterface should only be needed in Linux builds and moving to where an is_linux assert in font_service holds true is the right approach.

You can also find the users of //components/services/font/public/cpp using gn refs, e.g.
$ gn refs out/gnrelease //components/services/font/public/cpp
//chrome/renderer:renderer
//components/services/font:font_service_unittests
//content/child:child
//content/ppapi_plugin:ppapi_plugin_sources
//content/renderer:renderer
//services/content/simple_browser:simple_browser
 



Sign in to add a comment