[webkit-changes] cvs commit: WebCore/khtml/xml dom_nodeimpl.cpp
Beth
bdakin at opensource.apple.com
Thu Oct 6 13:46:43 PDT 2005
bdakin 05/10/06 13:46:42
Modified: . ChangeLog
khtml/rendering render_container.cpp
khtml/xml dom_nodeimpl.cpp
Log:
Bug #:
Revision Changes Path
1.210 +35 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.209
retrieving revision 1.210
diff -u -r1.209 -r1.210
--- ChangeLog 6 Oct 2005 19:16:12 -0000 1.209
+++ ChangeLog 6 Oct 2005 20:46:36 -0000 1.210
@@ -1,5 +1,40 @@
2005-10-06 Beth Dakin <bdakin at apple.com>
+ Reviewed by Dave Harrison
+
+ No test cases added because this does not affect layout.
+
+ Added assertions to catch WebCore whenever it tries to dispatch an event
+ while it is modifying the DOM tree. This does not fix any bugs but was
+ inspired by 4134884 and 4132581.
+
+ * khtml/rendering/render_container.cpp: Assert that m_first is anonymous
+ (RenderContainer::destroy):
+ * khtml/xml/dom_nodeimpl.cpp: Added static int eventDispatchForbidden, and
+ forbidEventDispatch() and allowEventDispatch()
+ to wrap code that modifies the tree.
+ (DOM::forbidEventDispatch):
+ (DOM::allowEventDispatch):
+ (DOM::NodeImpl::dispatchEvent): Added assertion.
+ (DOM::NodeImpl::dispatchGenericEvent): Added assertion.
+ (DOM::NodeImpl::dispatchHTMLEvent): Added assertion.
+ (DOM::NodeImpl::dispatchWindowEvent): Added assertion.
+ (DOM::NodeImpl::dispatchMouseEvent): Added assertion.
+ (DOM::NodeImpl::dispatchSimulatedMouseEvent): Added assertion.
+ (DOM::NodeImpl::dispatchUIEvent): Added assertion.
+ (DOM::NodeImpl::dispatchKeyEvent): Added assertion.
+ (DOM::NodeImpl::dispatchWheelEvent): Added assertion.
+ (DOM::NodeImpl::detach): Added assertion.
+ (DOM::ContainerNodeImpl::insertBefore): Wrapped tree-modifying code.
+ (DOM::ContainerNodeImpl::replaceChild): Wrapped tree-modifying code.
+ (DOM::ContainerNodeImpl::removeChild): Wrapped tree-modifying code.
+ (DOM::ContainerNodeImpl::removeChildren): Wrapped tree-modifying code.
+ (DOM::ContainerNodeImpl::appendChild): Wrapped tree-modifying code.
+ (DOM::ContainerNodeImpl::addChild): Wrapped tree-modifying code.
+ (DOM::ContainerNodeImpl::dispatchChildInsertedEvents): Added. assertion.
+
+2005-10-06 Beth Dakin <bdakin at apple.com>
+
Reviewed by Hyatt
No test case added because you need to interact with a page to see this crash.
1.72 +1 -0 WebCore/khtml/rendering/render_container.cpp
Index: render_container.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/rendering/render_container.cpp,v
retrieving revision 1.71
retrieving revision 1.72
diff -u -r1.71 -r1.72
--- render_container.cpp 6 Oct 2005 19:16:13 -0000 1.71
+++ render_container.cpp 6 Oct 2005 20:46:41 -0000 1.72
@@ -63,6 +63,7 @@
continuation()->destroy();
while (m_first) {
+ assert(m_first->isAnonymous());
if (m_first->isListMarker())
m_first->remove();
else
1.198 +60 -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.197
retrieving revision 1.198
diff -u -r1.197 -r1.198
--- dom_nodeimpl.cpp 6 Oct 2005 00:54:06 -0000 1.197
+++ dom_nodeimpl.cpp 6 Oct 2005 20:46:41 -0000 1.198
@@ -95,13 +95,25 @@
};
#ifndef NDEBUG
+
+static int eventDispatchForbidden = 0;
+
+inline void forbidEventDispatch() { ++eventDispatchForbidden; }
+inline void allowEventDispatch() { ASSERT(eventDispatchForbidden > 0); --eventDispatchForbidden; }
+
struct NodeImplCounter {
static int count;
~NodeImplCounter() { /* if (count != 0) fprintf(stderr, "LEAK: %d NodeImpl\n", count); */ }
};
int NodeImplCounter::count = 0;
static NodeImplCounter nodeImplCounter;
-#endif NDEBUG
+
+#else
+
+inline void forbidEventDispatch() { }
+inline void allowEventDispatch() { }
+
+#endif
NodeImpl::NodeImpl(DocumentPtr *doc)
: document(doc),
@@ -507,6 +519,8 @@
bool NodeImpl::dispatchEvent(EventImpl *evt, int &exceptioncode, bool tempEvent)
{
+ ASSERT(eventDispatchForbidden == 0);
+
if (!evt || evt->type().isEmpty()) {
exceptioncode = EventException::_EXCEPTION_OFFSET + EventException::UNSPECIFIED_EVENT_TYPE_ERR;
return false;
@@ -550,6 +564,8 @@
bool NodeImpl::dispatchGenericEvent( EventImpl *evt, int &/*exceptioncode */)
{
+ ASSERT(eventDispatchForbidden == 0);
+
evt->ref();
// ### check that type specified
@@ -648,12 +664,16 @@
bool NodeImpl::dispatchHTMLEvent(const AtomicString &eventType, bool canBubbleArg, bool cancelableArg)
{
+ ASSERT(eventDispatchForbidden == 0);
+
int exceptioncode = 0;
return dispatchEvent(new EventImpl(eventType, canBubbleArg, cancelableArg), exceptioncode, true);
}
bool NodeImpl::dispatchWindowEvent(const AtomicString &eventType, bool canBubbleArg, bool cancelableArg)
{
+ ASSERT(eventDispatchForbidden == 0);
+
int exceptioncode = 0;
EventImpl *evt = new EventImpl(eventType, canBubbleArg, cancelableArg);
evt->ref();
@@ -693,6 +713,8 @@
bool NodeImpl::dispatchMouseEvent(QMouseEvent *_mouse, const AtomicString &overrideType, int overrideDetail)
{
+ ASSERT(eventDispatchForbidden == 0);
+
int detail = overrideDetail; // defaults to 0
AtomicString eventType;
if (!overrideType.isEmpty()) {
@@ -751,6 +773,8 @@
bool NodeImpl::dispatchSimulatedMouseEvent(const AtomicString &eventType)
{
+ ASSERT(eventDispatchForbidden == 0);
+
// Like Gecko, we just pass 0 for everything when we make a fake mouse event.
// Internet Explorer instead gives the current mouse position and state.
return dispatchMouseEvent(eventType, 0, 0, 0, 0, 0, 0, false, false, false, false);
@@ -760,6 +784,8 @@
int clientX, int clientY, int screenX, int screenY,
bool ctrlKey, bool altKey, bool shiftKey, bool metaKey)
{
+ ASSERT(eventDispatchForbidden == 0);
+
if (disabled()) // Don't even send DOM events for disabled controls..
return true;
@@ -815,6 +841,8 @@
bool NodeImpl::dispatchUIEvent(const AtomicString &eventType, int detail)
{
+ ASSERT(eventDispatchForbidden == 0);
+
assert (!( (eventType != DOMFocusInEvent &&
eventType != DOMFocusOutEvent &&
eventType != DOMActivateEvent)));
@@ -907,6 +935,8 @@
bool NodeImpl::dispatchKeyEvent(QKeyEvent *key)
{
+ ASSERT(eventDispatchForbidden == 0);
+
int exceptioncode = 0;
//kdDebug(6010) << "DOM::NodeImpl: dispatching keyboard event" << endl;
KeyboardEventImpl *keyboardEventImpl = new KeyboardEventImpl(key, getDocument()->defaultView());
@@ -931,6 +961,8 @@
void NodeImpl::dispatchWheelEvent(QWheelEvent *e)
{
+ ASSERT(eventDispatchForbidden == 0);
+
if (e->delta() == 0)
return;
@@ -1226,7 +1258,7 @@
void NodeImpl::detach()
{
// assert(m_attached);
-
+
if (m_render)
m_render->destroy();
m_render = 0;
@@ -1823,6 +1855,8 @@
if ( exceptioncode )
return 0;
+ forbidEventDispatch();
+
// Add child in the correct position
if (prev)
prev->setNextSibling(child);
@@ -1838,6 +1872,8 @@
if (attached() && !child->attached())
child->attach();
+ allowEventDispatch();
+
// Dispatch the mutation events
dispatchChildInsertedEvents(child,exceptioncode);
@@ -1884,6 +1920,8 @@
// Add the new child(ren)
while (child) {
+ forbidEventDispatch();
+
nextChild = isFragment ? child->nextSibling() : 0;
// If child is already present in the tree, first remove it
@@ -1907,6 +1945,8 @@
if (attached() && !child->attached())
child->attach();
+ allowEventDispatch();
+
// Dispatch the mutation events
dispatchChildInsertedEvents(child,exceptioncode);
@@ -1949,6 +1989,8 @@
if (exceptioncode)
return 0;
+ forbidEventDispatch();
+
// Remove from rendering tree
if (oldChild->attached())
oldChild->detach();
@@ -1969,6 +2011,8 @@
getDocument()->setDocumentChanged(true);
+ allowEventDispatch();
+
// Dispatch post-removal mutation events
dispatchSubtreeModifiedEvent();
@@ -1994,6 +2038,8 @@
// Fire removed from document mutation events.
dispatchChildRemovalEvents(n, exceptionCode);
+ forbidEventDispatch();
+
if (n->attached())
n->detach();
n->setPreviousSibling(0);
@@ -2006,6 +2052,8 @@
n->deref();
_first = next;
+
+ allowEventDispatch();
}
_last = 0;
@@ -2040,6 +2088,8 @@
NodeImpl *child = isFragment ? newChild->firstChild() : newChild;
while (child) {
+ forbidEventDispatch();
+
nextChild = isFragment ? child->nextSibling() : 0;
// If child is already present in the tree, first remove it
@@ -2068,6 +2118,8 @@
// ### should we detach() it first if it's already attached?
if (attached() && !child->attached())
child->attach();
+
+ allowEventDispatch();
// Dispatch the mutation events
dispatchChildInsertedEvents(child,exceptioncode);
@@ -2132,6 +2184,8 @@
if (getDocument()->isHTMLDocument() && !childAllowed(newChild))
return 0;
+ forbidEventDispatch();
+
// just add it...
newChild->setParent(this);
@@ -2150,6 +2204,8 @@
newChild->insertedIntoDocument();
childrenChanged();
+ allowEventDispatch();
+
if(newChild->nodeType() == Node::ELEMENT_NODE)
return newChild;
return this;
@@ -2424,6 +2480,8 @@
void ContainerNodeImpl::dispatchChildInsertedEvents( NodeImpl *child, int &exceptioncode )
{
+ ASSERT(eventDispatchForbidden == 0);
+
if (inDocument())
child->insertedIntoDocument();
else
More information about the webkit-changes
mailing list