From 42e100c0db547b7a06d0131da02b308fdd268885 Mon Sep 17 00:00:00 2001 From: Xiaocheng Hu Date: Wed, 6 Jul 2022 19:58:42 +0000 Subject: [PATCH] Revert "[parallel-text-shaping] CSS changes" This reverts commit 1f36b64dd81cc0c4f4443598fb09307b78c7ac84. Reason for revert: Caused a nullptr deref crbug.com/1335246#c23 Original change's description: > [parallel-text-shaping] CSS changes > > This patch changes "core/css", "platform/{fonts,testing}" to make font > loading, both system and web, to work with font threads for prepration > of parallel text shaping[1]. > > This patch is split from the CL[2][3] to exclude `CSSSegmentedFontFace` > and `CSSFontFace` changes to isolate memory related issues > > > Note: `USE_PARALLEL_TEXT_SHAPING` will be defined on Win and MacOS. > > core/css > * Make objects thread-safe. Mostly adding locks. > * Utilize `ExecutionContext::GetTaskRunner()` to call > `FontMatchingMetrics` in owner execution context from font > threads. > > platform/fonts/font_selector.{cc,h} > * Make `SimpleFontData` parameter in reporting function can be cross > thread. > > platform/fonts/font_test_helper.cc > * Adapt for `FontSelector` changes. > > [1] http://crrev.com/c/3211416 Parallel Text Shaping implementation > [2] http://crrev.com/c/3670315: [parallel-text-shaping] CSS changes > [3] http://crrev.com/c/3703810: [parallel-text-shaping] Use > Persistent in CSSSegmentedFontFace > > Design Doc: https://bit.ly/3GqpJLD > Font slid: https://bit.ly/3JFTSHa > > Bug: 1264280 > Change-Id: Ia1dbb155a6cd7d653cd2b52e03e1945db823bf42 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708289 > Auto-Submit: Yoshifumi Inoue > Reviewed-by: Kent Tamura > Commit-Queue: Kent Tamura > Commit-Queue: Yoshifumi Inoue > Cr-Commit-Position: refs/heads/main@{#1015233} Bug: 1264280, 1335246 Change-Id: I80943ab56c6c86234f857f392e95edfb9e775403 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3748643 Reviewed-by: Chris Harrelson Commit-Queue: Xiaocheng Hu Cr-Commit-Position: refs/heads/main@{#1021325} NOKEYCHECK=True GitOrigin-RevId: a2f82f37ee3c62c899778e18169ae00f0582c365 --- .../renderer/core/css/css_custom_font_data.h | 6 - blink/renderer/core/css/css_font_selector.cc | 15 +- blink/renderer/core/css/css_font_selector.h | 1 - .../core/css/css_font_selector_base.cc | 218 ++---------------- .../core/css/css_font_selector_base.h | 25 +- blink/renderer/core/css/font_face.cc | 3 +- .../core/css/offscreen_font_selector.cc | 3 +- .../core/css/remote_font_face_source.cc | 60 ++--- .../core/css/remote_font_face_source.h | 10 +- .../renderer/platform/fonts/font_selector.cc | 11 +- blink/renderer/platform/fonts/font_selector.h | 10 +- .../platform/testing/font_test_helpers.cc | 9 +- 12 files changed, 53 insertions(+), 318 deletions(-) diff --git a/blink/renderer/core/css/css_custom_font_data.h b/blink/renderer/core/css/css_custom_font_data.h index 12263212e09..a23ffeb10fd 100644 --- a/blink/renderer/core/css/css_custom_font_data.h +++ b/blink/renderer/core/css/css_custom_font_data.h @@ -69,15 +69,9 @@ class CSSCustomFontData final : public CustomFontData { // TODO(Oilpan): consider moving (Custom)FontFace hierarchy to the heap, // thereby making this reference a Member<>. -#if defined(USE_PARALLEL_TEXT_SHAPING) - CrossThreadWeakPersistent font_face_source_; - std::atomic fallback_visibility_; - mutable std::atomic is_loading_{false}; -#else WeakPersistent font_face_source_; FallbackVisibility fallback_visibility_; mutable bool is_loading_ = false; -#endif }; } // namespace blink diff --git a/blink/renderer/core/css/css_font_selector.cc b/blink/renderer/core/css/css_font_selector.cc index 9ca049ae74d..c4cb3ff3f6b 100644 --- a/blink/renderer/core/css/css_font_selector.cc +++ b/blink/renderer/core/css/css_font_selector.cc @@ -40,11 +40,7 @@ namespace blink { CSSFontSelector::CSSFontSelector(const TreeScope& tree_scope) - : CSSFontSelectorBase( - tree_scope.GetDocument().GetExecutionContext()->GetTaskRunner( - TaskType::kInternalDefault)), - tree_scope_(&tree_scope) { - DCHECK(tree_scope.GetDocument().GetExecutionContext()->IsContextThread()); + : tree_scope_(&tree_scope) { DCHECK(tree_scope.GetDocument().GetFrame()); generic_font_family_settings_ = tree_scope.GetDocument() .GetFrame() @@ -61,8 +57,7 @@ CSSFontSelector::CSSFontSelector(const TreeScope& tree_scope) CSSFontSelector::~CSSFontSelector() = default; UseCounter* CSSFontSelector::GetUseCounter() const { - auto* const context = GetExecutionContext(); - return context && context->IsContextThread() ? context : nullptr; + return GetExecutionContext(); } void CSSFontSelector::RegisterForInvalidationCallbacks( @@ -153,7 +148,7 @@ scoped_refptr CSSFontSelector::GetFontData( FontCache::Get().GetFontData(request_description, settings_family_name); ReportFontLookupByUniqueOrFamilyName(settings_family_name, - request_description, font_data); + request_description, font_data.get()); return font_data; } @@ -170,10 +165,6 @@ FontMatchingMetrics* CSSFontSelector::GetFontMatchingMetrics() const { return GetDocument().GetFontMatchingMetrics(); } -bool CSSFontSelector::IsAlive() const { - return tree_scope_; -} - void CSSFontSelector::Trace(Visitor* visitor) const { visitor->Trace(tree_scope_); visitor->Trace(clients_); diff --git a/blink/renderer/core/css/css_font_selector.h b/blink/renderer/core/css/css_font_selector.h index c568a0e40cf..74b7ee2d8dc 100644 --- a/blink/renderer/core/css/css_font_selector.h +++ b/blink/renderer/core/css/css_font_selector.h @@ -82,7 +82,6 @@ class CORE_EXPORT CSSFontSelector : public CSSFontSelectorBase { void DispatchInvalidationCallbacks(FontInvalidationReason); // `CSSFontSelectorBase` overrides - bool IsAlive() const override; FontMatchingMetrics* GetFontMatchingMetrics() const override; UseCounter* GetUseCounter() const override; diff --git a/blink/renderer/core/css/css_font_selector_base.cc b/blink/renderer/core/css/css_font_selector_base.cc index 104ac309387..aeec9b275cb 100644 --- a/blink/renderer/core/css/css_font_selector_base.cc +++ b/blink/renderer/core/css/css_font_selector_base.cc @@ -16,58 +16,17 @@ namespace blink { -CSSFontSelectorBase::CSSFontSelectorBase( - scoped_refptr task_runner) -#if defined(USE_PARALLEL_TEXT_SHAPING) - : task_runner_(task_runner) -#endif -{ - DCHECK(IsContextThread()); -} - void CSSFontSelectorBase::CountUse(WebFeature feature) const { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (IsContextThread()) - return UseCounter::Count(GetUseCounter(), feature); - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce(&CSSFontSelectorBase::CountUse, - WrapCrossThreadPersistent(this), feature)); -#endif + return UseCounter::Count(GetUseCounter(), feature); } AtomicString CSSFontSelectorBase::FamilyNameFromSettings( const FontDescription& font_description, const FontFamily& generic_family_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsContextThread()) { - if (IsWebkitBodyFamily(font_description)) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::CountUse, WrapCrossThreadPersistent(this), - WebFeature::kFontSelectorCSSFontFamilyWebKitPrefixBody)); - } - return FontSelector::FamilyNameFromSettings(generic_font_family_settings_, - font_description, - generic_family_name, nullptr); - } -#endif return FontSelector::FamilyNameFromSettings( generic_font_family_settings_, font_description, generic_family_name, GetUseCounter()); } - -bool CSSFontSelectorBase::IsContextThread() const { -#if defined(USE_PARALLEL_TEXT_SHAPING) - return task_runner_->RunsTasksInCurrentSequence(); -#else - return true; -#endif -} - bool CSSFontSelectorBase::IsPlatformFamilyMatchAvailable( const FontDescription& font_description, const FontFamily& passed_family) { @@ -81,19 +40,6 @@ bool CSSFontSelectorBase::IsPlatformFamilyMatchAvailable( void CSSFontSelectorBase::ReportEmojiSegmentGlyphCoverage( unsigned num_clusters, unsigned num_broken_clusters) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::ReportEmojiSegmentGlyphCoverage, - WrapCrossThreadPersistent(this), num_clusters, - num_broken_clusters)); - return; - } -#endif GetFontMatchingMetrics()->ReportEmojiSegmentGlyphCoverage( num_clusters, num_broken_clusters); } @@ -103,19 +49,6 @@ void CSSFontSelectorBase::ReportFontFamilyLookupByGenericFamily( UScriptCode script, FontDescription::GenericFamilyType generic_family_type, const AtomicString& resulting_font_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::ReportFontFamilyLookupByGenericFamily, - WrapCrossThreadPersistent(this), generic_font_family_name, script, - generic_family_type, resulting_font_name)); - return; - } -#endif GetFontMatchingMetrics()->ReportFontFamilyLookupByGenericFamily( generic_font_family_name, script, generic_family_type, resulting_font_name); @@ -123,153 +56,56 @@ void CSSFontSelectorBase::ReportFontFamilyLookupByGenericFamily( void CSSFontSelectorBase::ReportSuccessfulFontFamilyMatch( const AtomicString& font_family_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce(&CSSFontSelectorBase::ReportFailedFontFamilyMatch, - WrapCrossThreadPersistent(this), font_family_name)); - return; - } -#endif GetFontMatchingMetrics()->ReportSuccessfulFontFamilyMatch(font_family_name); } void CSSFontSelectorBase::ReportFailedFontFamilyMatch( const AtomicString& font_family_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce(&CSSFontSelectorBase::ReportFailedFontFamilyMatch, - WrapCrossThreadPersistent(this), font_family_name)); - return; - } -#endif GetFontMatchingMetrics()->ReportFailedFontFamilyMatch(font_family_name); } void CSSFontSelectorBase::ReportSuccessfulLocalFontMatch( const AtomicString& font_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::ReportSuccessfulLocalFontMatch, - WrapCrossThreadPersistent(this), font_name)); - return; - } -#endif GetFontMatchingMetrics()->ReportSuccessfulLocalFontMatch(font_name); } void CSSFontSelectorBase::ReportFailedLocalFontMatch( const AtomicString& font_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce(&CSSFontSelectorBase::ReportFailedLocalFontMatch, - WrapCrossThreadPersistent(this), font_name)); - return; - } -#endif GetFontMatchingMetrics()->ReportFailedLocalFontMatch(font_name); } void CSSFontSelectorBase::ReportFontLookupByUniqueOrFamilyName( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::ReportFontLookupByUniqueOrFamilyName, - WrapCrossThreadPersistent(this), name, font_description, - resulting_font_data)); - return; - } -#endif + SimpleFontData* resulting_font_data) { GetFontMatchingMetrics()->ReportFontLookupByUniqueOrFamilyName( - name, font_description, resulting_font_data.get()); + name, font_description, resulting_font_data); } void CSSFontSelectorBase::ReportFontLookupByUniqueNameOnly( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data, + SimpleFontData* resulting_font_data, bool is_loading_fallback) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::ReportFontLookupByUniqueNameOnly, - WrapCrossThreadPersistent(this), name, font_description, - resulting_font_data, is_loading_fallback)); - return; - } -#endif GetFontMatchingMetrics()->ReportFontLookupByUniqueNameOnly( - name, font_description, resulting_font_data.get(), is_loading_fallback); + name, font_description, resulting_font_data, is_loading_fallback); } void CSSFontSelectorBase::ReportFontLookupByFallbackCharacter( UChar32 fallback_character, FontFallbackPriority fallback_priority, const FontDescription& font_description, - scoped_refptr resulting_font_data) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::ReportFontLookupByFallbackCharacter, - WrapCrossThreadPersistent(this), fallback_character, - fallback_priority, font_description, resulting_font_data)); - return; - } -#endif + SimpleFontData* resulting_font_data) { GetFontMatchingMetrics()->ReportFontLookupByFallbackCharacter( fallback_character, fallback_priority, font_description, - resulting_font_data.get()); + resulting_font_data); } void CSSFontSelectorBase::ReportLastResortFallbackFontLookup( const FontDescription& font_description, - scoped_refptr resulting_font_data) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce( - &CSSFontSelectorBase::ReportLastResortFallbackFontLookup, - WrapCrossThreadPersistent(this), font_description, - resulting_font_data)); - return; - } -#endif + SimpleFontData* resulting_font_data) { GetFontMatchingMetrics()->ReportLastResortFallbackFontLookup( - font_description, resulting_font_data.get()); + font_description, resulting_font_data); } void CSSFontSelectorBase::ReportNotDefGlyph() const { @@ -278,33 +114,11 @@ void CSSFontSelectorBase::ReportNotDefGlyph() const { void CSSFontSelectorBase::ReportSystemFontFamily( const AtomicString& font_family_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce(&CSSFontSelectorBase::ReportSystemFontFamily, - WrapCrossThreadPersistent(this), font_family_name)); - return; - } -#endif GetFontMatchingMetrics()->ReportSystemFontFamily(font_family_name); } void CSSFontSelectorBase::ReportWebFontFamily( const AtomicString& font_family_name) { -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!IsAlive()) - return; - if (!IsContextThread()) { - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce(&CSSFontSelectorBase::ReportWebFontFamily, - WrapCrossThreadPersistent(this), font_family_name)); - return; - } -#endif GetFontMatchingMetrics()->ReportWebFontFamily(font_family_name); } @@ -313,7 +127,7 @@ void CSSFontSelectorBase::WillUseFontData( const FontFamily& family, const String& text) { if (family.FamilyIsGeneric()) { - if (family.IsPrewarmed() || UNLIKELY(family.FamilyName().IsEmpty())) + if (family.IsPrewarmed()) return; family.SetIsPrewarmed(); // |FamilyNameFromSettings| has a visible impact on the load performance. @@ -321,13 +135,9 @@ void CSSFontSelectorBase::WillUseFontData( // only when the |Font| is shared across elements, and therefore it can't // help when e.g., the font size is different, check once more if this // generic family is already prewarmed. - { - AutoLockForParallelTextShaping guard(prewarmed_generic_families_lock_); - const auto result = - prewarmed_generic_families_.insert(family.FamilyName()); - if (!result.is_new_entry) - return; - } + const auto result = prewarmed_generic_families_.insert(family.FamilyName()); + if (!result.is_new_entry) + return; const AtomicString& family_name = FamilyNameFromSettings(font_description, family); if (!family_name.IsEmpty()) @@ -341,7 +151,7 @@ void CSSFontSelectorBase::WillUseFontData( return; } - if (family.IsPrewarmed() || UNLIKELY(family.FamilyName().IsEmpty())) + if (family.IsPrewarmed()) return; family.SetIsPrewarmed(); FontCache::PrewarmFamily(family.FamilyName()); diff --git a/blink/renderer/core/css/css_font_selector_base.h b/blink/renderer/core/css/css_font_selector_base.h index 5c4047d008f..00eea2809a5 100644 --- a/blink/renderer/core/css/css_font_selector_base.h +++ b/blink/renderer/core/css/css_font_selector_base.h @@ -9,7 +9,6 @@ #include "third_party/blink/renderer/core/css/font_face_cache.h" #include "third_party/blink/renderer/platform/fonts/font_selector.h" #include "third_party/blink/renderer/platform/fonts/generic_font_family_settings.h" -#include "third_party/blink/renderer/platform/fonts/lock_for_parallel_text_shaping.h" #include "third_party/blink/renderer/platform/wtf/forward.h" namespace blink { @@ -47,23 +46,23 @@ class CORE_EXPORT CSSFontSelectorBase : public FontSelector { void ReportFontLookupByUniqueOrFamilyName( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data) override; + SimpleFontData* resulting_font_data) override; void ReportFontLookupByUniqueNameOnly( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data, + SimpleFontData* resulting_font_data, bool is_loading_fallback = false) override; void ReportFontLookupByFallbackCharacter( UChar32 fallback_character, FontFallbackPriority fallback_priority, const FontDescription& font_description, - scoped_refptr resulting_font_data) override; + SimpleFontData* resulting_font_data) override; void ReportLastResortFallbackFontLookup( const FontDescription& font_description, - scoped_refptr resulting_font_data) override; + SimpleFontData* resulting_font_data) override; void ReportFontFamilyLookupByGenericFamily( const AtomicString& generic_font_family_name, @@ -76,18 +75,9 @@ class CORE_EXPORT CSSFontSelectorBase : public FontSelector { void ReportEmojiSegmentGlyphCoverage(unsigned num_clusters, unsigned num_broken_clusters) override; - bool IsContextThread() const override; - void Trace(Visitor*) const override; protected: - explicit CSSFontSelectorBase( - scoped_refptr task_runner); - - // TODO(crbug.com/383860): We should get rid of `IsAlive()` once lifetime - // issue of `CSSFontSelector` is solved. It will be alive after `TreeScope` - // is dead. - virtual bool IsAlive() const { return true; } virtual FontMatchingMetrics* GetFontMatchingMetrics() const = 0; virtual UseCounter* GetUseCounter() const = 0; @@ -99,12 +89,7 @@ class CORE_EXPORT CSSFontSelectorBase : public FontSelector { Member font_face_cache_; GenericFontFamilySettings generic_font_family_settings_; - LockForParallelTextShaping prewarmed_generic_families_lock_; - HashSet prewarmed_generic_families_ - GUARDED_BY(prewarmed_generic_families_lock_); -#if defined(USE_PARALLEL_TEXT_SHAPING) - scoped_refptr task_runner_; -#endif + HashSet prewarmed_generic_families_; }; } // namespace blink diff --git a/blink/renderer/core/css/font_face.cc b/blink/renderer/core/css/font_face.cc index 36cf51bae23..2c8bcf693f4 100644 --- a/blink/renderer/core/css/font_face.cc +++ b/blink/renderer/core/css/font_face.cc @@ -870,8 +870,7 @@ void FontFace::InitCSSFontFace(ExecutionContext* context, const CSSValue& src) { RemoteFontFaceSource* source = MakeGarbageCollected( css_font_face_, font_selector, - CSSValueToFontDisplay(display_.Get()), - GetExecutionContext()->GetTaskRunner(TaskType::kFontLoading)); + CSSValueToFontDisplay(display_.Get())); item.Fetch(context, source); css_font_face_->AddSource(source); } diff --git a/blink/renderer/core/css/offscreen_font_selector.cc b/blink/renderer/core/css/offscreen_font_selector.cc index efae34ddd52..fa1b592f7f5 100644 --- a/blink/renderer/core/css/offscreen_font_selector.cc +++ b/blink/renderer/core/css/offscreen_font_selector.cc @@ -12,8 +12,7 @@ namespace blink { OffscreenFontSelector::OffscreenFontSelector(WorkerGlobalScope* worker) - : CSSFontSelectorBase(worker->GetTaskRunner(TaskType::kInternalDefault)), - worker_(worker) { + : worker_(worker) { DCHECK(worker); font_face_cache_ = MakeGarbageCollected(); FontCache::Get().AddClient(this); diff --git a/blink/renderer/core/css/remote_font_face_source.cc b/blink/renderer/core/css/remote_font_face_source.cc index cf944c61169..11d1ef140c7 100644 --- a/blink/renderer/core/css/remote_font_face_source.cc +++ b/blink/renderer/core/css/remote_font_face_source.cc @@ -28,10 +28,6 @@ #include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h" #include "third_party/blink/renderer/platform/loader/fetch/resource_load_priority.h" #include "third_party/blink/renderer/platform/network/network_state_notifier.h" -#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h" -#include "third_party/blink/renderer/platform/scheduler/public/thread.h" -#include "third_party/blink/renderer/platform/wtf/cross_thread_copier.h" -#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h" namespace blink { @@ -137,16 +133,11 @@ RemoteFontFaceSource::DisplayPeriod RemoteFontFaceSource::ComputePeriod() return kSwapPeriod; } -RemoteFontFaceSource::RemoteFontFaceSource( - CSSFontFace* css_font_face, - FontSelector* font_selector, - FontDisplay display, - scoped_refptr task_runner) +RemoteFontFaceSource::RemoteFontFaceSource(CSSFontFace* css_font_face, + FontSelector* font_selector, + FontDisplay display) : face_(css_font_face), font_selector_(font_selector), -#if defined(USE_PARALLEL_TEXT_SHAPING) - task_runner_(task_runner), -#endif // No need to report the violation here since the font is not loaded yet display_( GetFontDisplayWithDocumentPolicyCheck(display, @@ -185,7 +176,6 @@ void RemoteFontFaceSource::NotifyFinished(Resource* resource) { ExecutionContext* execution_context = font_selector_->GetExecutionContext(); if (!execution_context) return; - DCHECK(execution_context->IsContextThread()); // Prevent promise rejection while shutting down the document. // See crbug.com/960290 auto* window = DynamicTo(execution_context); @@ -382,29 +372,8 @@ RemoteFontFaceSource::CreateLoadingFallbackFontData( } void RemoteFontFaceSource::BeginLoadIfNeeded() { - if (IsLoaded()) - return; - ExecutionContext* const execution_context = - font_selector_->GetExecutionContext(); - if (!execution_context) + if (IsLoaded() || !font_selector_->GetExecutionContext()) return; -#if defined(USE_PARALLEL_TEXT_SHAPING) - if (!execution_context->IsContextThread()) { - // Following tests reache here. - // * fast/css3-text/css3-text-decoration/text-decoration-skip-ink-links.html - // * fast/css3-text/css3-word-break/word-break-break-all-in-span.html - // * virtual/text-antialias/justify-vertical.html - // * virtual/text-antialias/line-break-8bit-after-16bit.html - // Note: |ExecutionContext::GetTaskRunner()| works only for context - // thread, so we ask main thread to handle |BeginLoadIfNeeded()|. - PostCrossThreadTask( - *task_runner_, FROM_HERE, - CrossThreadBindOnce(&RemoteFontFaceSource::BeginLoadIfNeeded, - WrapCrossThreadPersistent(this))); - return; - } -#endif - DCHECK(GetResource()); SetDisplay(face_->GetFontFace()->GetFontDisplay()); @@ -412,19 +381,20 @@ void RemoteFontFaceSource::BeginLoadIfNeeded() { auto* font = To(GetResource()); if (font->StillNeedsLoad()) { if (font->IsLowPriorityLoadingAllowedForRemoteFont()) { - execution_context->AddConsoleMessage(MakeGarbageCollected( - mojom::blink::ConsoleMessageSource::kIntervention, - mojom::blink::ConsoleMessageLevel::kInfo, - "Slow network is detected. See " - "https://www.chromestatus.com/feature/5636954674692096 for more " - "details. Fallback font will be used while loading: " + - font->Url().ElidedString())); + font_selector_->GetExecutionContext()->AddConsoleMessage( + MakeGarbageCollected( + mojom::ConsoleMessageSource::kIntervention, + mojom::ConsoleMessageLevel::kInfo, + "Slow network is detected. See " + "https://www.chromestatus.com/feature/5636954674692096 for more " + "details. Fallback font will be used while loading: " + + font->Url().ElidedString())); // Set the loading priority to VeryLow only when all other clients agreed // that this font is not required for painting the text. font->DidChangePriority(ResourceLoadPriority::kVeryLow, 0); } - if (execution_context->Fetcher()->StartLoad(font)) + if (font_selector_->GetExecutionContext()->Fetcher()->StartLoad(font)) histograms_.LoadStarted(); } @@ -432,7 +402,9 @@ void RemoteFontFaceSource::BeginLoadIfNeeded() { // Note that may have initiated loading without kicking // off the timers. font->StartLoadLimitTimersIfNecessary( - execution_context->GetTaskRunner(TaskType::kInternalLoading).get()); + font_selector_->GetExecutionContext() + ->GetTaskRunner(TaskType::kInternalLoading) + .get()); face_->DidBeginLoad(); } diff --git a/blink/renderer/core/css/remote_font_face_source.h b/blink/renderer/core/css/remote_font_face_source.h index 6fe503d002a..7744db3882a 100644 --- a/blink/renderer/core/css/remote_font_face_source.h +++ b/blink/renderer/core/css/remote_font_face_source.h @@ -23,10 +23,7 @@ class RemoteFontFaceSource final : public CSSFontFaceSource, public: enum Phase { kNoLimitExceeded, kShortLimitExceeded, kLongLimitExceeded }; - RemoteFontFaceSource(CSSFontFace*, - FontSelector*, - FontDisplay, - scoped_refptr); + RemoteFontFaceSource(CSSFontFace*, FontSelector*, FontDisplay); ~RemoteFontFaceSource() override; bool IsLoading() const override; @@ -151,11 +148,6 @@ class RemoteFontFaceSource final : public CSSFontFaceSource, Member face_; Member font_selector_; -#if defined(USE_PARALLEL_TEXT_SHAPING) - // Post `BeginLoadIfNeeded()` unless context thread. - scoped_refptr task_runner_; -#endif - // |nullptr| if font is not loaded or failed to decode. scoped_refptr custom_font_data_; // |nullptr| if font is not loaded or failed to decode. diff --git a/blink/renderer/platform/fonts/font_selector.cc b/blink/renderer/platform/fonts/font_selector.cc index 4d996e65ded..0df0d4fe3cb 100644 --- a/blink/renderer/platform/fonts/font_selector.cc +++ b/blink/renderer/platform/fonts/font_selector.cc @@ -33,13 +33,10 @@ AtomicString FontSelector::FamilyNameFromSettings( return g_empty_atom; if (IsWebkitBodyFamily(font_description)) { - // TODO(yosin): We should make |use_counter| available for font threads. - if (use_counter) { - // TODO(crbug.com/1065468): Remove this counter when it's no longer - // necessary. - UseCounter::Count(use_counter, - WebFeature::kFontSelectorCSSFontFamilyWebKitPrefixBody); - } + // TODO(crbug.com/1065468): Remove this counter when it's no longer + // necessary. + UseCounter::Count(use_counter, + WebFeature::kFontSelectorCSSFontFamilyWebKitPrefixBody); } else if (generic_family_name == font_family_names::kWebkitStandard && !generic_family.FamilyIsGeneric()) { // -webkit-standard is set internally only with a kGenericFamily type in diff --git a/blink/renderer/platform/fonts/font_selector.h b/blink/renderer/platform/fonts/font_selector.h index ff22b4e60e1..66a14e43c55 100644 --- a/blink/renderer/platform/fonts/font_selector.h +++ b/blink/renderer/platform/fonts/font_selector.h @@ -92,7 +92,7 @@ class PLATFORM_EXPORT FontSelector : public FontCacheClient { virtual void ReportFontLookupByUniqueOrFamilyName( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data) = 0; + SimpleFontData* resulting_font_data) = 0; // Called whenever a page attempts to find a local font based on a name. This // only includes lookups where the name is allowed to match PostScript names @@ -100,7 +100,7 @@ class PLATFORM_EXPORT FontSelector : public FontCacheClient { virtual void ReportFontLookupByUniqueNameOnly( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data, + SimpleFontData* resulting_font_data, bool is_loading_fallback = false) = 0; // Called whenever a page attempts to find a local font based on a fallback @@ -109,12 +109,12 @@ class PLATFORM_EXPORT FontSelector : public FontCacheClient { UChar32 fallback_character, FontFallbackPriority fallback_priority, const FontDescription& font_description, - scoped_refptr resulting_font_data) = 0; + SimpleFontData* resulting_font_data) = 0; // Called whenever a page attempts to find a last-resort font. virtual void ReportLastResortFallbackFontLookup( const FontDescription& font_description, - scoped_refptr resulting_font_data) = 0; + SimpleFontData* resulting_font_data) = 0; virtual void ReportNotDefGlyph() const = 0; @@ -137,8 +137,6 @@ class PLATFORM_EXPORT FontSelector : public FontCacheClient { const FontDescription&, const FontFamily& passed_family) = 0; - virtual bool IsContextThread() const = 0; - FontFallbackMap& GetFontFallbackMap(); void Trace(Visitor* visitor) const override; diff --git a/blink/renderer/platform/testing/font_test_helpers.cc b/blink/renderer/platform/testing/font_test_helpers.cc index 49d6857fe63..f6a7a47fc21 100644 --- a/blink/renderer/platform/testing/font_test_helpers.cc +++ b/blink/renderer/platform/testing/font_test_helpers.cc @@ -79,25 +79,24 @@ class TestFontSelector : public FontSelector { void ReportFontLookupByUniqueOrFamilyName( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data) override {} + SimpleFontData* resulting_font_data) override {} void ReportFontLookupByUniqueNameOnly( const AtomicString& name, const FontDescription& font_description, - scoped_refptr resulting_font_data, + SimpleFontData* resulting_font_data, bool is_loading_fallback = false) override {} void ReportFontLookupByFallbackCharacter( UChar32 hint, FontFallbackPriority fallback_priority, const FontDescription& font_description, - scoped_refptr resulting_font_data) override {} + SimpleFontData* resulting_font_data) override {} void ReportLastResortFallbackFontLookup( const FontDescription& font_description, - scoped_refptr resulting_font_data) override {} + SimpleFontData* resulting_font_data) override {} void ReportNotDefGlyph() const override {} void ReportEmojiSegmentGlyphCoverage(unsigned, unsigned) override {} ExecutionContext* GetExecutionContext() const override { return nullptr; } FontFaceCache* GetFontFaceCache() override { return nullptr; } - bool IsContextThread() const override { return true; } void RegisterForInvalidationCallbacks(FontSelectorClient*) override {} void UnregisterForInvalidationCallbacks(FontSelectorClient*) override {}