Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(329)

Unified Diff: Source/core/css/CSSFontSelector.cpp

Issue 18375005: [oilpan] Move CSSFontFace and CSSSegmentedFontFace to the managed heap (Closed) Base URL: svn://svn.chromium.org/blink/branches/oilpan
Patch Set: Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/css/CSSFontSelector.cpp
diff --git a/Source/core/css/CSSFontSelector.cpp b/Source/core/css/CSSFontSelector.cpp
index 3871cdf714f693ee1c50d4f81ede65514fe17b17..234c8ca8f93853eb0b5e0a81ede4a86fe884319f 100644
--- a/Source/core/css/CSSFontSelector.cpp
+++ b/Source/core/css/CSSFontSelector.cpp
@@ -195,7 +195,7 @@ void CSSFontSelector::addFontFaceRule(const Handle<StyleRuleFontFace>& fontFaceR
traitsMask |= FontVariantMask;
// Each item in the src property's list is a single CSSFontFaceSource. Put them all into a CSSFontFace.
- RefPtr<CSSFontFace> fontFace;
+ Handle<CSSFontFace> fontFace;
int srcLength = srcList->length();
@@ -292,29 +292,35 @@ void CSSFontSelector::addFontFaceRule(const Handle<StyleRuleFontFace>& fontFaceR
if (familyName.isEmpty())
continue;
- OwnPtr<Vector<RefPtr<CSSFontFace> > >& familyFontFaces = m_fontFaces.add(familyName, nullptr).iterator->value;
+ // FIXME(oilpan): This is dangerous because the below 'new' can trigger a GC and
+ // some finalizer can get rid of the collection from m_fontFaces.
+ // Then subsequent access to familyFontFaces causes use-after-free.
+ // We should move the collection to the managed heap.
+ CSSFontFaceVectorCollection* familyFontFaces = m_fontFaces.add(familyName, nullptr).iterator->value.get();
if (!familyFontFaces) {
- familyFontFaces = adoptPtr(new Vector<RefPtr<CSSFontFace> >);
+ familyFontFaces = new CSSFontFaceVectorCollection;
+ m_fontFaces.set(familyName, adoptPtr(familyFontFaces));
ASSERT(!m_locallyInstalledFontFaces.contains(familyName));
Vector<unsigned> locallyInstalledFontsTraitsMasks;
fontCache()->getTraitsInFamily(familyName, locallyInstalledFontsTraitsMasks);
if (unsigned numLocallyInstalledFaces = locallyInstalledFontsTraitsMasks.size()) {
- OwnPtr<Vector<RefPtr<CSSFontFace> > > familyLocallyInstalledFaces = adoptPtr(new Vector<RefPtr<CSSFontFace> >);
+ OwnPtr<CSSFontFaceVectorCollection> familyLocallyInstalledFaces = adoptPtr(new CSSFontFaceVectorCollection);
for (unsigned i = 0; i < numLocallyInstalledFaces; ++i) {
- RefPtr<CSSFontFace> locallyInstalledFontFace = CSSFontFace::create(static_cast<FontTraitsMask>(locallyInstalledFontsTraitsMasks[i]), nullptr, true);
+ HandleScope scope;
+ Handle<CSSFontFace> locallyInstalledFontFace = CSSFontFace::create(static_cast<FontTraitsMask>(locallyInstalledFontsTraitsMasks[i]), nullptr, true);
locallyInstalledFontFace->addSource(adoptPtr(new CSSFontFaceSource(familyName)));
ASSERT(locallyInstalledFontFace->isValid());
- familyLocallyInstalledFaces->append(locallyInstalledFontFace);
+ (*familyLocallyInstalledFaces)->append(locallyInstalledFontFace);
}
m_locallyInstalledFontFaces.set(familyName, familyLocallyInstalledFaces.release());
}
}
- familyFontFaces->append(fontFace);
+ (*familyFontFaces)->append(fontFace);
++m_version;
}
@@ -392,7 +398,7 @@ static PassRefPtr<FontData> fontDataForGenericFamily(Document* document, const F
static FontTraitsMask desiredTraitsMaskForComparison;
-static inline bool compareFontFaces(CSSFontFace* first, CSSFontFace* second)
+static inline bool compareFontFaces(Handle<CSSFontFace> first, Handle<CSSFontFace> second)
{
FontTraitsMask firstTraitsMask = first->traitsMask();
FontTraitsMask secondTraitsMask = second->traitsMask();
@@ -482,7 +488,7 @@ PassRefPtr<FontData> CSSFontSelector::getFontData(const FontDescription& fontDes
return 0;
}
- CSSSegmentedFontFace* face = getFontFace(fontDescription, familyName);
+ Handle<CSSSegmentedFontFace> face = getFontFace(fontDescription, familyName);
// If no face was found, then return 0 and let the OS come up with its best match for the name.
if (!face) {
// If we were handed a generic family, but there was no match, go ahead and return the correct font based off our
@@ -496,60 +502,70 @@ PassRefPtr<FontData> CSSFontSelector::getFontData(const FontDescription& fontDes
return face->getFontData(fontDescription);
}
-CSSSegmentedFontFace* CSSFontSelector::getFontFace(const FontDescription& fontDescription, const AtomicString& family)
+Result<CSSSegmentedFontFace> CSSFontSelector::getFontFace(const FontDescription& fontDescription, const AtomicString& family)
{
- Vector<RefPtr<CSSFontFace> >* familyFontFaces = m_fontFaces.get(family);
- if (!familyFontFaces || familyFontFaces->isEmpty())
- return 0;
-
- OwnPtr<HashMap<unsigned, RefPtr<CSSSegmentedFontFace> > >& segmentedFontFaceCache = m_fonts.add(family, nullptr).iterator->value;
- if (!segmentedFontFaceCache)
- segmentedFontFaceCache = adoptPtr(new HashMap<unsigned, RefPtr<CSSSegmentedFontFace> >);
+ CSSFontFaceVectorCollection* familyFontFaces = m_fontFaces.get(family);
+ if (!familyFontFaces || (*familyFontFaces)->isEmpty())
+ return nullptr;
+
+ // FIXME(oilpan): This is dangerous because the below 'new' can trigger a GC and
+ // some finalizer can get rid of the hashmap from m_fonts.
+ // Then subsequent access to segmentedFontFaceCache causes use-after-free.
+ // We should move the hashmap to the managed heap.
+ HashMap<unsigned, Persistent<CSSSegmentedFontFace> >* segmentedFontFaceCache = m_fonts.add(family, nullptr).iterator->value.get();
+ if (!segmentedFontFaceCache) {
+ segmentedFontFaceCache = new HashMap<unsigned, Persistent<CSSSegmentedFontFace> >;
+ m_fonts.set(family, adoptPtr(segmentedFontFaceCache));
+ }
FontTraitsMask traitsMask = fontDescription.traitsMask();
- RefPtr<CSSSegmentedFontFace>& face = segmentedFontFaceCache->add(traitsMask, 0).iterator->value;
- if (!face) {
- face = CSSSegmentedFontFace::create(this);
+ Handle<CSSSegmentedFontFace> face = segmentedFontFaceCache->add(traitsMask, nullptr).iterator->value;
+ if (face)
+ return face;
+
+ face = CSSSegmentedFontFace::create(this);
+ segmentedFontFaceCache->set(traitsMask, face);
+
+ // Collect all matching faces and sort them in order of preference.
+ CollectionRoot<Vector<Member<CSSFontFace>, 32> > candidateFontFaces;
+ for (int i = (*familyFontFaces)->size() - 1; i >= 0; --i) {
+ HandleScope scope;
+ Handle<CSSFontFace> candidate = (*familyFontFaces)->at(i);
+ unsigned candidateTraitsMask = candidate->traitsMask();
+ if ((traitsMask & FontStyleNormalMask) && !(candidateTraitsMask & FontStyleNormalMask))
+ continue;
+ if ((traitsMask & FontVariantNormalMask) && !(candidateTraitsMask & FontVariantNormalMask))
+ continue;
+#if ENABLE(SVG_FONTS)
+ // For SVG Fonts that specify that they only support the "normal" variant, we will assume they are incapable
+ // of small-caps synthesis and just ignore the font face as a candidate.
+ if (candidate->hasSVGFontFaceSource() && (traitsMask & FontVariantSmallCapsMask) && !(candidateTraitsMask & FontVariantSmallCapsMask))
+ continue;
+#endif
+ candidateFontFaces->append(candidate);
+ }
- // Collect all matching faces and sort them in order of preference.
- Vector<CSSFontFace*, 32> candidateFontFaces;
- for (int i = familyFontFaces->size() - 1; i >= 0; --i) {
- CSSFontFace* candidate = familyFontFaces->at(i).get();
+ CSSFontFaceVectorCollection* familyLocallyInstalledFontFaces = m_locallyInstalledFontFaces.get(family);
+ if (familyLocallyInstalledFontFaces) {
+ unsigned numLocallyInstalledFontFaces = (*familyLocallyInstalledFontFaces)->size();
+ for (unsigned i = 0; i < numLocallyInstalledFontFaces; ++i) {
+ Handle<CSSFontFace> candidate = (*familyLocallyInstalledFontFaces)->at(i);
unsigned candidateTraitsMask = candidate->traitsMask();
if ((traitsMask & FontStyleNormalMask) && !(candidateTraitsMask & FontStyleNormalMask))
continue;
if ((traitsMask & FontVariantNormalMask) && !(candidateTraitsMask & FontVariantNormalMask))
continue;
-#if ENABLE(SVG_FONTS)
- // For SVG Fonts that specify that they only support the "normal" variant, we will assume they are incapable
- // of small-caps synthesis and just ignore the font face as a candidate.
- if (candidate->hasSVGFontFaceSource() && (traitsMask & FontVariantSmallCapsMask) && !(candidateTraitsMask & FontVariantSmallCapsMask))
- continue;
-#endif
- candidateFontFaces.append(candidate);
+ candidateFontFaces->append(candidate);
}
-
- if (Vector<RefPtr<CSSFontFace> >* familyLocallyInstalledFontFaces = m_locallyInstalledFontFaces.get(family)) {
- unsigned numLocallyInstalledFontFaces = familyLocallyInstalledFontFaces->size();
- for (unsigned i = 0; i < numLocallyInstalledFontFaces; ++i) {
- CSSFontFace* candidate = familyLocallyInstalledFontFaces->at(i).get();
- unsigned candidateTraitsMask = candidate->traitsMask();
- if ((traitsMask & FontStyleNormalMask) && !(candidateTraitsMask & FontStyleNormalMask))
- continue;
- if ((traitsMask & FontVariantNormalMask) && !(candidateTraitsMask & FontVariantNormalMask))
- continue;
- candidateFontFaces.append(candidate);
- }
- }
-
- desiredTraitsMaskForComparison = traitsMask;
- stable_sort(candidateFontFaces.begin(), candidateFontFaces.end(), compareFontFaces);
- unsigned numCandidates = candidateFontFaces.size();
- for (unsigned i = 0; i < numCandidates; ++i)
- face->appendFontFace(candidateFontFaces[i]);
}
- return face.get();
+
+ desiredTraitsMaskForComparison = traitsMask;
+ stable_sort(candidateFontFaces->begin(), candidateFontFaces->end(), compareFontFaces);
+ unsigned numCandidates = candidateFontFaces->size();
+ for (unsigned i = 0; i < numCandidates; ++i)
+ face->appendFontFace(candidateFontFaces->at(i));
+ return face;
}
void CSSFontSelector::clearDocument()

Powered by Google App Engine
This is Rietveld 408576698