[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