From 5371f196990828c4677a7a58b1cd41329b8cf32d Mon Sep 17 00:00:00 2001 From: liguisheng Date: Fri, 11 Dec 2020 11:56:29 +0800 Subject: [PATCH 1/5] fix crash in FontCollection::init() when FontFamily is empty --- .../txt/src/minikin/FontCollection.cpp | 40 ++++++++++--------- third_party/txt/src/minikin/FontCollection.h | 10 ++--- third_party/txt/src/txt/font_collection.cc | 7 +++- third_party/txt/src/txt/paragraph_txt.cc | 8 ++++ 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/third_party/txt/src/minikin/FontCollection.cpp b/third_party/txt/src/minikin/FontCollection.cpp index f89cde86e3c04..49eb4931baeef 100644 --- a/third_party/txt/src/minikin/FontCollection.cpp +++ b/third_party/txt/src/minikin/FontCollection.cpp @@ -51,20 +51,11 @@ std::string GetFontLocale(uint32_t langListId) { return langs.size() ? langs[0].getString() : ""; } -FontCollection::FontCollection(std::shared_ptr&& typeface) +FontCollection::FontCollection() : mMaxChar(0) { - std::vector> typefaces; - typefaces.push_back(typeface); - init(typefaces); } -FontCollection::FontCollection( - const vector>& typefaces) - : mMaxChar(0) { - init(typefaces); -} - -void FontCollection::init( +bool FontCollection::init( const vector>& typefaces) { std::scoped_lock _l(gMinikinLock); mId = sNextId++; @@ -91,10 +82,13 @@ void FontCollection::init( mSupportedAxes.insert(supportedAxes.begin(), supportedAxes.end()); } nTypefaces = mFamilies.size(); - LOG_ALWAYS_FATAL_IF(nTypefaces == 0, - "Font collection must have at least one valid typeface"); - LOG_ALWAYS_FATAL_IF(nTypefaces > 254, - "Font collection may only have up to 254 font families."); + if (nTypefaces == 0 || nTypefaces > 254) { + if (nTypefaces == 0) + ALOGE("Font collection must have at least one valid typeface."); + else + ALOGE("Font collection may only have up to 254 font families."); + return false; + } size_t nPages = (mMaxChar + kPageMask) >> kLogCharsPerPage; // TODO: Use variation selector map for mRanges construction. // A font can have a glyph for a base code point and variation selector pair @@ -122,9 +116,12 @@ void FontCollection::init( } range->end = mFamilyVec.size(); } - // See the comment in Range for more details. - LOG_ALWAYS_FATAL_IF(mFamilyVec.size() >= 0xFFFF, - "Exceeded the maximum indexable cmap coverage."); + + if (mFamilyVec.size() >= 0xFFFF) { + ALOGE("Exceeded the maximum indexable cmap coverage."); + return false; + } + return true; } // Special scores for the font fallback. @@ -566,7 +563,12 @@ std::shared_ptr FontCollection::createCollectionWithVariation( } } - return std::shared_ptr(new FontCollection(families)); + auto font_collection = std::make_shared(); + if (!font_collection || !font_collection->init(std::move(families))) { + if (font_collection) font_collection.reset(); + return nullptr; + } + return font_collection; } uint32_t FontCollection::getId() const { diff --git a/third_party/txt/src/minikin/FontCollection.h b/third_party/txt/src/minikin/FontCollection.h index f79c24ba6fa06..7b7f9fe95f7c7 100644 --- a/third_party/txt/src/minikin/FontCollection.h +++ b/third_party/txt/src/minikin/FontCollection.h @@ -29,9 +29,7 @@ namespace minikin { class FontCollection { public: - explicit FontCollection( - const std::vector>& typefaces); - explicit FontCollection(std::shared_ptr&& typeface); + explicit FontCollection(); // libtxt extension: an interface for looking up fallback fonts for characters // that do not match this collection's font families. @@ -78,6 +76,9 @@ class FontCollection { mFallbackFontProvider = std::move(ffp); } + // Initialize the FontCollection. + bool init(const std::vector>& typefaces); + private: static const int kLogCharsPerPage = 8; static const int kPageMask = (1 << kLogCharsPerPage) - 1; @@ -93,9 +94,6 @@ class FontCollection { uint16_t end; }; - // Initialize the FontCollection. - void init(const std::vector>& typefaces); - const std::shared_ptr& getFamilyForChar(uint32_t ch, uint32_t vs, uint32_t langListId, diff --git a/third_party/txt/src/txt/font_collection.cc b/third_party/txt/src/txt/font_collection.cc index 01edddb440167..53335c2db5dfe 100644 --- a/third_party/txt/src/txt/font_collection.cc +++ b/third_party/txt/src/txt/font_collection.cc @@ -206,8 +206,11 @@ FontCollection::GetMinikinFontCollectionForFamilies( } } // Create the minikin font collection. - auto font_collection = - std::make_shared(std::move(minikin_families)); + auto font_collection = std::make_shared(); + if (!font_collection || !font_collection->init(std::move(minikin_families))) { + font_collections_cache_[family_key] = nullptr; + return nullptr; + } if (enable_font_fallback_) { font_collection->set_fallback_font_provider( std::make_unique(shared_from_this())); diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index f395ba8160cc1..e39985206e8b5 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -807,6 +807,14 @@ void ParagraphTxt::Layout(double width) { std::shared_ptr minikin_font_collection = GetMinikinFontCollectionForStyle(run.style()); + if (minikin_font_collection == nullptr) { + FML_LOG(INFO) << "Could not find font collection for families \"" + << (run.style().font_families.empty() + ? "" + : run.style().font_families[0]) + << "\"."; + return; + } // Lay out this run. uint16_t* text_ptr = text_.data(); From 797797b91bd0ed99afcecfd0b1d872c6fe441ab0 Mon Sep 17 00:00:00 2001 From: liguisheng Date: Sun, 13 Dec 2020 22:33:59 +0800 Subject: [PATCH 2/5] fix crash in FontCollection::init() when FontFamily is empty --- third_party/txt/src/minikin/FontCollection.cpp | 16 ++++++++++++---- third_party/txt/src/minikin/FontCollection.h | 12 ++++++++---- third_party/txt/src/txt/font_collection.cc | 4 ++-- third_party/txt/src/txt/paragraph_txt.cc | 5 ----- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/third_party/txt/src/minikin/FontCollection.cpp b/third_party/txt/src/minikin/FontCollection.cpp index 49eb4931baeef..18cc07f8a1a9c 100644 --- a/third_party/txt/src/minikin/FontCollection.cpp +++ b/third_party/txt/src/minikin/FontCollection.cpp @@ -51,12 +51,21 @@ std::string GetFontLocale(uint32_t langListId) { return langs.size() ? langs[0].getString() : ""; } +std::shared_ptr FontCollection::Create( + const std::vector>& typefaces) { + std::shared_ptr font_collection(new minikin::FontCollection()); + if (!font_collection || !font_collection->init(typefaces)) { + return nullptr; + } + return font_collection; +} + FontCollection::FontCollection() : mMaxChar(0) { } bool FontCollection::init( - const vector>& typefaces) { + const std::vector>& typefaces) { std::scoped_lock _l(gMinikinLock); mId = sNextId++; vector lastChar; @@ -563,9 +572,8 @@ std::shared_ptr FontCollection::createCollectionWithVariation( } } - auto font_collection = std::make_shared(); - if (!font_collection || !font_collection->init(std::move(families))) { - if (font_collection) font_collection.reset(); + auto font_collection = minikin::FontCollection::Create(std::move(families)); + if (!font_collection) { return nullptr; } return font_collection; diff --git a/third_party/txt/src/minikin/FontCollection.h b/third_party/txt/src/minikin/FontCollection.h index 7b7f9fe95f7c7..b3ccd3065e0e6 100644 --- a/third_party/txt/src/minikin/FontCollection.h +++ b/third_party/txt/src/minikin/FontCollection.h @@ -28,9 +28,13 @@ namespace minikin { class FontCollection { - public: + private: explicit FontCollection(); + public: + static std::shared_ptr Create( + const std::vector>& typefaces); + // libtxt extension: an interface for looking up fallback fonts for characters // that do not match this collection's font families. class FallbackFontProvider { @@ -76,9 +80,6 @@ class FontCollection { mFallbackFontProvider = std::move(ffp); } - // Initialize the FontCollection. - bool init(const std::vector>& typefaces); - private: static const int kLogCharsPerPage = 8; static const int kPageMask = (1 << kLogCharsPerPage) - 1; @@ -94,6 +95,9 @@ class FontCollection { uint16_t end; }; + // Initialize the FontCollection. + bool init(const std::vector>& typefaces); + const std::shared_ptr& getFamilyForChar(uint32_t ch, uint32_t vs, uint32_t langListId, diff --git a/third_party/txt/src/txt/font_collection.cc b/third_party/txt/src/txt/font_collection.cc index 53335c2db5dfe..ac15b40b80848 100644 --- a/third_party/txt/src/txt/font_collection.cc +++ b/third_party/txt/src/txt/font_collection.cc @@ -206,8 +206,8 @@ FontCollection::GetMinikinFontCollectionForFamilies( } } // Create the minikin font collection. - auto font_collection = std::make_shared(); - if (!font_collection || !font_collection->init(std::move(minikin_families))) { + auto font_collection = minikin::FontCollection::Create(std::move(minikin_families)); + if (!font_collection) { font_collections_cache_[family_key] = nullptr; return nullptr; } diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index e39985206e8b5..39feda5dde9dd 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -808,11 +808,6 @@ void ParagraphTxt::Layout(double width) { std::shared_ptr minikin_font_collection = GetMinikinFontCollectionForStyle(run.style()); if (minikin_font_collection == nullptr) { - FML_LOG(INFO) << "Could not find font collection for families \"" - << (run.style().font_families.empty() - ? "" - : run.style().font_families[0]) - << "\"."; return; } From 0044a13082cf66bd7214ebcdb5b633fb992c3961 Mon Sep 17 00:00:00 2001 From: liguisheng Date: Mon, 14 Dec 2020 12:55:21 +0800 Subject: [PATCH 3/5] format update --- third_party/txt/src/minikin/FontCollection.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/third_party/txt/src/minikin/FontCollection.cpp b/third_party/txt/src/minikin/FontCollection.cpp index 18cc07f8a1a9c..45396a6db4629 100644 --- a/third_party/txt/src/minikin/FontCollection.cpp +++ b/third_party/txt/src/minikin/FontCollection.cpp @@ -91,13 +91,15 @@ bool FontCollection::init( mSupportedAxes.insert(supportedAxes.begin(), supportedAxes.end()); } nTypefaces = mFamilies.size(); - if (nTypefaces == 0 || nTypefaces > 254) { - if (nTypefaces == 0) - ALOGE("Font collection must have at least one valid typeface."); - else - ALOGE("Font collection may only have up to 254 font families."); + if (nTypefaces == 0) { + ALOGE("Font collection must have at least one valid typeface."); return false; } + if (nTypefaces > 254) { + ALOGE("Font collection may only have up to 254 font families."); + return false; + } + size_t nPages = (mMaxChar + kPageMask) >> kLogCharsPerPage; // TODO: Use variation selector map for mRanges construction. // A font can have a glyph for a base code point and variation selector pair @@ -572,11 +574,7 @@ std::shared_ptr FontCollection::createCollectionWithVariation( } } - auto font_collection = minikin::FontCollection::Create(std::move(families)); - if (!font_collection) { - return nullptr; - } - return font_collection; + return minikin::FontCollection::Create(std::move(families)); } uint32_t FontCollection::getId() const { From 7796cfe035d2a8b8e2e84158e15eb2fc62793d7a Mon Sep 17 00:00:00 2001 From: liguisheng Date: Wed, 16 Dec 2020 11:56:55 +0800 Subject: [PATCH 4/5] code format update --- third_party/txt/src/minikin/FontCollection.cpp | 7 +++---- third_party/txt/src/minikin/FontCollection.h | 2 +- third_party/txt/src/txt/font_collection.cc | 3 ++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/third_party/txt/src/minikin/FontCollection.cpp b/third_party/txt/src/minikin/FontCollection.cpp index 45396a6db4629..7d6860ce0fa74 100644 --- a/third_party/txt/src/minikin/FontCollection.cpp +++ b/third_party/txt/src/minikin/FontCollection.cpp @@ -53,16 +53,15 @@ std::string GetFontLocale(uint32_t langListId) { std::shared_ptr FontCollection::Create( const std::vector>& typefaces) { - std::shared_ptr font_collection(new minikin::FontCollection()); + std::shared_ptr font_collection( + new minikin::FontCollection()); if (!font_collection || !font_collection->init(typefaces)) { return nullptr; } return font_collection; } -FontCollection::FontCollection() - : mMaxChar(0) { -} +FontCollection::FontCollection() : mMaxChar(0) {} bool FontCollection::init( const std::vector>& typefaces) { diff --git a/third_party/txt/src/minikin/FontCollection.h b/third_party/txt/src/minikin/FontCollection.h index b3ccd3065e0e6..85ca1f4b86619 100644 --- a/third_party/txt/src/minikin/FontCollection.h +++ b/third_party/txt/src/minikin/FontCollection.h @@ -33,7 +33,7 @@ class FontCollection { public: static std::shared_ptr Create( - const std::vector>& typefaces); + const std::vector>& typefaces); // libtxt extension: an interface for looking up fallback fonts for characters // that do not match this collection's font families. diff --git a/third_party/txt/src/txt/font_collection.cc b/third_party/txt/src/txt/font_collection.cc index ac15b40b80848..353a10e51c08e 100644 --- a/third_party/txt/src/txt/font_collection.cc +++ b/third_party/txt/src/txt/font_collection.cc @@ -206,7 +206,8 @@ FontCollection::GetMinikinFontCollectionForFamilies( } } // Create the minikin font collection. - auto font_collection = minikin::FontCollection::Create(std::move(minikin_families)); + auto font_collection = + minikin::FontCollection::Create(std::move(minikin_families)); if (!font_collection) { font_collections_cache_[family_key] = nullptr; return nullptr; From be54a8128dfd74f1bbc0bb597404f092ae22edca Mon Sep 17 00:00:00 2001 From: liguisheng Date: Wed, 16 Dec 2020 14:58:00 +0800 Subject: [PATCH 5/5] code format update --- third_party/txt/src/minikin/FontCollection.cpp | 3 +-- third_party/txt/src/txt/paragraph_txt.cc | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/third_party/txt/src/minikin/FontCollection.cpp b/third_party/txt/src/minikin/FontCollection.cpp index 7d6860ce0fa74..91d1ac295cdc1 100644 --- a/third_party/txt/src/minikin/FontCollection.cpp +++ b/third_party/txt/src/minikin/FontCollection.cpp @@ -98,7 +98,6 @@ bool FontCollection::init( ALOGE("Font collection may only have up to 254 font families."); return false; } - size_t nPages = (mMaxChar + kPageMask) >> kLogCharsPerPage; // TODO: Use variation selector map for mRanges construction. // A font can have a glyph for a base code point and variation selector pair @@ -573,7 +572,7 @@ std::shared_ptr FontCollection::createCollectionWithVariation( } } - return minikin::FontCollection::Create(std::move(families)); + return FontCollection::Create(std::move(families)); } uint32_t FontCollection::getId() const { diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index 39feda5dde9dd..42943d22b8e16 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -807,7 +807,7 @@ void ParagraphTxt::Layout(double width) { std::shared_ptr minikin_font_collection = GetMinikinFontCollectionForStyle(run.style()); - if (minikin_font_collection == nullptr) { + if (!minikin_font_collection) { return; }