[webkit-changes] cvs commit: WebCore/khtml/xml dom_elementimpl.cpp
dom_elementimpl.h dom_nodeimpl.cpp
Darin
darin at opensource.apple.com
Mon Dec 19 12:41:48 PST 2005
darin 05/12/19 12:41:48
Modified: . ChangeLog
khtml/html htmlparser.cpp htmlparser.h
khtml/xml dom_elementimpl.cpp dom_elementimpl.h
dom_nodeimpl.cpp
Log:
Reviewed by Maciej.
- fix http://bugzilla.opendarwin.org/show_bug.cgi?id=6143
DOM::ElementImpl should use a RefPtr for the attribute map
* khtml/xml/dom_elementimpl.cpp:
(ElementImpl::ElementImpl): Remove code to initialize the pointer; not needed since
RefPtr gets initialized to 0.
(ElementImpl::~ElementImpl): Remove code to deref the pointer; RefPtr handles that.
(ElementImpl::attributes): Add get() call to get raw pointer.
(ElementImpl::setAttributeMap): Remove code to deref the old map and set the new map.
But added code to clear the element pointer from the old map (missing in the old
version). Also added a FIXME.
(ElementImpl::createAttributeMap): Remove ref(); RefPtr handles that.
(NamedAttrMapImpl::addAttribute): Use a RefPtr to guarantee the element does not go
away in the middle of dispatching DOM events.
(StyledElementImpl::attributeChanged): Clean up code by using the inline function
mappedAttributes() instead of doing type casts.
(StyledElementImpl::parseMappedAttribute): Ditto.
(StyledElementImpl::getClassList): Ditto.
* khtml/xml/dom_elementimpl.h: Make ElementImpl::namedAttrMap be a RefPtr instead
of raw pointer. Added an overload of StyledElementImpl::mappedAttributes for both
const and non-const.
* khtml/xml/dom_nodeimpl.cpp: (DOM::NodeImpl::addChild): Use a RefPtr to ref/deref
the child so that it doesn't leak.
* khtml/html/htmlparser.h: Changed isindex to use a RefPtr.
* khtml/html/htmlparser.cpp:
(HTMLParser::~HTMLParser): Removed now-unneeded ref.
(HTMLParser::isindexCreateErrorCheck): Remove now-unneeded deref/ref.
(HTMLParser::handleIsindex): Put isindex element into a RefPtr. This prevents a
crash that was otherwise happening during layout tests (caused indirectly by
the changes above).
(HTMLParser::startBody): Added call to get().
Revision Changes Path
1.6 +42 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- ChangeLog 19 Dec 2005 19:52:46 -0000 1.5
+++ ChangeLog 19 Dec 2005 20:41:45 -0000 1.6
@@ -1,5 +1,44 @@
2005-12-19 Darin Adler <darin at apple.com>
+ Reviewed by Maciej.
+
+ - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=6143
+ DOM::ElementImpl should use a RefPtr for the attribute map
+
+ * khtml/xml/dom_elementimpl.cpp:
+ (ElementImpl::ElementImpl): Remove code to initialize the pointer; not needed since
+ RefPtr gets initialized to 0.
+ (ElementImpl::~ElementImpl): Remove code to deref the pointer; RefPtr handles that.
+ (ElementImpl::attributes): Add get() call to get raw pointer.
+ (ElementImpl::setAttributeMap): Remove code to deref the old map and set the new map.
+ But added code to clear the element pointer from the old map (missing in the old
+ version). Also added a FIXME.
+ (ElementImpl::createAttributeMap): Remove ref(); RefPtr handles that.
+ (NamedAttrMapImpl::addAttribute): Use a RefPtr to guarantee the element does not go
+ away in the middle of dispatching DOM events.
+ (StyledElementImpl::attributeChanged): Clean up code by using the inline function
+ mappedAttributes() instead of doing type casts.
+ (StyledElementImpl::parseMappedAttribute): Ditto.
+ (StyledElementImpl::getClassList): Ditto.
+
+ * khtml/xml/dom_elementimpl.h: Make ElementImpl::namedAttrMap be a RefPtr instead
+ of raw pointer. Added an overload of StyledElementImpl::mappedAttributes for both
+ const and non-const.
+
+ * khtml/xml/dom_nodeimpl.cpp: (DOM::NodeImpl::addChild): Use a RefPtr to ref/deref
+ the child so that it doesn't leak.
+
+ * khtml/html/htmlparser.h: Changed isindex to use a RefPtr.
+ * khtml/html/htmlparser.cpp:
+ (HTMLParser::~HTMLParser): Removed now-unneeded ref.
+ (HTMLParser::isindexCreateErrorCheck): Remove now-unneeded deref/ref.
+ (HTMLParser::handleIsindex): Put isindex element into a RefPtr. This prevents a
+ crash that was otherwise happening during layout tests (caused indirectly by
+ the changes above).
+ (HTMLParser::startBody): Added call to get().
+
+2005-12-19 Darin Adler <darin at apple.com>
+
Reviewed by Geoff Garen and Eric Seidel.
- fix http://bugzilla.opendarwin.org/show_bug.cgi?id=4923
@@ -57,6 +96,9 @@
- fix http://bugzilla.opendarwin.org/show_bug.cgi?id=4312
XMLHttpRequest headers that have two CRLF sequences lead to Obj-C exception
+ I found this by code inspection after examining a security report about
+ vulnerabilities in other browsers' XMLHttpRequest implementations.
+
* kwq/KWQLoader.mm:
(+[NSDictionary _webcore_dictionaryWithHeaderString:_webcore_dictionaryWithHeaderString:]):
Check length of string before calling characterAtIndex:0 since it will fail for an empty string.
1.125 +8 -14 WebCore/khtml/html/htmlparser.cpp
Index: htmlparser.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/html/htmlparser.cpp,v
retrieving revision 1.124
retrieving revision 1.125
diff -u -r1.124 -r1.125
--- htmlparser.cpp 10 Dec 2005 18:56:00 -0000 1.124
+++ htmlparser.cpp 19 Dec 2005 20:41:45 -0000 1.125
@@ -146,9 +146,6 @@
freeBlock();
setCurrent(0);
-
- if (isindex)
- isindex->deref();
}
void HTMLParser::reset()
@@ -672,10 +669,7 @@
{
NodeImpl *n = handleIsindex(t);
if (!inBody) {
- if (isindex)
- isindex->deref();
isindex = n;
- isindex->ref();
} else {
t->flat = true;
result = n;
@@ -1304,14 +1298,14 @@
}
}
-NodeImpl *HTMLParser::handleIsindex( Token *t )
+NodeImpl* HTMLParser::handleIsindex(Token* t)
{
- NodeImpl *n = new HTMLDivElementImpl(document);
+ NodeImpl* n = new HTMLDivElementImpl(document);
- NamedMappedAttrMapImpl *attrs = t->attrs;
- t->attrs = NULL;
+ NamedMappedAttrMapImpl* attrs = t->attrs;
+ t->attrs = 0;
- HTMLIsIndexElementImpl *isIndex = new HTMLIsIndexElementImpl(document, form);
+ RefPtr<HTMLIsIndexElementImpl> isIndex = new HTMLIsIndexElementImpl(document, form);
isIndex->setAttributeMap(attrs);
isIndex->setAttribute(typeAttr, "khtml_isindex");
@@ -1324,7 +1318,7 @@
n->addChild(new HTMLHRElementImpl(document));
n->addChild(new TextImpl(document, text));
- n->addChild(isIndex);
+ n->addChild(isIndex.get());
n->addChild(new HTMLHRElementImpl(document));
return n;
@@ -1336,8 +1330,8 @@
inBody = true;
- if( isindex ) {
- insertNode( isindex, true /* don't decend into this node */ );
+ if (isindex) {
+ insertNode(isindex.get(), true /* don't decend into this node */);
isindex = 0;
}
}
1.31 +2 -2 WebCore/khtml/html/htmlparser.h
Index: htmlparser.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/html/htmlparser.h,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -r1.30 -r1.31
--- htmlparser.h 8 Nov 2005 08:11:02 -0000 1.30
+++ htmlparser.h 19 Dec 2005 20:41:46 -0000 1.31
@@ -165,8 +165,8 @@
* a possible <isindex> element in the head. Compatibility hack for
* html from the stone age
*/
- DOM::NodeImpl *isindex;
- DOM::NodeImpl *handleIsindex( khtml::Token *t );
+ RefPtr<DOM::NodeImpl> isindex;
+ DOM::NodeImpl* handleIsindex(khtml::Token*);
/*
* inserts the stupid isIndex element.
1.99 +28 -33 WebCore/khtml/xml/dom_elementimpl.cpp
Index: dom_elementimpl.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_elementimpl.cpp,v
retrieving revision 1.98
retrieving revision 1.99
diff -u -r1.98 -r1.99
--- dom_elementimpl.cpp 11 Dec 2005 09:26:18 -0000 1.98
+++ dom_elementimpl.cpp 19 Dec 2005 20:41:46 -0000 1.99
@@ -262,7 +262,6 @@
#ifndef NDEBUG
++ElementImplCounter::count;
#endif
- namedAttrMap = 0;
}
ElementImpl::~ElementImpl()
@@ -270,10 +269,8 @@
#ifndef NDEBUG
--ElementImplCounter::count;
#endif
- if (namedAttrMap) {
+ if (namedAttrMap)
namedAttrMap->detachFromElement();
- namedAttrMap->deref();
- }
}
NodeImpl *ElementImpl::cloneNode(bool deep)
@@ -296,9 +293,8 @@
{
if (namedAttrMap) {
namedAttrMap->removeNamedItem(name, exceptioncode);
- if (exceptioncode == DOMException::NOT_FOUND_ERR) {
+ if (exceptioncode == DOMException::NOT_FOUND_ERR)
exceptioncode = 0;
- }
}
}
@@ -319,7 +315,7 @@
updateStyleAttributeIfNeeded();
if (!readonly && !namedAttrMap)
createAttributeMap();
- return namedAttrMap;
+ return namedAttrMap.get();
}
unsigned short ElementImpl::nodeType() const
@@ -347,10 +343,10 @@
if (name == styleAttr)
updateStyleAttributeIfNeeded();
- if (namedAttrMap) {
- AttributeImpl* a = namedAttrMap->getAttributeItem(name);
- if (a) return a->value();
- }
+ if (namedAttrMap)
+ if (AttributeImpl* a = namedAttrMap->getAttributeItem(name))
+ return a->value();
+
return nullAtom;
}
@@ -408,32 +404,30 @@
return new AttributeImpl(name, value);
}
-void ElementImpl::setAttributeMap( NamedAttrMapImpl* list )
+void ElementImpl::setAttributeMap(NamedAttrMapImpl* list)
{
if (inDocument())
getDocument()->incDOMTreeVersion();
- // If setting the whole map changes the id attribute, we need to
- // call updateId.
+ // If setting the whole map changes the id attribute, we need to call updateId.
AttributeImpl *oldId = namedAttrMap ? namedAttrMap->getAttributeItem(idAttr) : 0;
AttributeImpl *newId = list ? list->getAttributeItem(idAttr) : 0;
- if (oldId || newId) {
+ if (oldId || newId)
updateId(oldId ? oldId->value() : nullAtom, newId ? newId->value() : nullAtom);
- }
- if(namedAttrMap)
- namedAttrMap->deref();
+ if (namedAttrMap)
+ namedAttrMap->element = 0;
namedAttrMap = list;
- if(namedAttrMap) {
- namedAttrMap->ref();
+ if (namedAttrMap) {
namedAttrMap->element = this;
- unsigned int len = namedAttrMap->length();
- for(unsigned int i = 0; i < len; i++)
+ unsigned len = namedAttrMap->length();
+ for (unsigned i = 0; i < len; i++)
attributeChanged(namedAttrMap->attrs[i]);
+ // FIXME: What about attributes that were in the old map that are not in the new map?
}
}
@@ -460,7 +454,6 @@
void ElementImpl::createAttributeMap() const
{
namedAttrMap = new NamedAttrMapImpl(const_cast<ElementImpl*>(this));
- namedAttrMap->ref();
}
bool ElementImpl::isURLAttribute(AttributeImpl *attr) const
@@ -1036,10 +1029,11 @@
// Notify the element that the attribute has been added, and dispatch appropriate mutation events
// Note that element may be null here if we are called from insertAttr() during parsing
- if (element) {
- element->attributeChanged(attr);
- element->dispatchAttrAdditionEvent(attr);
- element->dispatchSubtreeModifiedEvent(false);
+ if (RefPtr<ElementImpl> e = element) {
+ RefPtr<AttributeImpl> a = attr;
+ e->attributeChanged(a.get());
+ e->dispatchAttrAdditionEvent(a.get());
+ e->dispatchSubtreeModifiedEvent(false);
}
}
@@ -1321,7 +1315,7 @@
mappedAttr->setDecl(0);
setChanged();
if (namedAttrMap)
- static_cast<NamedMappedAttrMapImpl*>(namedAttrMap)->declRemoved();
+ mappedAttributes()->declRemoved();
}
bool checkDecl = true;
@@ -1331,7 +1325,7 @@
if (mappedAttr->decl()) {
setChanged();
if (namedAttrMap)
- static_cast<NamedMappedAttrMapImpl*>(namedAttrMap)->declAdded();
+ mappedAttributes()->declAdded();
checkDecl = false;
}
}
@@ -1341,7 +1335,7 @@
mappedAttr->setDecl(decl);
setChanged();
if (namedAttrMap)
- static_cast<NamedMappedAttrMapImpl*>(namedAttrMap)->declAdded();
+ mappedAttributes()->declAdded();
checkDecl = false;
} else
needToParse = true;
@@ -1357,7 +1351,7 @@
mappedAttr->decl()->setParent(0);
mappedAttr->decl()->setNode(0);
if (namedAttrMap)
- static_cast<NamedMappedAttrMapImpl*>(namedAttrMap)->declAdded();
+ mappedAttributes()->declAdded();
}
}
@@ -1386,7 +1380,8 @@
} else if (attr->name() == classAttr) {
// class
setHasClass(!attr->isNull());
- if (namedAttrMap) static_cast<NamedMappedAttrMapImpl*>(namedAttrMap)->parseClassAttribute(attr->value());
+ if (namedAttrMap)
+ mappedAttributes()->parseClassAttribute(attr->value());
setChanged();
} else if (attr->name() == styleAttr) {
setHasStyle(!attr->isNull());
@@ -1424,7 +1419,7 @@
const AtomicStringList* StyledElementImpl::getClassList() const
{
- return namedAttrMap ? static_cast<NamedMappedAttrMapImpl*>(namedAttrMap)->getClassList() : 0;
+ return namedAttrMap ? mappedAttributes()->getClassList() : 0;
}
static inline bool isHexDigit( const QChar &c ) {
1.67 +9 -7 WebCore/khtml/xml/dom_elementimpl.h
Index: dom_elementimpl.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_elementimpl.h,v
retrieving revision 1.66
retrieving revision 1.67
diff -u -r1.66 -r1.67
--- dom_elementimpl.h 11 Dec 2005 09:26:19 -0000 1.66
+++ dom_elementimpl.h 19 Dec 2005 20:41:46 -0000 1.67
@@ -23,8 +23,9 @@
* Boston, MA 02111-1307, USA.
*
*/
-#ifndef _DOM_ELEMENTIMPL_h_
-#define _DOM_ELEMENTIMPL_h_
+
+#ifndef DOM_ELEMENTIMPL_H
+#define DOM_ELEMENTIMPL_H
#include "dom_nodeimpl.h"
#include "xml/dom_stringimpl.h"
@@ -234,7 +235,7 @@
virtual void attributeChanged(AttributeImpl* attr, bool preserveDecls = false) {}
// not part of the DOM
- void setAttributeMap ( NamedAttrMapImpl* list );
+ void setAttributeMap(NamedAttrMapImpl*);
// State of the element.
virtual QString state() { return QString::null; }
@@ -276,10 +277,10 @@
private:
void updateId(const AtomicString& oldId, const AtomicString& newId);
- virtual void updateStyleAttributeIfNeeded() const {};
+ virtual void updateStyleAttributeIfNeeded() const {}
protected: // member variables
- mutable NamedAttrMapImpl *namedAttrMap;
+ mutable RefPtr<NamedAttrMapImpl> namedAttrMap;
QualifiedName m_tagName;
};
@@ -438,8 +439,9 @@
virtual bool isStyledElement() const { return true; }
- bool hasMappedAttributes() const { return namedAttrMap ? static_cast<NamedMappedAttrMapImpl*>(namedAttrMap)->hasMappedAttributes() : false; }
- const NamedMappedAttrMapImpl* mappedAttributes() const { return static_cast<NamedMappedAttrMapImpl*>(namedAttrMap); }
+ NamedMappedAttrMapImpl* mappedAttributes() { return static_cast<NamedMappedAttrMapImpl*>(namedAttrMap.get()); }
+ const NamedMappedAttrMapImpl* mappedAttributes() const { return static_cast<NamedMappedAttrMapImpl*>(namedAttrMap.get()); }
+ bool hasMappedAttributes() const { return namedAttrMap && mappedAttributes()->hasMappedAttributes(); }
bool isMappedAttribute(const QualifiedName& name) const { MappedAttributeEntry res = eNone; mapToEntry(name, res); return res != eNone; }
void addCSSLength(MappedAttributeImpl* attr, int id, const DOMString &value);
1.225 +3 -2 WebCore/khtml/xml/dom_nodeimpl.cpp
Index: dom_nodeimpl.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_nodeimpl.cpp,v
retrieving revision 1.224
retrieving revision 1.225
diff -u -r1.224 -r1.225
--- dom_nodeimpl.cpp 16 Dec 2005 16:13:12 -0000 1.224
+++ dom_nodeimpl.cpp 19 Dec 2005 20:41:47 -0000 1.225
@@ -334,9 +334,10 @@
{
}
-NodeImpl *NodeImpl::addChild(NodeImpl *)
+NodeImpl *NodeImpl::addChild(NodeImpl *newChild)
{
- return 0;
+ RefPtr<NodeImpl> protectNewChild(newChild); // make sure the new child is ref'd and deref'd so we don't leak it
+ return 0;
}
bool NodeImpl::isContentEditable() const
More information about the webkit-changes
mailing list