Diff
Modified: trunk/Source/WebCore/ChangeLog (199624 => 199625)
--- trunk/Source/WebCore/ChangeLog 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/ChangeLog 2016-04-16 17:53:18 UTC (rev 199625)
@@ -1,3 +1,82 @@
+2016-04-16 Antti Koivisto <[email protected]>
+
+ CSSCursorImageValue shouldn't mutate element during style resolution
+ https://bugs.webkit.org/show_bug.cgi?id=156659
+
+ Reviewed by Darin Adler.
+
+ CSSCursorImageValue::updateIfSVGCursorIsUsed may mutate the argument element.
+
+ This patch removes the code that caches cursor element and image to SVGElement rare data.
+ The whole things is basically unused. CSSCursorImageValue now maintains a weak map to
+ SVGCursorElements directly instead of indirectly via the using SVGElements.
+
+ * css/CSSCursorImageValue.cpp:
+ (WebCore::CSSCursorImageValue::CSSCursorImageValue):
+ (WebCore::CSSCursorImageValue::~CSSCursorImageValue):
+ (WebCore::CSSCursorImageValue::customCSSText):
+ (WebCore::CSSCursorImageValue::updateCursorElement):
+
+ We no longer rely on SVGElement rare data so no need to test for SVGElement.
+
+ (WebCore::CSSCursorImageValue::cursorElementRemoved):
+ (WebCore::CSSCursorImageValue::cursorElementChanged):
+
+ Factor to a function.
+
+ (WebCore::CSSCursorImageValue::cachedImage):
+ (WebCore::CSSCursorImageValue::clearCachedImage):
+ (WebCore::CSSCursorImageValue::equals):
+ (WebCore::CSSCursorImageValue::removeReferencedElement): Deleted.
+
+ Don't track client elements anymore. Just track referenced SVGCursorElements.
+
+ * css/CSSCursorImageValue.h:
+ * css/StyleBuilderCustom.h:
+ (WebCore::StyleBuilderCustom::applyValueCursor):
+
+ No need to make style unique. Initialization is now done in updateSVGCursorElement.
+
+ * svg/SVGCursorElement.cpp:
+ (WebCore::SVGCursorElement::~SVGCursorElement):
+ (WebCore::SVGCursorElement::isSupportedAttribute):
+ (WebCore::SVGCursorElement::parseAttribute):
+ (WebCore::SVGCursorElement::addClient):
+ (WebCore::SVGCursorElement::removeClient):
+
+ Client is now an CSSCursorImageValue rather than SVGElement.
+
+ (WebCore::SVGCursorElement::svgAttributeChanged):
+
+ Instead of invalidating element style just invalidate the CSSCursorImageValue directly.
+
+ (WebCore::SVGCursorElement::addSubresourceAttributeURLs):
+ (WebCore::SVGCursorElement::removeReferencedElement): Deleted.
+ * svg/SVGCursorElement.h:
+ * svg/SVGElement.cpp:
+ (WebCore::SVGElement::~SVGElement):
+ (WebCore::SVGElement::getBoundingBox):
+ (WebCore::SVGElement::correspondingElement):
+ (WebCore::SVGElement::setCursorElement): Deleted.
+ (WebCore::SVGElement::cursorElementRemoved): Deleted.
+ (WebCore::SVGElement::setCursorImageValue): Deleted.
+ (WebCore::SVGElement::cursorImageValueRemoved): Deleted.
+
+ SVGElements no longer need to know about their cursors.
+
+ * svg/SVGElement.h:
+ * svg/SVGElementRareData.h:
+ (WebCore::SVGElementRareData::instanceUpdatesBlocked):
+ (WebCore::SVGElementRareData::setInstanceUpdatesBlocked):
+ (WebCore::SVGElementRareData::correspondingElement):
+ (WebCore::SVGElementRareData::setCorrespondingElement):
+ (WebCore::SVGElementRareData::animatedSMILStyleProperties):
+ (WebCore::SVGElementRareData::ensureAnimatedSMILStyleProperties):
+ (WebCore::SVGElementRareData::cursorElement): Deleted.
+ (WebCore::SVGElementRareData::setCursorElement): Deleted.
+ (WebCore::SVGElementRareData::cursorImageValue): Deleted.
+ (WebCore::SVGElementRareData::setCursorImageValue): Deleted.
+
2016-04-15 Darin Adler <[email protected]>
Reduce use of Deprecated::ScriptXXX classes
Modified: trunk/Source/WebCore/css/CSSCursorImageValue.cpp (199624 => 199625)
--- trunk/Source/WebCore/css/CSSCursorImageValue.cpp 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/css/CSSCursorImageValue.cpp 2016-04-16 17:53:18 UTC (rev 199625)
@@ -44,21 +44,11 @@
namespace WebCore {
-static inline SVGCursorElement* resourceReferencedByCursorElement(const String& url, Document& document)
-{
- Element* element = SVGURIReference::targetElementFromIRIString(url, document);
- if (is<SVGCursorElement>(element))
- return downcast<SVGCursorElement>(element);
-
- return nullptr;
-}
-
CSSCursorImageValue::CSSCursorImageValue(Ref<CSSValue>&& imageValue, bool hasHotSpot, const IntPoint& hotSpot)
: CSSValue(CursorImageClass)
, m_imageValue(WTFMove(imageValue))
, m_hasHotSpot(hasHotSpot)
, m_hotSpot(hotSpot)
- , m_accessedImage(false)
{
}
@@ -72,18 +62,8 @@
{
detachPendingImage();
- if (!isSVGCursor())
- return;
-
- HashSet<SVGElement*>::const_iterator it = m_referencedElements.begin();
- HashSet<SVGElement*>::const_iterator end = m_referencedElements.end();
-
- for (; it != end; ++it) {
- SVGElement* referencedElement = *it;
- referencedElement->cursorImageValueRemoved();
- if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(downcast<CSSImageValue>(m_imageValue.get()).url(), referencedElement->document()))
- cursorElement->removeClient(referencedElement);
- }
+ for (auto* element : m_cursorElements)
+ element->removeClient(*this);
}
String CSSCursorImageValue::customCSSText() const
@@ -99,35 +79,41 @@
return result.toString();
}
-bool CSSCursorImageValue::updateIfSVGCursorIsUsed(Element* element)
+SVGCursorElement* CSSCursorImageValue::updateCursorElement(const Document& document)
{
- if (!element || !element->isSVGElement())
- return false;
-
if (!isSVGCursor())
- return false;
+ return nullptr;
- if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(downcast<CSSImageValue>(m_imageValue.get()).url(), element->document())) {
- // FIXME: This will override hot spot specified in CSS, which is probably incorrect.
- SVGLengthContext lengthContext(0);
- m_hasHotSpot = true;
- float x = roundf(cursorElement->x().value(lengthContext));
- m_hotSpot.setX(static_cast<int>(x));
+ auto* element = SVGURIReference::targetElementFromIRIString(downcast<CSSImageValue>(m_imageValue.get()).url(), document);
+ if (!is<SVGCursorElement>(element))
+ return nullptr;
- float y = roundf(cursorElement->y().value(lengthContext));
- m_hotSpot.setY(static_cast<int>(y));
+ auto& cursorElement = downcast<SVGCursorElement>(*element);
+ if (m_cursorElements.add(&cursorElement).isNewEntry) {
+ cursorElementChanged(cursorElement);
+ cursorElement.addClient(*this);
+ }
+ return &cursorElement;
+}
- if (cachedImageURL() != element->document().completeURL(cursorElement->href()))
- clearCachedImage();
+void CSSCursorImageValue::cursorElementRemoved(SVGCursorElement& cursorElement)
+{
+ m_cursorElements.remove(&cursorElement);
+ clearCachedImage();
+}
- SVGElement& svgElement = downcast<SVGElement>(*element);
- m_referencedElements.add(&svgElement);
- svgElement.setCursorImageValue(this);
- cursorElement->addClient(&svgElement);
- return true;
- }
+void CSSCursorImageValue::cursorElementChanged(SVGCursorElement& cursorElement)
+{
+ clearCachedImage();
- return false;
+ // FIXME: This will override hot spot specified in CSS, which is probably incorrect.
+ SVGLengthContext lengthContext(nullptr);
+ m_hasHotSpot = true;
+ float x = std::round(cursorElement.x().value(lengthContext));
+ m_hotSpot.setX(static_cast<int>(x));
+
+ float y = std::round(cursorElement.y().value(lengthContext));
+ m_hotSpot.setY(static_cast<int>(y));
}
StyleImage* CSSCursorImageValue::cachedImage(CachedResourceLoader& loader, const ResourceLoaderOptions& options)
@@ -137,21 +123,19 @@
return downcast<CSSImageSetValue>(m_imageValue.get()).cachedImageSet(loader, options);
#endif
- if (!m_accessedImage) {
- m_accessedImage = true;
+ auto* cursorElement = loader.document() ? updateCursorElement(*loader.document()) : nullptr;
+ if (!m_isImageValid) {
+ m_isImageValid = true;
// For SVG images we need to lazily substitute in the correct URL. Rather than attempt
// to change the URL of the CSSImageValue (which would then change behavior like cssText),
// we create an alternate CSSImageValue to use.
- if (isSVGCursor() && loader.document()) {
- // FIXME: This will fail if the <cursor> element is in a shadow DOM (bug 59827)
- if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(downcast<CSSImageValue>(m_imageValue.get()).url(), *loader.document())) {
- detachPendingImage();
- Ref<CSSImageValue> svgImageValue(CSSImageValue::create(cursorElement->href()));
- StyleCachedImage* cachedImage = svgImageValue->cachedImage(loader, options);
- m_image = cachedImage;
- return cachedImage;
- }
+ if (cursorElement) {
+ detachPendingImage();
+ Ref<CSSImageValue> svgImageValue(CSSImageValue::create(cursorElement->href()));
+ StyleCachedImage* cachedImage = svgImageValue->cachedImage(loader, options);
+ m_image = cachedImage;
+ return cachedImage;
}
if (is<CSSImageValue>(m_imageValue.get())) {
@@ -202,14 +186,9 @@
{
detachPendingImage();
m_image = nullptr;
- m_accessedImage = false;
+ m_isImageValid = false;
}
-void CSSCursorImageValue::removeReferencedElement(SVGElement* element)
-{
- m_referencedElements.remove(element);
-}
-
bool CSSCursorImageValue::equals(const CSSCursorImageValue& other) const
{
return m_hasHotSpot ? other.m_hasHotSpot && m_hotSpot == other.m_hotSpot : !other.m_hasHotSpot
Modified: trunk/Source/WebCore/css/CSSCursorImageValue.h (199624 => 199625)
--- trunk/Source/WebCore/css/CSSCursorImageValue.h 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/css/CSSCursorImageValue.h 2016-04-16 17:53:18 UTC (rev 199625)
@@ -29,6 +29,7 @@
class Document;
class Element;
+class SVGCursorElement;
class SVGElement;
class CSSCursorImageValue : public CSSValue {
@@ -51,7 +52,7 @@
String customCSSText() const;
- bool updateIfSVGCursorIsUsed(Element*);
+ SVGCursorElement* updateCursorElement(const Document&);
StyleImage* cachedImage(CachedResourceLoader&, const ResourceLoaderOptions&);
StyleImage* cachedOrPendingImage(Document&);
@@ -59,6 +60,9 @@
bool equals(const CSSCursorImageValue&) const;
+ void cursorElementRemoved(SVGCursorElement&);
+ void cursorElementChanged(SVGCursorElement&);
+
private:
CSSCursorImageValue(Ref<CSSValue>&& imageValue, bool hasHotSpot, const IntPoint& hotSpot);
@@ -73,9 +77,8 @@
bool m_hasHotSpot;
IntPoint m_hotSpot;
RefPtr<StyleImage> m_image;
- bool m_accessedImage;
-
- HashSet<SVGElement*> m_referencedElements;
+ bool m_isImageValid { false };
+ HashSet<SVGCursorElement*> m_cursorElements;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/css/StyleBuilderCustom.h (199624 => 199625)
--- trunk/Source/WebCore/css/StyleBuilderCustom.h 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/css/StyleBuilderCustom.h 2016-04-16 17:53:18 UTC (rev 199625)
@@ -1146,8 +1146,6 @@
for (auto& item : list) {
if (is<CSSCursorImageValue>(item.get())) {
auto& image = downcast<CSSCursorImageValue>(item.get());
- if (image.updateIfSVGCursorIsUsed(styleResolver.element())) // Elements with SVG cursors are not allowed to share style.
- styleResolver.style()->setUnique();
styleResolver.style()->addCursor(styleResolver.styleImage(CSSPropertyCursor, image), image.hotSpot());
continue;
}
Modified: trunk/Source/WebCore/svg/SVGCursorElement.cpp (199624 => 199625)
--- trunk/Source/WebCore/svg/SVGCursorElement.cpp 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/svg/SVGCursorElement.cpp 2016-04-16 17:53:18 UTC (rev 199625)
@@ -21,6 +21,7 @@
#include "config.h"
#include "SVGCursorElement.h"
+#include "CSSCursorImageValue.h"
#include "Document.h"
#include "SVGNames.h"
#include "XLinkNames.h"
@@ -59,7 +60,7 @@
SVGCursorElement::~SVGCursorElement()
{
for (auto& client : m_clients)
- client->cursorElementRemoved();
+ client->cursorElementRemoved(*this);
}
bool SVGCursorElement::isSupportedAttribute(const QualifiedName& attrName)
@@ -92,23 +93,16 @@
SVGURIReference::parseAttribute(name, value);
}
-void SVGCursorElement::addClient(SVGElement* element)
+void SVGCursorElement::addClient(CSSCursorImageValue& value)
{
- m_clients.add(element);
- element->setCursorElement(this);
+ m_clients.add(&value);
}
-void SVGCursorElement::removeClient(SVGElement* element)
+void SVGCursorElement::removeClient(CSSCursorImageValue& value)
{
- if (m_clients.remove(element))
- element->cursorElementRemoved();
+ m_clients.remove(&value);
}
-void SVGCursorElement::removeReferencedElement(SVGElement* element)
-{
- m_clients.remove(element);
-}
-
void SVGCursorElement::svgAttributeChanged(const QualifiedName& attrName)
{
if (!isSupportedAttribute(attrName)) {
@@ -118,7 +112,7 @@
InstanceInvalidationGuard guard(*this);
for (auto& client : m_clients)
- client->setNeedsStyleRecalc();
+ client->cursorElementChanged(*this);
}
void SVGCursorElement::addSubresourceAttributeURLs(ListHashSet<URL>& urls) const
Modified: trunk/Source/WebCore/svg/SVGCursorElement.h (199624 => 199625)
--- trunk/Source/WebCore/svg/SVGCursorElement.h 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/svg/SVGCursorElement.h 2016-04-16 17:53:18 UTC (rev 199625)
@@ -31,6 +31,8 @@
namespace WebCore {
+class CSSCursorImageValue;
+
class SVGCursorElement final : public SVGElement,
public SVGTests,
public SVGExternalResourcesRequired,
@@ -40,9 +42,8 @@
virtual ~SVGCursorElement();
- void addClient(SVGElement*);
- void removeClient(SVGElement*);
- void removeReferencedElement(SVGElement*);
+ void addClient(CSSCursorImageValue&);
+ void removeClient(CSSCursorImageValue&);
private:
SVGCursorElement(const QualifiedName&, Document&);
@@ -69,7 +70,7 @@
void synchronizeRequiredExtensions() override { SVGTests::synchronizeRequiredExtensions(this); }
void synchronizeSystemLanguage() override { SVGTests::synchronizeSystemLanguage(this); }
- HashSet<SVGElement*> m_clients;
+ HashSet<CSSCursorImageValue*> m_clients;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/svg/SVGElement.cpp (199624 => 199625)
--- trunk/Source/WebCore/svg/SVGElement.cpp 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/svg/SVGElement.cpp 2016-04-16 17:53:18 UTC (rev 199625)
@@ -26,7 +26,6 @@
#include "config.h"
#include "SVGElement.h"
-#include "CSSCursorImageValue.h"
#include "CSSParser.h"
#include "DOMImplementation.h"
#include "Document.h"
@@ -40,7 +39,6 @@
#include "RenderSVGResource.h"
#include "RenderSVGResourceFilter.h"
#include "RenderSVGResourceMasker.h"
-#include "SVGCursorElement.h"
#include "SVGDocumentExtensions.h"
#include "SVGElementRareData.h"
#include "SVGGraphicsElement.h"
@@ -286,10 +284,6 @@
if (m_svgRareData) {
for (SVGElement* instance : m_svgRareData->instances())
instance->m_svgRareData->setCorrespondingElement(nullptr);
- if (SVGCursorElement* cursorElement = m_svgRareData->cursorElement())
- cursorElement->removeClient(this);
- if (CSSCursorImageValue* cursorImageValue = m_svgRareData->cursorImageValue())
- cursorImageValue->removeReferencedElement(this);
if (SVGElement* correspondingElement = m_svgRareData->correspondingElement())
correspondingElement->m_svgRareData->instances().remove(this);
@@ -444,40 +438,6 @@
return false;
}
-void SVGElement::setCursorElement(SVGCursorElement* cursorElement)
-{
- SVGElementRareData& rareData = ensureSVGRareData();
- if (SVGCursorElement* oldCursorElement = rareData.cursorElement()) {
- if (cursorElement == oldCursorElement)
- return;
- oldCursorElement->removeReferencedElement(this);
- }
- rareData.setCursorElement(cursorElement);
-}
-
-void SVGElement::cursorElementRemoved()
-{
- ASSERT(m_svgRareData);
- m_svgRareData->setCursorElement(nullptr);
-}
-
-void SVGElement::setCursorImageValue(CSSCursorImageValue* cursorImageValue)
-{
- SVGElementRareData& rareData = ensureSVGRareData();
- if (CSSCursorImageValue* oldCursorImageValue = rareData.cursorImageValue()) {
- if (cursorImageValue == oldCursorImageValue)
- return;
- oldCursorImageValue->removeReferencedElement(this);
- }
- rareData.setCursorImageValue(cursorImageValue);
-}
-
-void SVGElement::cursorImageValueRemoved()
-{
- ASSERT(m_svgRareData);
- m_svgRareData->setCursorImageValue(nullptr);
-}
-
SVGElement* SVGElement::correspondingElement() const
{
return m_svgRareData ? m_svgRareData->correspondingElement() : nullptr;
Modified: trunk/Source/WebCore/svg/SVGElement.h (199624 => 199625)
--- trunk/Source/WebCore/svg/SVGElement.h 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/svg/SVGElement.h 2016-04-16 17:53:18 UTC (rev 199625)
@@ -38,12 +38,10 @@
namespace WebCore {
class AffineTransform;
-class CSSCursorImageValue;
class CSSStyleDeclaration;
class CSSValue;
class Document;
class SVGAttributeToPropertyMap;
-class SVGCursorElement;
class SVGDocumentExtensions;
class SVGElementRareData;
class SVGSVGElement;
@@ -106,11 +104,6 @@
bool getBoundingBox(FloatRect&, SVGLocatable::StyleUpdateStrategy = SVGLocatable::AllowStyleUpdate);
- void setCursorElement(SVGCursorElement*);
- void cursorElementRemoved();
- void setCursorImageValue(CSSCursorImageValue*);
- void cursorImageValueRemoved();
-
SVGElement* correspondingElement() const;
SVGUseElement* correspondingUseElement() const;
Modified: trunk/Source/WebCore/svg/SVGElementRareData.h (199624 => 199625)
--- trunk/Source/WebCore/svg/SVGElementRareData.h 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/svg/SVGElementRareData.h 2016-04-16 17:53:18 UTC (rev 199625)
@@ -48,15 +48,9 @@
bool instanceUpdatesBlocked() const { return m_instancesUpdatesBlocked; }
void setInstanceUpdatesBlocked(bool value) { m_instancesUpdatesBlocked = value; }
- SVGCursorElement* cursorElement() const { return m_cursorElement; }
- void setCursorElement(SVGCursorElement* cursorElement) { m_cursorElement = cursorElement; }
-
SVGElement* correspondingElement() { return m_correspondingElement; }
void setCorrespondingElement(SVGElement* correspondingElement) { m_correspondingElement = correspondingElement; }
- CSSCursorImageValue* cursorImageValue() const { return m_cursorImageValue; }
- void setCursorImageValue(CSSCursorImageValue* cursorImageValue) { m_cursorImageValue = cursorImageValue; }
-
MutableStyleProperties* animatedSMILStyleProperties() const { return m_animatedSMILStyleProperties.get(); }
MutableStyleProperties& ensureAnimatedSMILStyleProperties()
{
@@ -84,8 +78,6 @@
private:
HashSet<SVGElement*> m_instances;
- SVGCursorElement* m_cursorElement { nullptr };
- CSSCursorImageValue* m_cursorImageValue { nullptr };
SVGElement* m_correspondingElement { nullptr };
bool m_instancesUpdatesBlocked : 1;
bool m_useOverrideComputedStyle : 1;
Modified: trunk/Source/WebCore/svg/SVGURIReference.cpp (199624 => 199625)
--- trunk/Source/WebCore/svg/SVGURIReference.cpp 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/svg/SVGURIReference.cpp 2016-04-16 17:53:18 UTC (rev 199625)
@@ -39,7 +39,7 @@
return attrName.matches(XLinkNames::hrefAttr);
}
-String SVGURIReference::fragmentIdentifierFromIRIString(const String& url, Document& document)
+String SVGURIReference::fragmentIdentifierFromIRIString(const String& url, const Document& document)
{
size_t start = url.find('#');
if (start == notFound)
@@ -55,7 +55,7 @@
return emptyString();
}
-static inline URL urlFromIRIStringWithFragmentIdentifier(const String& url, Document& document, String& fragmentIdentifier)
+static inline URL urlFromIRIStringWithFragmentIdentifier(const String& url, const Document& document, String& fragmentIdentifier)
{
size_t startOfFragmentIdentifier = url.find('#');
if (startOfFragmentIdentifier == notFound)
@@ -71,7 +71,7 @@
return URL(document.baseURI(), url.substring(startOfFragmentIdentifier));
}
-Element* SVGURIReference::targetElementFromIRIString(const String& iri, Document& document, String* fragmentIdentifier, Document* externalDocument)
+Element* SVGURIReference::targetElementFromIRIString(const String& iri, const Document& document, String* fragmentIdentifier, const Document* externalDocument)
{
// If there's no fragment identifier contained within the IRI string, we can't lookup an element.
String id;
Modified: trunk/Source/WebCore/svg/SVGURIReference.h (199624 => 199625)
--- trunk/Source/WebCore/svg/SVGURIReference.h 2016-04-16 16:08:54 UTC (rev 199624)
+++ trunk/Source/WebCore/svg/SVGURIReference.h 2016-04-16 17:53:18 UTC (rev 199625)
@@ -34,10 +34,10 @@
static bool isKnownAttribute(const QualifiedName&);
static void addSupportedAttributes(HashSet<QualifiedName>&);
- static String fragmentIdentifierFromIRIString(const String&, Document&);
- static Element* targetElementFromIRIString(const String&, Document&, String* = nullptr, Document* = nullptr);
+ static String fragmentIdentifierFromIRIString(const String&, const Document&);
+ static Element* targetElementFromIRIString(const String&, const Document&, String* fragmentIdentifier = nullptr, const Document* externalDocument = nullptr);
- static bool isExternalURIReference(const String& uri, Document& document)
+ static bool isExternalURIReference(const String& uri, const Document& document)
{
// Fragment-only URIs are always internal
if (uri.startsWith('#'))
