Changeset 14055 in webkit

Timestamp:

Apr 25, 2006 12:33:22 PM (18 years ago)

Author:

eseidel

Message:

2006-04-25 Eric Seidel <​eseidel@apple.com>

Reviewed by ggaren.

Fixes for our 3rd and 5th most common crashtracers:
<rdar://problem/4233435> CrashTracer: 2698 crashes in Safari at com.apple.WebCore: khtml::RenderBlock::addChildToFlow + 156
<rdar://problem/4129744> [REGRESSION]CrashTracer: ..400 crashes at com.apple.WebCore: DOM::NodeImpl::createRendererIfNeeded + 44

Test case landed on trunk but not on branch.

• khtml/xml/dom_docimpl.cpp: (DocumentImpl::setFocusNode): use m_inDetach bool copied from trunk
• khtml/xml/dom_nodeimpl.cpp: (NodeImpl::NodeImpl): (NodeImpl::detach): set m_inDetach (NodeImpl::createRendererIfNeeded): (NodeBaseImpl::removeChildren): remove node from tree before calling detach.
• khtml/xml/dom_nodeimpl.h:

Location:

branches/Safari-2-0-branch/WebCore

Files:

• ChangeLog (modified) (1 diff)
• khtml/xml/dom_docimpl.cpp (modified) (1 diff)
• khtml/xml/dom_nodeimpl.cpp (modified) (6 diffs, 1 prop)
• khtml/xml/dom_nodeimpl.h (modified) (1 diff)

branches/Safari-2-0-branch/WebCore/ChangeLog

@@ +1 @@
+2006-04-25  Eric Seidel  <eseidel@apple.com>
+
+        Reviewed by ggaren.
+
+        Fixes for our 3rd and 5th most common crashtracers:
+        <rdar://problem/4233435> CrashTracer: 2698 crashes in Safari at com.apple.WebCore: khtml::RenderBlock::addChildToFlow + 156
+        <rdar://problem/4129744> [REGRESSION]CrashTracer: ..400 crashes at com.apple.WebCore: DOM::NodeImpl::createRendererIfNeeded + 44
+
+        Test case landed on trunk but not on branch.
+
+        * khtml/xml/dom_docimpl.cpp:
+        (DocumentImpl::setFocusNode): use m_inDetach bool copied from trunk
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::NodeImpl):
+        (NodeImpl::detach): set m_inDetach
+        (NodeImpl::createRendererIfNeeded):
+        (NodeBaseImpl::removeChildren): remove node from tree before calling detach.
+        * khtml/xml/dom_nodeimpl.h:
+
 2006-04-25  Timothy Hatcher  <timothy@apple.com>
 

branches/Safari-2-0-branch/WebCore/khtml/xml/dom_docimpl.cpp

@@ -2594 +2594 @@
 
     // Remove focus from the existing focus node (if any)
-    if (oldFocusNode) {
+    if (oldFocusNode && !oldFocusNode->m_inDetach) {
         // This goes hand in hand with the Qt focus setting below.
         if (!newFocusNode && view()) {

branches/Safari-2-0-branch/WebCore/khtml/xml/dom_nodeimpl.cpp

@@ -101 +101 @@
       m_inActiveChain( false ),
       m_styleElement( false ),
-      m_implicit( false )
+      m_implicit(false),
+      m_inDetach(false)
 {
     if (document)
@@ -1202 +1203 @@
 void NodeImpl::detach()
 {
+    m_inDetach = true;
 //    assert(m_attached);
 
@@ -1212 +1214 @@
         doc->incDOMTreeVersion();
     m_attached = false;
+    m_inDetach = false;
 }
 
@@ -1379 +1382 @@
     assert(!m_render);
     
-    NodeImpl *parent = parentNode();    
+    NodeImpl *parent = parentNode();
     assert(parent);
     
@@ -1941 +1944 @@
         
         n->ref();
-
-        if (n->attached())
-            n->detach();
+        
+        // Remove the node from the tree before calling detach or removedFromDocument (4427024, 4129744)
         n->setPreviousSibling(0);
         n->setNextSibling(0);
         n->setParent(0);
+        _first = next;
+        if (n == _last)
+            _last = 0;
+
+        if (n->attached())
+            n->detach();
         
         if (n->inDocument())
@@ -1952 +1960 @@
 
         n->deref();
-
-        _first = next;
-    }
-    _last = 0;
+    }
     allowEventDispatch();
     

branches/Safari-2-0-branch/WebCore/khtml/xml/dom_nodeimpl.h

@@ -502 +502 @@
     bool m_styleElement : 1; // contains stylesheet text
     bool m_implicit : 1; // implicitely generated by the parser
-
-    // 3 bits unused
+    
+    bool m_inDetach : 1;
+
+    // 2 bits unused
 };