Skip to content

GitLab

T
tangled
Commit 51f0e878 authored by tasak@google.com's avatar tasak@google.com
Browse files
Options

Hold SVGFontFaceElement in SVGDocumentExtensions until StyleRecalc is finished. 

- added HashSet holding SVGFontFaceElement and cleared after document's style recalc is finished, and

- made widthIterator return default value when SVGFontFaceElement is not in document.

BUG=352178
TEST=fast/dom/remove-svg-font-face-element-crash.xhtml

Review URL: https://codereview.chromium.org/198783006

git-svn-id: svn://svn.chromium.org/blink/trunk@169774 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 1032d69a
Hide whitespace changes
Inline Side-by-side
Showing with 83 additions and 5 deletions
third_party/WebKit/LayoutTests/fast/dom/remove-svg-font-face-element-crash.xhtml 0 → 100644
View file @ 51f0e878
<html xmlns="http://www.w3.org/1999/xhtml" xlink="http://www.w3.org/1999/xlink">
<body>
<svg version="1.1" xlink="http://www.w3.org/1999/xlink" width="100%" xmlns="http://www.w3.org/2000/svg">
<g>
<defs>
<font horiz-adv-x="224" id="embeded">
<font-face font-family="embeded" id="fontWillBeRemoved"></font-face>
<glyph horiz-adv-x="1500" unicode="1"></glyph>
<glyph horiz-adv-x="1500" unicode="2"></glyph>
<glyph horiz-adv-x="1500" unicode="3"></glyph>
<glyph horiz-adv-x="1500" unicode="4"></glyph>
<glyph horiz-adv-x="1500" unicode="fi"></glyph>
</font>
</defs>
<g>
<use id="use"></use>
<text font-family="embeded" id="text">fi1234</text>
</g>
</g>
</svg>
</body>
<script>
// crbug.com/352178: Heap-use-after-free in WebCore::SVGFontFaceElement::associatedFontElement.
// PASS if no crash occurs.
if (window.testRunner)
window.testRunner.dumpAsText();
document.execCommand("SelectAll");
function runTest() {
setTimeout(function() {
var fontWillBeRemoved = document.getElementById("fontWillBeRemoved");
var use = document.getElementById("use");
var text = document.getElementById("text");
fontWillBeRemoved.parentNode.removeChild(fontWillBeRemoved);
delete fontWillBeRemoved;
fontWillBeRemoved = null;
use.appendChild(text);
}, 0);
}
document.addEventListener("DOMContentLoaded", runTest, false);
</script>
</html>
third_party/WebKit/Source/core/dom/Document.cpp
View file @ 51f0e878
......@@ -1825,6 +1825,10 @@ void Document::updateStyle(StyleRecalcChange change)
if (m_focusedElement && !m_focusedElement->isFocusable())
clearFocusedElementSoon();
#if ENABLE(SVG_FONTS)
if (svgExtensions())
accessSVGExtensions().removePendingSVGFontFaceElementsForRemoval();
#endif
InspectorInstrumentation::didRecalculateStyle(cookie);
}
......
third_party/WebKit/Source/core/svg/SVGDocumentExtensions.cpp
View file @ 51f0e878
......@@ -26,6 +26,7 @@
#include "core/dom/Document.h"
#include "core/rendering/svg/SVGResourcesCache.h"
#include "core/svg/SVGElement.h"
#include "core/svg/SVGFontFaceElement.h"
#include "core/svg/SVGSVGElement.h"
#include "core/svg/animation/SMILTimeContainer.h"
#include "wtf/TemporaryChange.h"
......@@ -430,6 +431,17 @@ void SVGDocumentExtensions::unregisterSVGFontFaceElement(SVGFontFaceElement* ele
ASSERT(m_svgFontFaceElements.contains(element));
m_svgFontFaceElements.remove(element);
}
void SVGDocumentExtensions::registerPendingSVGFontFaceElementsForRemoval(PassRefPtr<SVGFontFaceElement> font)
{
m_pendingSVGFontFaceElementsForRemoval.add(font);
}
void SVGDocumentExtensions::removePendingSVGFontFaceElementsForRemoval()
{
m_pendingSVGFontFaceElementsForRemoval.clear();
}
#endif
}
third_party/WebKit/Source/core/svg/SVGDocumentExtensions.h
View file @ 51f0e878
......@@ -81,6 +81,9 @@ public:
const HashSet<SVGFontFaceElement*>& svgFontFaceElements() const { return m_svgFontFaceElements; }
void registerSVGFontFaceElement(SVGFontFaceElement*);
void unregisterSVGFontFaceElement(SVGFontFaceElement*);
void registerPendingSVGFontFaceElementsForRemoval(PassRefPtr<SVGFontFaceElement>);
void removePendingSVGFontFaceElementsForRemoval();
#endif
private:
......@@ -88,6 +91,8 @@ private:
HashSet<SVGSVGElement*> m_timeContainers; // For SVG 1.2 support this will need to be made more general.
#if ENABLE(SVG_FONTS)
HashSet<SVGFontFaceElement*> m_svgFontFaceElements;
// SVGFontFaceElements that are pending and scheduled for removal.
HashSet<RefPtr<SVGFontFaceElement> > m_pendingSVGFontFaceElementsForRemoval;
#endif
HashMap<AtomicString, RenderSVGResourceContainer*> m_resources;
HashMap<AtomicString, OwnPtr<SVGPendingElements> > m_pendingResources; // Resources that are pending.
......
third_party/WebKit/Source/core/svg/SVGFontData.cpp
View file @ 51f0e878
......@@ -62,7 +62,7 @@ void SVGFontData::initializeFontData(SimpleFontData* fontData, float fontSize)
ASSERT(fontData);
SVGFontFaceElement* svgFontFaceElement = this->svgFontFaceElement();
ASSERT(svgFontFaceElement);
ASSERT(svgFontFaceElement && svgFontFaceElement->inDocument());
SVGFontElement* svgFontElement = svgFontFaceElement->associatedFontElement();
ASSERT(svgFontElement);
......@@ -123,6 +123,13 @@ float SVGFontData::widthForSVGGlyph(Glyph glyph, float fontSize) const
{
SVGFontFaceElement* svgFontFaceElement = this->svgFontFaceElement();
ASSERT(svgFontFaceElement);
// RenderView::clearSelection is invoked while removing some element, e.g.
// Document::nodeWillBeRemoved => FrameSelection::nodeWillBeRemoved => RenderView::clearSelection.
// Since recalc style has not been executed yet, RenderStyle might have some reference to
// SVGFontFaceElement which was also removed.
// In this case, use default horizontalAdvanceX instead of associatedFontElement's one.
if (!svgFontFaceElement->inDocument())
return m_horizontalAdvanceX * scaleEmToUnits(fontSize, svgFontFaceElement->unitsPerEm());
SVGFontElement* associatedFontElement = svgFontFaceElement->associatedFontElement();
ASSERT(associatedFontElement);
......@@ -155,7 +162,7 @@ bool SVGFontData::applySVGGlyphSelection(WidthIterator& iterator, GlyphData& gly
arabicForms = charactersWithArabicForm(remainingTextInRun, mirror);
SVGFontFaceElement* svgFontFaceElement = this->svgFontFaceElement();
ASSERT(svgFontFaceElement);
ASSERT(svgFontFaceElement && svgFontFaceElement->inDocument());
SVGFontElement* associatedFontElement = svgFontFaceElement->associatedFontElement();
ASSERT(associatedFontElement);
......@@ -226,7 +233,7 @@ bool SVGFontData::fillSVGGlyphPage(GlyphPage* pageToFill, unsigned offset, unsig
ASSERT(fontData->isSVGFont());
SVGFontFaceElement* fontFaceElement = this->svgFontFaceElement();
ASSERT(fontFaceElement);
ASSERT(fontFaceElement && fontFaceElement->inDocument());
SVGFontElement* fontElement = fontFaceElement->associatedFontElement();
ASSERT(fontElement);
......
third_party/WebKit/Source/core/svg/SVGFontFaceElement.cpp
View file @ 51f0e878
......@@ -331,13 +331,15 @@ void SVGFontFaceElement::removedFrom(ContainerNode* rootParent)
if (rootParent->inDocument()) {
m_fontElement = 0;
document().accessSVGExtensions().unregisterSVGFontFaceElement(this);
// FIXME: HTMLTemplateElement's document or imported document can be active?
// If so, we also need to check whether fontSelector() is nullptr or not.
// Otherwise, we will use just document().isActive() here.
if (document().isActive() && document().styleEngine()->fontSelector())
if (document().isActive() && document().styleEngine()->fontSelector()) {
document().styleEngine()->fontSelector()->fontFaceCache()->remove(m_fontFaceRule.get());
document().accessSVGExtensions().registerPendingSVGFontFaceElementsForRemoval(this);
}
m_fontFaceRule->mutableProperties().clear();
document().styleResolverChanged(RecalcStyleDeferred);
} else
ASSERT(!m_fontElement);
......
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