Changeset 39149 in webkit

Timestamp:

Dec 9, 2008, 3:36:39 PM (16 years ago)

Author:

weinig@apple.com

Message:

2008-12-09 Sam Weinig <​sam@webkit.org>

Reviewed by Darin Adler.

Fix intermittent crash in buildbot. The CSSCursorImageValues and
SVGCursorElements held onto raw SVGElement pointers without any
guarantee that the element is still around.

We did not fix the design that resulted in this issue, we just fixed
the pointer lifetimes.

• css/CSSCursorImageValue.cpp: (WebCore::CSSCursorImageValue::~CSSCursorImageValue): Zero out the back pointers. (WebCore::CSSCursorImageValue::updateIfSVGCursorIsUsed): Set up a back pointer. (WebCore::CSSCursorImageValue::removeReferencedElement): Added. Used when the element is destroyed.
• css/CSSCursorImageValue.h: Added removeReferencedElement.

• svg/SVGCursorElement.cpp: (WebCore::SVGCursorElement::~SVGCursorElement): Zero out the back pointers. (WebCore::SVGCursorElement::addClient): Set up a back pointer. (WebCore::SVGCursorElement::removeClient): Zero out the back pointer.

• svg/SVGElement.cpp: (WebCore::SVGElement::SVGElement): Initialize back pointers to zero. (WebCore::SVGElement::~SVGElement): Call both the element and cursor image value to remove the element from their sets.
• svg/SVGElement.h: (WebCore::SVGElement::setCursorElement): Added. (WebCore::SVGElement::setCursorImageValue): Added.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/CSSCursorImageValue.cpp (modified) (3 diffs)
• css/CSSCursorImageValue.h (modified) (1 diff)
• svg/SVGCursorElement.cpp (modified) (3 diffs)
• svg/SVGElement.cpp (modified) (5 diffs)
• svg/SVGElement.h (modified) (3 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-12-09  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Fix intermittent crash in buildbot. The CSSCursorImageValues and
+        SVGCursorElements held onto raw SVGElement pointers without any
+        guarantee that the element is still around.
+
+        We did not fix the design that resulted in this issue, we just fixed
+        the pointer lifetimes.
+
+        * css/CSSCursorImageValue.cpp:
+        (WebCore::CSSCursorImageValue::~CSSCursorImageValue): Zero out the back pointers.
+        (WebCore::CSSCursorImageValue::updateIfSVGCursorIsUsed): Set up a back pointer.
+        (WebCore::CSSCursorImageValue::removeReferencedElement): Added. Used when the element
+        is destroyed.
+        * css/CSSCursorImageValue.h: Added removeReferencedElement.
+
+        * svg/SVGCursorElement.cpp:
+        (WebCore::SVGCursorElement::~SVGCursorElement): Zero out the back pointers.
+        (WebCore::SVGCursorElement::addClient): Set up a back pointer.
+        (WebCore::SVGCursorElement::removeClient): Zero out the back pointer.
+
+        * svg/SVGElement.cpp:
+        (WebCore::SVGElement::SVGElement): Initialize back pointers to zero.
+        (WebCore::SVGElement::~SVGElement): Call both the element and cursor image value
+        to remove the element from their sets.
+        * svg/SVGElement.h:
+        (WebCore::SVGElement::setCursorElement): Added.
+        (WebCore::SVGElement::setCursorImageValue): Added.
+
 2008-12-09  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/css/CSSCursorImageValue.cpp

@@ -71 +71 @@
     for (; it != end; ++it) {
         SVGElement* referencedElement = *it;
+        referencedElement->setCursorImageValue(0);
         if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(url, referencedElement->document()))
             cursorElement->removeClient(referencedElement);
@@ -99 +100 @@
         SVGElement* svgElement = static_cast<SVGElement*>(element);
         m_referencedElements.add(svgElement);
+        svgElement->setCursorImageValue(this);
         cursorElement->addClient(svgElement);
         return true;
@@ -121 +123 @@
 }
 
+#if ENABLE(SVG)
+void CSSCursorImageValue::removeReferencedElement(SVGElement* element)
+{
+    m_referencedElements.remove(element);
+}
+#endif
+
 } // namespace WebCore

trunk/WebCore/css/CSSCursorImageValue.h

@@ -45 +45 @@
     virtual StyleCachedImage* cachedImage(DocLoader*);
 
+#if ENABLE(SVG)
+    void removeReferencedElement(SVGElement*);
+#endif
+
 private:
     CSSCursorImageValue(const String& url, const IntPoint& hotspot);

trunk/WebCore/svg/SVGCursorElement.cpp

@@ -45 +45 @@
 SVGCursorElement::~SVGCursorElement()
 {
+    HashSet<SVGElement*>::iterator end = m_clients.end();
+    for (HashSet<SVGElement*>::iterator it = m_clients.begin(); it != end; ++it)
+        (*it)->setCursorElement(0);
 }
 
@@ -68 +71 @@
 {
     m_clients.add(element);
+    element->setCursorElement(this);
 }
 
@@ -73 +77 @@
 {
     m_clients.remove(element);
+    element->setCursorElement(0);
 }
 

trunk/WebCore/svg/SVGElement.cpp

@@ -26 +26 @@
 #include "SVGElement.h"
 
+#include "CSSCursorImageValue.h"
 #include "DOMImplementation.h"
 #include "Document.h"
@@ -34 +35 @@
 #include "HTMLNames.h"
 #include "PlatformString.h"
+#include "RegisteredEventListener.h"
 #include "RenderObject.h"
+#include "SVGCursorElement.h"
 #include "SVGDocumentExtensions.h"
 #include "SVGElementInstance.h"
@@ -43 +46 @@
 #include "SVGUseElement.h"
 #include "XMLNames.h"
-#include "RegisteredEventListener.h"
 
 namespace WebCore {
@@ -52 +54 @@
     : StyledElement(tagName, doc)
     , m_shadowParent(0)
+    , m_cursorElement(0)
+    , m_cursorImageValue(0)
 {
 }
@@ -57 +61 @@
 SVGElement::~SVGElement()
 {
+    if (m_cursorElement)
+        m_cursorElement->removeClient(this);
+    if (m_cursorImageValue)
+        m_cursorImageValue->removeReferencedElement(this);
 }
 

trunk/WebCore/svg/SVGElement.h

@@ -32 +32 @@
 
     class AffineTransform;
+    class CSSCursorImageValue;
     class Document;
+    class SVGCursorElement;
+    class SVGDocumentExtensions;
     class SVGElementInstance;
-    class SVGDocumentExtensions;
     class SVGSVGElement;
 
@@ -118 +120 @@
         }
 
+        void setCursorElement(SVGCursorElement* cursorElement) { m_cursorElement = cursorElement; }
+        void setCursorImageValue(CSSCursorImageValue* cursorImageValue) { m_cursorImageValue = cursorImageValue; }
+
     private:
         friend class SVGElementInstance;
@@ -129 +134 @@
         mutable HashMap<String, const SVGAnimatedPropertyBase*> m_svgPropertyMap;
 
+        SVGCursorElement* m_cursorElement;
+        CSSCursorImageValue* m_cursorImageValue;
+
         HashSet<SVGElementInstance*> m_elementInstances;
     };