commit 764d247061e775e04b39897919c3f6271907bfc6 Author: Antonio Sartori Date: Mon Feb 19 14:40:03 2024 +0000 [fonts] Allow GetFontMatchingMetrics to return nullptr This CL makes Document::GetFontMatchingMetrics return null if the document is stopped, and adds nullptr checks on all callers. This avoids nullptr dereferences in the fonts matching metrics reporting code, since a stopped document has no valid ExecutionContext anymore (which is required for metrics reporting). Bug: 324450006, 41481990 Change-Id: Ia5bcc10e4db36f1b971a28c4d4e9395387419583 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5307138 Commit-Queue: Antonio Sartori Reviewed-by: Dominik Röttsches Cr-Commit-Position: refs/heads/main@{#1262379} --- a/third_party/blink/renderer/core/css/css_font_selector_base.cc +++ b/third_party/blink/renderer/core/css/css_font_selector_base.cc @@ -38,10 +38,8 @@ bool CSSFontSelectorBase::IsPlatformFami void CSSFontSelectorBase::ReportEmojiSegmentGlyphCoverage( unsigned num_clusters, unsigned num_broken_clusters) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportEmojiSegmentGlyphCoverage(num_clusters, - num_broken_clusters); - } + GetFontMatchingMetrics()->ReportEmojiSegmentGlyphCoverage( + num_clusters, num_broken_clusters); } void CSSFontSelectorBase::ReportFontFamilyLookupByGenericFamily( @@ -49,49 +47,37 @@ void CSSFontSelectorBase::ReportFontFami UScriptCode script, FontDescription::GenericFamilyType generic_family_type, const AtomicString& resulting_font_name) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportFontFamilyLookupByGenericFamily( - generic_font_family_name, script, generic_family_type, - resulting_font_name); - } + GetFontMatchingMetrics()->ReportFontFamilyLookupByGenericFamily( + generic_font_family_name, script, generic_family_type, + resulting_font_name); } void CSSFontSelectorBase::ReportSuccessfulFontFamilyMatch( const AtomicString& font_family_name) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportSuccessfulFontFamilyMatch(font_family_name); - } + GetFontMatchingMetrics()->ReportSuccessfulFontFamilyMatch(font_family_name); } void CSSFontSelectorBase::ReportFailedFontFamilyMatch( const AtomicString& font_family_name) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportFailedFontFamilyMatch(font_family_name); - } + GetFontMatchingMetrics()->ReportFailedFontFamilyMatch(font_family_name); } void CSSFontSelectorBase::ReportSuccessfulLocalFontMatch( const AtomicString& font_name) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportSuccessfulLocalFontMatch(font_name); - } + GetFontMatchingMetrics()->ReportSuccessfulLocalFontMatch(font_name); } void CSSFontSelectorBase::ReportFailedLocalFontMatch( const AtomicString& font_name) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportFailedLocalFontMatch(font_name); - } + GetFontMatchingMetrics()->ReportFailedLocalFontMatch(font_name); } void CSSFontSelectorBase::ReportFontLookupByUniqueOrFamilyName( const AtomicString& name, const FontDescription& font_description, const SimpleFontData* resulting_font_data) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportFontLookupByUniqueOrFamilyName( - name, font_description, resulting_font_data); - } + GetFontMatchingMetrics()->ReportFontLookupByUniqueOrFamilyName( + name, font_description, resulting_font_data); } void CSSFontSelectorBase::ReportFontLookupByUniqueNameOnly( @@ -99,10 +85,8 @@ void CSSFontSelectorBase::ReportFontLook const FontDescription& font_description, const SimpleFontData* resulting_font_data, bool is_loading_fallback) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportFontLookupByUniqueNameOnly( - name, font_description, resulting_font_data, is_loading_fallback); - } + GetFontMatchingMetrics()->ReportFontLookupByUniqueNameOnly( + name, font_description, resulting_font_data, is_loading_fallback); } void CSSFontSelectorBase::ReportFontLookupByFallbackCharacter( @@ -110,20 +94,16 @@ void CSSFontSelectorBase::ReportFontLook FontFallbackPriority fallback_priority, const FontDescription& font_description, const SimpleFontData* resulting_font_data) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportFontLookupByFallbackCharacter( - fallback_character, fallback_priority, font_description, - resulting_font_data); - } + GetFontMatchingMetrics()->ReportFontLookupByFallbackCharacter( + fallback_character, fallback_priority, font_description, + resulting_font_data); } void CSSFontSelectorBase::ReportLastResortFallbackFontLookup( const FontDescription& font_description, const SimpleFontData* resulting_font_data) { - if (FontMatchingMetrics* font_matching_metrics = GetFontMatchingMetrics()) { - font_matching_metrics->ReportLastResortFallbackFontLookup( - font_description, resulting_font_data); - } + GetFontMatchingMetrics()->ReportLastResortFallbackFontLookup( + font_description, resulting_font_data); } void CSSFontSelectorBase::ReportNotDefGlyph() const { --- a/third_party/blink/renderer/core/css/css_font_selector_base.h +++ b/third_party/blink/renderer/core/css/css_font_selector_base.h @@ -82,8 +82,6 @@ class CORE_EXPORT CSSFontSelectorBase : // issue of `CSSFontSelector` is solved. It will be alive after `TreeScope` // is dead. virtual bool IsAlive() const { return true; } - - // Might return null. virtual FontMatchingMetrics* GetFontMatchingMetrics() const = 0; virtual UseCounter* GetUseCounter() const = 0; --- a/third_party/blink/renderer/core/dom/document.cc +++ b/third_party/blink/renderer/core/dom/document.cc @@ -133,7 +133,6 @@ #include "third_party/blink/renderer/core/dom/document_data.h" #include "third_party/blink/renderer/core/dom/document_fragment.h" #include "third_party/blink/renderer/core/dom/document_init.h" -#include "third_party/blink/renderer/core/dom/document_lifecycle.h" #include "third_party/blink/renderer/core/dom/document_parser_timing.h" #include "third_party/blink/renderer/core/dom/document_part_root.h" #include "third_party/blink/renderer/core/dom/document_type.h" @@ -8086,9 +8085,6 @@ ukm::SourceId Document::UkmSourceID() co } FontMatchingMetrics* Document::GetFontMatchingMetrics() { - if (Lifecycle().GetState() >= DocumentLifecycle::LifecycleState::kStopping) { - return nullptr; - } if (font_matching_metrics_) return font_matching_metrics_.get(); font_matching_metrics_ = std::make_unique( --- a/third_party/blink/renderer/core/dom/document.h +++ b/third_party/blink/renderer/core/dom/document.h @@ -1757,8 +1757,7 @@ class CORE_EXPORT Document : public Cont ukm::SourceId UkmSourceID() const; // Tracks and reports UKM metrics of the number of attempted font family match - // attempts (both successful and not successful) by the page. This will return - // null if the document is stopped. + // attempts (both successful and not successful) by the page. FontMatchingMetrics* GetFontMatchingMetrics(); scoped_refptr GetTaskRunner(TaskType); --- a/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc +++ b/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc @@ -3160,6 +3160,11 @@ TextMetrics* BaseRenderingContext2D::mea const Font& font = AccessFont(canvas); + if (HostAsOffscreenCanvas() && font.GetFontSelector() && + !font.GetFontSelector()->GetExecutionContext()) { + return MakeGarbageCollected(); + } + const CanvasRenderingContext2DState& state = GetState(); TextDirection direction = ToTextDirection(state.GetDirection(), canvas);