Commit a08036fb authored by Natalie Chouinard's avatar Natalie Chouinard Committed by Commit Bot

[GoogleSans] Add metrics on font names requested

Add metrics tracking which font names are requested by the renderer.
This may be used to determine whether it is worthwhile to continue
preloading fonts like Google Sans Bold, which are not used in the Chrome
UI.

Bug: 1129668
Change-Id: I45d4d671cdb989cb38c4d1d2e333a4ffaf54c86c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2433028Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarSky Malice <skym@chromium.org>
Reviewed-by: default avatarJesse Doherty <jwd@chromium.org>
Commit-Queue: Natalie Chouinard <chouinard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811770}
parent ef1887e4
...@@ -6,8 +6,11 @@ ...@@ -6,8 +6,11 @@
<resources xmlns:tools="http://schemas.android.com/tools" tools:ignore="UnusedResources"> <resources xmlns:tools="http://schemas.android.com/tools" tools:ignore="UnusedResources">
<!-- Fonts that are specified to be preloaded by AndroidManifest.xml. --> <!-- Fonts that are specified to be preloaded by AndroidManifest.xml. -->
<array name="preloaded_google_sans_fonts"> <array name="preloaded_google_sans_fonts">
<!-- The following 2 fonts are used in downstream Clank UI. -->
<item>@font/google_sans</item> <item>@font/google_sans</item>
<item>@font/google_sans_medium</item> <item>@font/google_sans_medium</item>
<!-- The following font is not used in Clank UI, but may be used by
AndroidFontLookupImpl#matchLocalFontByUniqueName. -->
<item>@font/google_sans_bold</item> <item>@font/google_sans_bold</item>
</array> </array>
</resources> </resources>
...@@ -12,6 +12,7 @@ import android.os.ParcelFileDescriptor; ...@@ -12,6 +12,7 @@ import android.os.ParcelFileDescriptor;
import android.os.SystemClock; import android.os.SystemClock;
import androidx.annotation.IntDef; import androidx.annotation.IntDef;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import androidx.core.provider.FontRequest; import androidx.core.provider.FontRequest;
import androidx.core.provider.FontsContractCompat; import androidx.core.provider.FontsContractCompat;
...@@ -50,13 +51,19 @@ public class AndroidFontLookupImpl implements AndroidFontLookup { ...@@ -50,13 +51,19 @@ public class AndroidFontLookupImpl implements AndroidFontLookup {
private static final String READ_ONLY_MODE = "r"; private static final String READ_ONLY_MODE = "r";
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
static final String FETCH_FONT_HISTOGRAM = "Android.FontLookup.FetchFontResult"; static final String FETCH_FONT_NAME_HISTOGRAM = "Android.FontLookup.FetchFontName";
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
static final String FETCH_FONT_RESULT_HISTOGRAM = "Android.FontLookup.FetchFontResult";
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
static final String MATCH_LOCAL_FONT_BY_UNIQUE_NAME_HISTOGRAM = static final String MATCH_LOCAL_FONT_BY_UNIQUE_NAME_HISTOGRAM =
"Android.FontLookup.MatchLocalFontByUniqueName.Time"; "Android.FontLookup.MatchLocalFontByUniqueName.Time";
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
static final String GMS_FONT_REQUEST_HISTOGRAM = "Android.FontLookup.GmsFontRequest.Time"; static final String GMS_FONT_REQUEST_HISTOGRAM = "Android.FontLookup.GmsFontRequest.Time";
private static final String GOOGLE_SANS_REGULAR = "google sans regular";
private static final String GOOGLE_SANS_MEDIUM = "google sans medium";
private static final String GOOGLE_SANS_BOLD = "google sans bold";
private final Context mAppContext; private final Context mAppContext;
private final FontsContractWrapper mFontsContract; private final FontsContractWrapper mFontsContract;
/** /**
...@@ -84,6 +91,19 @@ public class AndroidFontLookupImpl implements AndroidFontLookup { ...@@ -84,6 +91,19 @@ public class AndroidFontLookupImpl implements AndroidFontLookup {
mExpectedFonts = new HashSet<>(mFullFontNameToQuery.keySet()); mExpectedFonts = new HashSet<>(mFullFontNameToQuery.keySet());
} }
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused. These values must stay in sync with enums.xml.
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@IntDef({FetchFontName.GOOGLE_SANS_REGULAR, FetchFontName.GOOGLE_SANS_MEDIUM,
FetchFontName.GOOGLE_SANS_BOLD})
@interface FetchFontName {
int OTHER = 0;
int GOOGLE_SANS_REGULAR = 1;
int GOOGLE_SANS_MEDIUM = 2;
int GOOGLE_SANS_BOLD = 3;
int COUNT = 4;
}
// These values are persisted to logs. Entries should not be renumbered and // These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused. These values must stay in sync with enums.xml. // numeric values should never be reused. These values must stay in sync with enums.xml.
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
...@@ -132,9 +152,11 @@ public class AndroidFontLookupImpl implements AndroidFontLookup { ...@@ -132,9 +152,11 @@ public class AndroidFontLookupImpl implements AndroidFontLookup {
*/ */
@Override @Override
public void matchLocalFontByUniqueName( public void matchLocalFontByUniqueName(
String fontUniqueName, MatchLocalFontByUniqueNameResponse callback) { @NonNull String fontUniqueName, MatchLocalFontByUniqueNameResponse callback) {
long startTimeMs = SystemClock.elapsedRealtime(); long startTimeMs = SystemClock.elapsedRealtime();
logFetchFontName(fontUniqueName);
// Get executor associated with the current thread for running Mojo callback. // Get executor associated with the current thread for running Mojo callback.
Core core = CoreImpl.getInstance(); Core core = CoreImpl.getInstance();
Executor executor = ExecutorFactory.getExecutorForCurrentThread(core); Executor executor = ExecutorFactory.getExecutorForCurrentThread(core);
...@@ -239,20 +261,22 @@ public class AndroidFontLookupImpl implements AndroidFontLookup { ...@@ -239,20 +261,22 @@ public class AndroidFontLookupImpl implements AndroidFontLookup {
* Creates the map from ICU case folded full font name to GMS Core font provider query format, * Creates the map from ICU case folded full font name to GMS Core font provider query format,
* for a selected subset of Android Downloadable fonts. * for a selected subset of Android Downloadable fonts.
* *
* Note: Because the CaseMap.Fold Java API is only available in Android API 29+, these keys have * When adding additional fonts to this map:
* been manually converted from full font name to ICU case folded full font name (i.e. "Google * 1. Add the font to preloaded_fonts.xml, or consider alternatives to prefetch new fonts
* Sans Regular" to "google sans regular") using * programmatically at a different time in initialization as this may affect startup
* `third_party/blink/common/font_unique_name_lookup/icu_fold_case_util.cc`. When further map * performance.
* entries are added in future, consider importing ICU4J as a third_party library to do this * 2. Keys should be ICU case folded full font name. This can be done manually with
* case folding explicitly in Java code instead, or using the native utility via JNI. * icu_fold_case_util.cc, or in Java by importing the ICU4J third_party library. (The
* CaseMap.Fold Java API is only available in Android API 29+.)
* 3. Update the {@link FetchFontName} enum entries.
* *
* @return The created map from font names to queries. * @return The created map from font names to queries.
*/ */
private static Map<String, String> createFullFontNameToQueryMap() { private static Map<String, String> createFullFontNameToQueryMap() {
Map<String, String> map = new HashMap<>(); Map<String, String> map = new HashMap<>();
map.put("google sans regular", createFontQuery("Google Sans", 400)); map.put(GOOGLE_SANS_REGULAR, createFontQuery("Google Sans", 400));
map.put("google sans medium", createFontQuery("Google Sans", 500)); map.put(GOOGLE_SANS_MEDIUM, createFontQuery("Google Sans", 500));
map.put("google sans bold", createFontQuery("Google Sans", 700)); map.put(GOOGLE_SANS_BOLD, createFontQuery("Google Sans", 700));
return map; return map;
} }
...@@ -270,7 +294,27 @@ public class AndroidFontLookupImpl implements AndroidFontLookup { ...@@ -270,7 +294,27 @@ public class AndroidFontLookupImpl implements AndroidFontLookup {
private static void logFetchFontResult(@FetchFontResult int result) { private static void logFetchFontResult(@FetchFontResult int result) {
RecordHistogram.recordEnumeratedHistogram( RecordHistogram.recordEnumeratedHistogram(
FETCH_FONT_HISTOGRAM, result, FetchFontResult.COUNT); FETCH_FONT_RESULT_HISTOGRAM, result, FetchFontResult.COUNT);
}
private static void logFetchFontName(String fontName) {
@FetchFontName
int result;
switch (fontName) {
case GOOGLE_SANS_REGULAR:
result = FetchFontName.GOOGLE_SANS_REGULAR;
break;
case GOOGLE_SANS_MEDIUM:
result = FetchFontName.GOOGLE_SANS_MEDIUM;
break;
case GOOGLE_SANS_BOLD:
result = FetchFontName.GOOGLE_SANS_BOLD;
break;
default:
result = FetchFontName.OTHER;
}
RecordHistogram.recordEnumeratedHistogram(
FETCH_FONT_NAME_HISTOGRAM, result, FetchFontName.COUNT);
} }
@Override @Override
......
...@@ -47,6 +47,7 @@ import org.chromium.base.test.BaseJUnit4ClassRunner; ...@@ -47,6 +47,7 @@ import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.blink.mojom.AndroidFontLookup; import org.chromium.blink.mojom.AndroidFontLookup;
import org.chromium.blink.mojom.AndroidFontLookup.GetUniqueNameLookupTableResponse; import org.chromium.blink.mojom.AndroidFontLookup.GetUniqueNameLookupTableResponse;
import org.chromium.blink.mojom.AndroidFontLookup.MatchLocalFontByUniqueNameResponse; import org.chromium.blink.mojom.AndroidFontLookup.MatchLocalFontByUniqueNameResponse;
import org.chromium.content.browser.font.AndroidFontLookupImpl.FetchFontName;
import org.chromium.content.browser.font.AndroidFontLookupImpl.FetchFontResult; import org.chromium.content.browser.font.AndroidFontLookupImpl.FetchFontResult;
import org.chromium.content_public.browser.test.NativeLibraryTestUtils; import org.chromium.content_public.browser.test.NativeLibraryTestUtils;
import org.chromium.content_public.browser.test.util.CriteriaHelper; import org.chromium.content_public.browser.test.util.CriteriaHelper;
...@@ -171,7 +172,11 @@ public final class AndroidFontLookupImplTest { ...@@ -171,7 +172,11 @@ public final class AndroidFontLookupImplTest {
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, AndroidFontLookupImpl.FETCH_FONT_NAME_HISTOGRAM, FetchFontName.OTHER));
assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.FAILED_UNEXPECTED_NAME)); FetchFontResult.FAILED_UNEXPECTED_NAME));
assertEquals(1, assertEquals(1,
...@@ -196,7 +201,11 @@ public final class AndroidFontLookupImplTest { ...@@ -196,7 +201,11 @@ public final class AndroidFontLookupImplTest {
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, AndroidFontLookupImpl.FETCH_FONT_NAME_HISTOGRAM, FetchFontName.OTHER));
assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.FAILED_STATUS_CODE)); FetchFontResult.FAILED_STATUS_CODE));
assertEquals(1, assertEquals(1,
...@@ -220,7 +229,11 @@ public final class AndroidFontLookupImplTest { ...@@ -220,7 +229,11 @@ public final class AndroidFontLookupImplTest {
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, AndroidFontLookupImpl.FETCH_FONT_NAME_HISTOGRAM, FetchFontName.OTHER));
assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.FAILED_NON_UNIQUE_RESULT)); FetchFontResult.FAILED_NON_UNIQUE_RESULT));
assertEquals(1, assertEquals(1,
...@@ -246,7 +259,11 @@ public final class AndroidFontLookupImplTest { ...@@ -246,7 +259,11 @@ public final class AndroidFontLookupImplTest {
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, AndroidFontLookupImpl.FETCH_FONT_NAME_HISTOGRAM, FetchFontName.OTHER));
assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.FAILED_RESULT_CODE)); FetchFontResult.FAILED_RESULT_CODE));
assertEquals(1, assertEquals(1,
...@@ -269,7 +286,11 @@ public final class AndroidFontLookupImplTest { ...@@ -269,7 +286,11 @@ public final class AndroidFontLookupImplTest {
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, AndroidFontLookupImpl.FETCH_FONT_NAME_HISTOGRAM, FetchFontName.OTHER));
assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.FAILED_EXCEPTION)); FetchFontResult.FAILED_EXCEPTION));
assertEquals(1, assertEquals(1,
...@@ -293,7 +314,7 @@ public final class AndroidFontLookupImplTest { ...@@ -293,7 +314,7 @@ public final class AndroidFontLookupImplTest {
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.FAILED_EXCEPTION)); FetchFontResult.FAILED_EXCEPTION));
// Second request should early out with FAILED_AVOID_RETRY. // Second request should early out with FAILED_AVOID_RETRY.
...@@ -305,9 +326,13 @@ public final class AndroidFontLookupImplTest { ...@@ -305,9 +326,13 @@ public final class AndroidFontLookupImplTest {
timeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL).times(2)) timeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL).times(2))
.call(isNull()); .call(isNull());
assertEquals(2,
RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_NAME_HISTOGRAM, FetchFontName.OTHER));
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.FAILED_AVOID_RETRY)); FetchFontResult.FAILED_AVOID_RETRY));
assertEquals(2, assertEquals(2,
...@@ -333,7 +358,12 @@ public final class AndroidFontLookupImplTest { ...@@ -333,7 +358,12 @@ public final class AndroidFontLookupImplTest {
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_HISTOGRAM, FetchFontResult.SUCCESS)); AndroidFontLookupImpl.FETCH_FONT_NAME_HISTOGRAM, FetchFontName.OTHER));
assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting(
AndroidFontLookupImpl.FETCH_FONT_RESULT_HISTOGRAM,
FetchFontResult.SUCCESS));
assertEquals(1, assertEquals(1,
RecordHistogram.getHistogramTotalCountForTesting( RecordHistogram.getHistogramTotalCountForTesting(
......
...@@ -29774,6 +29774,13 @@ Called by update_feature_policy_enum.py.--> ...@@ -29774,6 +29774,13 @@ Called by update_feature_policy_enum.py.-->
<int value="2" label="CONTENT_DISMISSED"/> <int value="2" label="CONTENT_DISMISSED"/>
</enum> </enum>
<enum name="FetchFontName">
<int value="0" label="Other"/>
<int value="1" label="Google Sans Regular"/>
<int value="2" label="Google Sans Medium"/>
<int value="3" label="Google Sans Bold"/>
</enum>
<enum name="FetchFontResult"> <enum name="FetchFontResult">
<int value="0" label="Success"/> <int value="0" label="Success"/>
<int value="1" label="Failed due to unexpected font name"/> <int value="1" label="Failed due to unexpected font name"/>
...@@ -928,6 +928,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -928,6 +928,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary> </summary>
</histogram> </histogram>
<histogram name="Android.FontLookup.FetchFontName" enum="FetchFontName"
expires_after="M89">
<owner>chouinard@chromium.org</owner>
<owner>twellington@chromium.org</owner>
<summary>
Records font fetch requests by font name. These results may be used to
determine whether it remains worthwhile to preload fonts that are available
for renderer font match requests.
</summary>
</histogram>
<histogram name="Android.FontLookup.FetchFontResult" enum="FetchFontResult" <histogram name="Android.FontLookup.FetchFontResult" enum="FetchFontResult"
expires_after="M89"> expires_after="M89">
<owner>chouinard@chromium.org</owner> <owner>chouinard@chromium.org</owner>
......
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