New issue
Advanced search Search tips

Issue 849923 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 839344



Sign in to add a comment

Extend FontService to contain FontConfig methods previously answered by sandbox_ipc_linux

Project Member Reported by drott@chromium.org, Jun 6 2018

Issue description

In order to move font sandbox IPC from using the global filedescriptor to Mojo messages, we have to find a new home for the messages that were previously answered and handled in sandbox IPC Linux:

https://cs.chromium.org/chromium/src/services/service_manager/sandbox/linux/sandbox_linux.h?type=cs&sq=package:chromium&g=0&l=58

These methods will be added to FontService, so that FontService can be spawned inside a utility process and respond to OOP FontConfig requests.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 21 2018

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

commit 7d8b5f285d50f082803c532e251b2915dc2587e7
Author: Dominik Röttsches <drott@chromium.org>
Date: Thu Jun 21 22:49:33 2018

Add tests to font service

Add testing for font service to test methods on FontLoader. FontLoader
will be extended to carry additional FontConfig IPC methods for font
fallback, querying render style for strike, and performing PPAPI
specific font functions. (See "FontService replacing Fonts Sandbox IPC"
design doc [1] and issue [2]).

Before doing that, let's ensure we have a framework to test these
methods. As the font service runs in a child utility process, we need a
way to ensure that it is set up for loading the custom FontConfig
configuration that we use for consistent font access in testing. To that
end, pass a command line feature enabled flag to the child process, and
adding while setting up the TestSuite.

Test basic matching, test failed matching, and test matching the
empty font name.

[1] https://docs.google.com/document/d/1dLBekS3RlcQFldeYZlzFGkuLyX53GBr_7Hbf4cUz1OE/
[2] crbug.com/839344

Bug:  849923 
Change-Id: Ib3118d4a53b075725cbdc7fa89ba4f2b9eec8d5e
Reviewed-on: https://chromium-review.googlesource.com/1091754
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569387}
[modify] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/base/test/fontconfig_util_linux.cc
[modify] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/base/test/fontconfig_util_linux.h
[modify] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/base/test/test_suite.cc
[modify] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/components/services/font/BUILD.gn
[modify] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/components/services/font/OWNERS
[add] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/components/services/font/font_loader_test.cc
[add] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/components/services/font/font_loader_test.h
[modify] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/components/services/font/font_service_app.cc
[modify] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/components/services/font/manifest.json
[add] https://crrev.com/7d8b5f285d50f082803c532e251b2915dc2587e7/components/services/font/test_manifest.json

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 25 2018

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

commit e32cbb07faf18f133b89860a9e9ef3fe932fd684
Author: Dominik Röttsches <drott@chromium.org>
Date: Mon Jun 25 06:52:35 2018

Extend FontService to handle FontConfig Sandbox IPC methods

In order to turn OOP FontConfig queries from Sandbox IPC filedescriptor
based messages to Mojo messages, we need FontService to handle
additional queries for font fallback, retrieving render style and
performing font matching in a PPAPI context. These methods are moving
from sandbox_ipc_linux.cc as well as font_utils_linux.h and will be
temporarily duplicated until the methods will be removed from
where they were duplicated from.

For the design doc, please refer to crbug.com/839344.

Bug:  849923 
Change-Id: If0197eae2c9bba8495c065d641b3fef39e403a83
Reviewed-on: https://chromium-review.googlesource.com/1087951
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569974}
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/BUILD.gn
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/DEPS
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/font_loader_test.cc
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/font_service_app.cc
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/font_service_app.h
[add] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/ppapi_fontconfig_matching.cc
[add] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/ppapi_fontconfig_matching.h
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/public/cpp/font_loader.cc
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/public/cpp/font_loader.h
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/public/cpp/font_service_thread.cc
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/public/cpp/font_service_thread.h
[modify] https://crrev.com/e32cbb07faf18f133b89860a9e9ef3fe932fd684/components/services/font/public/interfaces/font_service.mojom

Comment 3 by drott@chromium.org, Jun 25 2018

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 26 2018

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

commit b83f33284b7f1e151da1b8950f8dc2b64cbc39b1
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Tue Jun 26 00:27:26 2018

Remove kFontConfigTestingEnvironment

CL [1] propagates the FONTCONFIG_FILE environment variable to subprocesses and
thus ensures, child/utility processes receive the correct fontconfig testing
environment.  This means adding a command line switch to indicate a testing
environment for fontconfig is no longer necessary.

R=drott,thestig
BUG= 849923 

Change-Id: I2234802904e9f725fa0b221fd65aced4885e0023
Reviewed-on: https://chromium-review.googlesource.com/1112768
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570263}
[modify] https://crrev.com/b83f33284b7f1e151da1b8950f8dc2b64cbc39b1/base/test/fontconfig_util_linux.cc
[modify] https://crrev.com/b83f33284b7f1e151da1b8950f8dc2b64cbc39b1/base/test/fontconfig_util_linux.h
[modify] https://crrev.com/b83f33284b7f1e151da1b8950f8dc2b64cbc39b1/base/test/test_suite.cc
[modify] https://crrev.com/b83f33284b7f1e151da1b8950f8dc2b64cbc39b1/components/services/font/BUILD.gn
[modify] https://crrev.com/b83f33284b7f1e151da1b8950f8dc2b64cbc39b1/components/services/font/font_loader_test.cc
[modify] https://crrev.com/b83f33284b7f1e151da1b8950f8dc2b64cbc39b1/components/services/font/font_service_app.cc

Sign in to add a comment