Commit 5f29fcaf authored by Daniel Cheng's avatar Daniel Cheng Committed by Commit Bot

[blink] Record FontDisplay enum using //base/metrics helpers.

This also converts FontDisplay to be a scoped enum, which allows:
- clang to enforce kMaxValue correctness
- autodeduction of the max value by UMA_HISTOGRAM_ENUMERATION.

Bug: 742517, 1047547
Change-Id: Iabb56121750acac4147e20bffe132bde190d9f7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505676
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Auto-Submit: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821901}
parent 6f5ed3f2
...@@ -12,20 +12,20 @@ FontDisplay CSSValueToFontDisplay(const CSSValue* value) { ...@@ -12,20 +12,20 @@ FontDisplay CSSValueToFontDisplay(const CSSValue* value) {
if (auto* identifier_value = DynamicTo<CSSIdentifierValue>(value)) { if (auto* identifier_value = DynamicTo<CSSIdentifierValue>(value)) {
switch (identifier_value->GetValueID()) { switch (identifier_value->GetValueID()) {
case CSSValueID::kAuto: case CSSValueID::kAuto:
return kFontDisplayAuto; return FontDisplay::kAuto;
case CSSValueID::kBlock: case CSSValueID::kBlock:
return kFontDisplayBlock; return FontDisplay::kBlock;
case CSSValueID::kSwap: case CSSValueID::kSwap:
return kFontDisplaySwap; return FontDisplay::kSwap;
case CSSValueID::kFallback: case CSSValueID::kFallback:
return kFontDisplayFallback; return FontDisplay::kFallback;
case CSSValueID::kOptional: case CSSValueID::kOptional:
return kFontDisplayOptional; return FontDisplay::kOptional;
default: default:
break; break;
} }
} }
return kFontDisplayAuto; return FontDisplay::kAuto;
} }
} // namespace blink } // namespace blink
...@@ -9,13 +9,15 @@ namespace blink { ...@@ -9,13 +9,15 @@ namespace blink {
class CSSValue; class CSSValue;
enum FontDisplay { // These values are persisted to logs. Entries should not be renumbered and
kFontDisplayAuto, // numeric values should never be reused.
kFontDisplayBlock, enum class FontDisplay {
kFontDisplaySwap, kAuto,
kFontDisplayFallback, kBlock,
kFontDisplayOptional, kSwap,
kFontDisplayEnumMax, kFallback,
kOptional,
kMaxValue = kOptional,
}; };
FontDisplay CSSValueToFontDisplay(const CSSValue*); FontDisplay CSSValueToFontDisplay(const CSSValue*);
......
...@@ -30,6 +30,7 @@ ...@@ -30,6 +30,7 @@
#include "third_party/blink/renderer/core/css/font_face.h" #include "third_party/blink/renderer/core/css/font_face.h"
#include "base/metrics/histogram_macros.h"
#include "third_party/blink/public/platform/task_type.h" #include "third_party/blink/public/platform/task_type.h"
#include "third_party/blink/renderer/bindings/core/v8/string_or_array_buffer_or_array_buffer_view.h" #include "third_party/blink/renderer/bindings/core/v8/string_or_array_buffer_or_array_buffer_view.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_font_face_descriptors.h" #include "third_party/blink/renderer/bindings/core/v8/v8_font_face_descriptors.h"
...@@ -65,7 +66,6 @@ ...@@ -65,7 +66,6 @@
#include "third_party/blink/renderer/platform/font_family_names.h" #include "third_party/blink/renderer/platform/font_family_names.h"
#include "third_party/blink/renderer/platform/fonts/font_metrics_override.h" #include "third_party/blink/renderer/platform/fonts/font_metrics_override.h"
#include "third_party/blink/renderer/platform/heap/heap.h" #include "third_party/blink/renderer/platform/heap/heap.h"
#include "third_party/blink/renderer/platform/instrumentation/histogram.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h" #include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h" #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
#include "third_party/blink/renderer/platform/wtf/shared_buffer.h" #include "third_party/blink/renderer/platform/wtf/shared_buffer.h"
...@@ -836,10 +836,8 @@ void FontFace::InitCSSFontFace(ExecutionContext* context, const CSSValue& src) { ...@@ -836,10 +836,8 @@ void FontFace::InitCSSFontFace(ExecutionContext* context, const CSSValue& src) {
} }
if (display_) { if (display_) {
DEFINE_THREAD_SAFE_STATIC_LOCAL( UMA_HISTOGRAM_ENUMERATION("WebFont.FontDisplayValue",
EnumerationHistogram, font_display_histogram, CSSValueToFontDisplay(display_.Get()));
("WebFont.FontDisplayValue", kFontDisplayEnumMax));
font_display_histogram.Count(CSSValueToFontDisplay(display_.Get()));
} }
} }
......
...@@ -31,7 +31,7 @@ ...@@ -31,7 +31,7 @@
namespace blink { namespace blink {
bool RemoteFontFaceSource::NeedsInterventionToAlignWithLCPGoal() const { bool RemoteFontFaceSource::NeedsInterventionToAlignWithLCPGoal() const {
DCHECK_EQ(display_, kFontDisplayAuto); DCHECK_EQ(display_, FontDisplay::kAuto);
if (!base::FeatureList::IsEnabled( if (!base::FeatureList::IsEnabled(
features::kAlignFontDisplayAutoTimeoutWithLCPGoal)) { features::kAlignFontDisplayAutoTimeoutWithLCPGoal)) {
return false; return false;
...@@ -51,7 +51,7 @@ bool RemoteFontFaceSource::NeedsInterventionToAlignWithLCPGoal() const { ...@@ -51,7 +51,7 @@ bool RemoteFontFaceSource::NeedsInterventionToAlignWithLCPGoal() const {
RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::DisplayPeriod
RemoteFontFaceSource::ComputeFontDisplayAutoPeriod() const { RemoteFontFaceSource::ComputeFontDisplayAutoPeriod() const {
DCHECK_EQ(display_, kFontDisplayAuto); DCHECK_EQ(display_, FontDisplay::kAuto);
if (NeedsInterventionToAlignWithLCPGoal()) { if (NeedsInterventionToAlignWithLCPGoal()) {
using Mode = features::AlignFontDisplayAutoTimeoutWithLCPGoalMode; using Mode = features::AlignFontDisplayAutoTimeoutWithLCPGoalMode;
Mode mode = Mode mode =
...@@ -79,9 +79,9 @@ RemoteFontFaceSource::ComputeFontDisplayAutoPeriod() const { ...@@ -79,9 +79,9 @@ RemoteFontFaceSource::ComputeFontDisplayAutoPeriod() const {
RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod() RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod()
const { const {
switch (display_) { switch (display_) {
case kFontDisplayAuto: case FontDisplay::kAuto:
return ComputeFontDisplayAutoPeriod(); return ComputeFontDisplayAutoPeriod();
case kFontDisplayBlock: case FontDisplay::kBlock:
switch (phase_) { switch (phase_) {
case kNoLimitExceeded: case kNoLimitExceeded:
case kShortLimitExceeded: case kShortLimitExceeded:
...@@ -90,10 +90,10 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod() ...@@ -90,10 +90,10 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod()
return kSwapPeriod; return kSwapPeriod;
} }
case kFontDisplaySwap: case FontDisplay::kSwap:
return kSwapPeriod; return kSwapPeriod;
case kFontDisplayFallback: case FontDisplay::kFallback:
switch (phase_) { switch (phase_) {
case kNoLimitExceeded: case kNoLimitExceeded:
return kBlockPeriod; return kBlockPeriod;
...@@ -103,7 +103,7 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod() ...@@ -103,7 +103,7 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod()
return kFailurePeriod; return kFailurePeriod;
} }
case kFontDisplayOptional: { case FontDisplay::kOptional: {
const bool use_phase_value = const bool use_phase_value =
!base::FeatureList::IsEnabled( !base::FeatureList::IsEnabled(
features::kFontPreloadingDelaysRendering) || features::kFontPreloadingDelaysRendering) ||
...@@ -132,9 +132,6 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod() ...@@ -132,9 +132,6 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod()
return kSwapPeriod; return kSwapPeriod;
} }
case kFontDisplayEnumMax:
NOTREACHED();
break;
} }
NOTREACHED(); NOTREACHED();
return kSwapPeriod; return kSwapPeriod;
...@@ -295,11 +292,11 @@ FontDisplay RemoteFontFaceSource::GetFontDisplayWithDocumentPolicyCheck( ...@@ -295,11 +292,11 @@ FontDisplay RemoteFontFaceSource::GetFontDisplayWithDocumentPolicyCheck(
const FontSelector* font_selector, const FontSelector* font_selector,
ReportOptions report_option) const { ReportOptions report_option) const {
ExecutionContext* context = font_selector->GetExecutionContext(); ExecutionContext* context = font_selector->GetExecutionContext();
if (display != kFontDisplayFallback && display != kFontDisplayOptional && if (display != FontDisplay::kFallback && display != FontDisplay::kOptional &&
context && context->IsWindow() && context && context->IsWindow() &&
!context->IsFeatureEnabled( !context->IsFeatureEnabled(
mojom::blink::DocumentPolicyFeature::kFontDisplay, report_option)) { mojom::blink::DocumentPolicyFeature::kFontDisplay, report_option)) {
return kFontDisplayOptional; return FontDisplay::kOptional;
} }
return display; return display;
} }
...@@ -315,7 +312,7 @@ bool RemoteFontFaceSource::ShouldTriggerWebFontsIntervention() { ...@@ -315,7 +312,7 @@ bool RemoteFontFaceSource::ShouldTriggerWebFontsIntervention() {
WebEffectiveConnectionType::kTypeOffline <= connection_type && WebEffectiveConnectionType::kTypeOffline <= connection_type &&
connection_type <= WebEffectiveConnectionType::kType3G; connection_type <= WebEffectiveConnectionType::kType3G;
return network_is_slow && display_ == kFontDisplayAuto; return network_is_slow && display_ == FontDisplay::kAuto;
} }
bool RemoteFontFaceSource::IsLowPriorityLoadingAllowedForRemoteFont() const { bool RemoteFontFaceSource::IsLowPriorityLoadingAllowedForRemoteFont() const {
......
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