LayoutTests:
authorggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Aug 2006 22:51:04 +0000 (22:51 +0000)
committerggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Aug 2006 22:51:04 +0000 (22:51 +0000)
        Layout test for <rdar://problem/4661982> (crash in KHTMLParser::popBlock)

        * fast/parser/remove-node-stack-expected.txt: Added.
        * fast/parser/remove-node-stack.html: Added.

WebCore:

        Reviewed by Darin.

        - This patch reworks a previous fix for <rdar://problem/3524912> repro
        crash in KHTMLParser::parseToken, due to parser's current element being
        destroyed (www.gnnetcom.dk), along with subsequent adjustments to fix
        leaks.

        The previous solutions caused a ~2% performance regression on iBench HTML,
        due to RefPtr churn. The optimizations here gain back that ~2% plus ~1% more,
        for a total win of ~3% vs current TOT.

        We can merge this fix to the branch to fix <rdar://problem/4661982>
        (crash in KHTMLParser::popBlock).

        The solution here is:
        (1) Don't let the parser ref document nodes -- that causes leaks.
        (2) Handle ref/deref manually, to avoid RefPtr churn. Specifically, when
        moving a node between stacks or to/from 'current', rather than deref'ing
        and then ref'ing again, simply move the node, along with its refcount, to
        its new location, and overwrite its old location.

        * WebCore.xcodeproj/project.pbxproj:
        * html/HTMLParser.cpp:
        (WebCore::HTMLStackElem::HTMLStackElem):
        (WebCore::HTMLStackElem::derefNode):
        (WebCore::HTMLParser::HTMLParser):
        (WebCore::HTMLParser::setCurrent):
        (WebCore::HTMLParser::insertNode):
        (WebCore::HTMLParser::popNestedHeaderTag):
        (WebCore::HTMLParser::handleResidualStyleCloseTagAcrossBlocks):
        (WebCore::HTMLParser::reopenResidualStyleTags):
        (WebCore::HTMLParser::pushBlock):
        (WebCore::HTMLParser::popBlock):
        (WebCore::HTMLParser::popOneBlockCommon):
        (WebCore::HTMLParser::popOneBlock):
        (WebCore::HTMLParser::moveOneBlockToStack):
        * html/HTMLParser.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@15890 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/parser/remove-node-stack-expected.txt [new file with mode: 0644]
LayoutTests/fast/parser/remove-node-stack.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/html/HTMLParser.cpp
WebCore/html/HTMLParser.h

index 16d975b526ebf5ca22c36063332eb184021ce3c7..88d76823e121cf859f4ed984f5bbe842f1da2d9e 100644 (file)
@@ -1,3 +1,10 @@
+2006-08-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Layout test for <rdar://problem/4661982> (crash in KHTMLParser::popBlock)
+
+        * fast/parser/remove-node-stack-expected.txt: Added.
+        * fast/parser/remove-node-stack.html: Added.
+
 2006-08-15  Adele Peterson  <adele@apple.com>
 
         Reviewed by Maciej.
diff --git a/LayoutTests/fast/parser/remove-node-stack-expected.txt b/LayoutTests/fast/parser/remove-node-stack-expected.txt
new file mode 100644 (file)
index 0000000..be3cf33
--- /dev/null
@@ -0,0 +1 @@
+If you can read this text, and the browser didn't crash, then we successfully removed the nodes in the parser's node stack.
diff --git a/LayoutTests/fast/parser/remove-node-stack.html b/LayoutTests/fast/parser/remove-node-stack.html
new file mode 100644 (file)
index 0000000..9791608
--- /dev/null
@@ -0,0 +1,12 @@
+<html>
+<body>
+    <div><div>
+    <script>
+        if (window.layoutTestController) 
+            layoutTestController.dumpAsText();
+
+        document.body.innerHTML = "If you can read this text, and the browser didn't crash, then we successfully removed the nodes in the parser's node stack.";
+    </script>
+    </div></div>
+</body>
+</html>
index bce8b1450fc4af97c9e87fa85f53d43cdbdffe59..30563b5029237d45ad1949af8edd6b34a89e23da 100644 (file)
@@ -1,86 +1,42 @@
-2006-08-15  David Hyatt  <hyatt@apple.com>
-
-        Fix for bug 3303, support CSS2 system fonts.  This patch adds a new API
-        to RenderTheme for obtaining a system font. In addition to supporting
-        the CSS2 values, extensions have been added for the 3 control sizes on
-        Mac.  The UA stylesheet now uses -webkit-small-control and no longer hardcodes
-        Lucida Grande 11px.  This means that controls in Safari should now respect
-        haxies that modify the default fonts on controls.
-
-        Reviewed by beth
-
-        Added fast/css/css2-system-fonts.html
-
-        * WebCore.xcodeproj/project.pbxproj:
-        * css/CSSValueKeywords.in:
-        * css/cssparser.cpp:
-        (WebCore::CSSParser::parseValue):
-        * css/cssstyleselector.cpp:
-        (WebCore::CSSStyleSelector::applyProperty):
-        * css/html4.css:
-        * platform/FontDescription.h:
-        (WebCore::FontDescription::setBold):
-        * platform/win/TemporaryLinkStubs.cpp:
-        (RenderThemeWin::systemFont):
-        * rendering/RenderMenuList.cpp:
-        (WebCore::RenderMenuList::calcMinMaxWidth):
-        * rendering/RenderTheme.h:
-        (WebCore::RenderTheme::minimumMenuListSize):
-        * rendering/RenderThemeMac.h:
-        * rendering/RenderThemeMac.mm:
-        (WebCore::RenderThemeMac::systemFont):
-        (WebCore::RenderThemeMac::minimumMenuListSize):
-        * rendering/RenderThemeWin.h:
-
-2006-08-15  Adele Peterson  <adele@apple.com>
-
-        Reviewed by Maciej.
-
-        - Fix for <rdar://problem/4680207> REGRESSION: select-all should fire onSelect event for text fields and textareas (9518)
-        http://bugzilla.opendarwin.org/show_bug.cgi?id=9518
-
-        Test: fast/forms/onselect-selectall.html
-
-        * page/Frame.cpp: (WebCore::Frame::selectAll): Call notifyRendererOfSelectionChange with userTriggered = true so that onSelect will fire.
-
-2006-08-15  Anders Carlsson  <acarlsson@apple.com>
+2006-08-15  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Darin.
-
-        * platform/PlatformString.h:
-        * platform/String.cpp:
-        (WebCore::String::charactersWithNullTermination):
-        Add new function which calls StringImpl::charactersWithNullTermination.
         
-        * platform/StringImpl.cpp:
-        (WebCore::StringImpl::init):
-        (WebCore::StringImpl::append):
-        (WebCore::StringImpl::insert):
-        (WebCore::StringImpl::truncate):
-        (WebCore::StringImpl::remove):
-        Set m_hasTerminatingNullCharacter to false;
+        - This patch reworks a previous fix for <rdar://problem/3524912> repro 
+        crash in KHTMLParser::parseToken, due to parser's current element being 
+        destroyed (www.gnnetcom.dk), along with subsequent adjustments to fix
+        leaks.
         
-        (WebCore::StringImpl::charactersWithNullTermination):
-        If the string already has a terminating null character, simply return it. Otherwise,
-        realloc to make room for one and set m_hasTerminatingNullCharacter to true;
-
-        * platform/StringImpl.h:
-        (WebCore::StringImpl::StringImpl):
-        Add m_hasTerminatingNullCharacter.
-
-2006-08-15  Adele Peterson  <adele@apple.com>
-
-        Reviewed by Adam.
-
-        - Fix for http://bugzilla.opendarwin.org/show_bug.cgi?id=9926
-        REGRESSION: pop-up looks bad because <option> respects color but not background-color
-
-        * rendering/RenderPopupMenuMac.mm:
-        (WebCore::RenderPopupMenuMac::addGroupLabel): Removed code to set foreground color in NSMenu. 
-        Added a FIXME that we need to handle both foreground and background color. (9846)
-        (WebCore::RenderPopupMenuMac::addOption): ditto.
-        * rendering/RenderThemeMac.mm: (WebCore::RenderThemeMac::adjustMenuListStyle):
-        For the aqua look, set the foreground color to black.
+        The previous solutions caused a ~2% performance regression on iBench HTML,
+        due to RefPtr churn. The optimizations here gain back that ~2% plus ~1% more,
+        for a total win of ~3% vs current TOT.
+        
+        We can merge this fix to the branch to fix <rdar://problem/4661982> 
+        (crash in KHTMLParser::popBlock).
+        
+        The solution here is:
+        (1) Don't let the parser ref document nodes -- that causes leaks.
+        (2) Handle ref/deref manually, to avoid RefPtr churn. Specifically, when 
+        moving a node between stacks or to/from 'current', rather than deref'ing
+        and then ref'ing again, simply move the node, along with its refcount, to
+        its new location, and overwrite its old location.
+        * WebCore.xcodeproj/project.pbxproj:
+        * html/HTMLParser.cpp:
+        (WebCore::HTMLStackElem::HTMLStackElem):
+        (WebCore::HTMLStackElem::derefNode):
+        (WebCore::HTMLParser::HTMLParser):
+        (WebCore::HTMLParser::setCurrent):
+        (WebCore::HTMLParser::insertNode):
+        (WebCore::HTMLParser::popNestedHeaderTag):
+        (WebCore::HTMLParser::handleResidualStyleCloseTagAcrossBlocks):
+        (WebCore::HTMLParser::reopenResidualStyleTags):
+        (WebCore::HTMLParser::pushBlock):
+        (WebCore::HTMLParser::popBlock):
+        (WebCore::HTMLParser::popOneBlockCommon):
+        (WebCore::HTMLParser::popOneBlock):
+        (WebCore::HTMLParser::moveOneBlockToStack):
+        * html/HTMLParser.h:
 
 2006-08-15  Mark Rowe  <opendarwin.org@bdash.net.nz>
 
         (-[WebCoreAXObject rendererForView:]):
         Clean up some old naming.
 
->>>>>>> .r15865
 2006-08-11  Anders Carlsson  <acarlsson@apple.com>
 
         Reviewed by Darin.
 
         * dom/DOMImplementation.cpp:
 
->>>>>>> .r15841
 2006-08-12  Oliver  <ojh16@student.canterbury.ac.nz>
 
         Rubber stamped by tim
index fe38eb2abf802d3087838e59fa15c8fd831a93c4..d239301ac54fb9b9b0a3ce86ffb7269dc6f71324 100644 (file)
@@ -60,127 +60,30 @@ const UChar nonBreakingSpace = 0xa0;
  * @internal
  */
 
-    class RefNonDocNodePtr
-    {
-    public:
-        RefNonDocNodePtr() : m_ptr(0) {}
-        RefNonDocNodePtr(Node* ptr) : m_ptr(ptr), m_isDoc(ptr->isDocumentNode()) { if (!m_isDoc && ptr) ptr->ref(); }
-        RefNonDocNodePtr(const RefNonDocNodePtr& o) : m_ptr(o.m_ptr), m_isDoc(o.m_isDoc) { if (!m_isDoc && m_ptr) m_ptr->ref(); }
-
-        ~RefNonDocNodePtr() { if (!m_isDoc && m_ptr) m_ptr->deref(); }
-        
-        Node *get() const { return m_ptr; }
-        
-        Node& operator*() const { return *m_ptr; }
-        Node *operator->() const { return m_ptr; }
-        
-        bool operator!() const { return !m_ptr; }
-    
-        // This conversion operator allows implicit conversion to bool but not to other integer types.
-        typedef Node* (RefNonDocNodePtr::*UnspecifiedBoolType)() const;
-        operator UnspecifiedBoolType() const { return m_ptr ? &RefNonDocNodePtr::get : 0; }
-        
-        RefNonDocNodePtr& operator=(const RefNonDocNodePtr&);
-        RefNonDocNodePtr& operator=(Node*);
-        RefNonDocNodePtr& operator=(RefPtr<Node>&);
-
-    private:
-        Node* m_ptr;
-        bool m_isDoc;
-    };
-    
-    inline RefNonDocNodePtr& RefNonDocNodePtr::operator=(const RefNonDocNodePtr& o)
-    {
-        Node* optr = o.get();
-        if (!o.m_isDoc && optr)
-            optr->ref();
-        Node* ptr = m_ptr;
-        m_ptr = optr;
-        if (!m_isDoc && ptr)
-            ptr->deref();
-        m_isDoc = o.m_isDoc;
-        return *this;
-    }
-    
-    inline RefNonDocNodePtr& RefNonDocNodePtr::operator=(Node* optr)
-    {
-        bool o_isDoc = optr->isDocumentNode();
-        if (!o_isDoc && optr)
-            optr->ref();
-        Node* ptr = m_ptr;
-        m_ptr = optr;
-        if (!m_isDoc && ptr)
-            ptr->deref();
-        m_isDoc = o_isDoc;
-        return *this;
-    }
-
-    inline RefNonDocNodePtr& RefNonDocNodePtr::operator=(RefPtr<Node>& o)
-    {
-        Node* optr = o.get();
-        bool o_isDoc = optr->isDocumentNode();
-        if (!o_isDoc && optr)
-            optr->ref();
-        Node* ptr = m_ptr;
-        m_ptr = optr;
-        if (!m_isDoc && ptr)
-            ptr->deref();
-        m_isDoc = o_isDoc;
-        return *this;
-    }
-
-    inline bool operator==(const RefNonDocNodePtr& a, const RefNonDocNodePtr& b)
-    { 
-        return a.get() == b.get(); 
-    }
-
-    inline bool operator==(const RefNonDocNodePtr& a, Node* b)
-    { 
-        return a.get() == b; 
-    }
-    
-    inline bool operator==(Node* a, const RefNonDocNodePtr& b) 
+class HTMLStackElem
+{
+public:
+    HTMLStackElem(const AtomicString& t, int lvl, Node* n, bool r, HTMLStackElem* nx)
+        : tagName(t)
+        , level(lvl)
+        , strayTableContent(false)
+        , node(n)
+        , didRefNode(r)
+        , next(nx)
     {
-        return a == b.get(); 
-    }
-    
-    inline bool operator!=(const RefNonDocNodePtr& a, const RefNonDocNodePtr& b)
-    { 
-        return a.get() != b.get(); 
     }
 
-    inline bool operator!=(const RefNonDocNodePtr& a, Node* b)
+    void derefNode()
     {
-        return a.get() != b; 
+        if (didRefNode)
+            node->deref();
     }
 
-    inline bool operator!=(Node* a, const RefNonDocNodePtr& b)
-    { 
-        return a != b.get(); 
-    }
-    
-
-
-class HTMLStackElem
-{
-public:
-    HTMLStackElem(const AtomicString& _tagName,
-                  int _level,
-                  Node *_node,
-                  HTMLStackElem * _next
-        )
-        :
-        tagName(_tagName),
-        level(_level),
-        strayTableContent(false),
-        node(_node),
-        next(_next)
-        { }
-
     AtomicString tagName;
     int level;
     bool strayTableContent;
-    RefNonDocNodePtr node;
+    Node* node;
+    bool didRefNode;
     HTMLStackElem* next;
 };
 
@@ -210,7 +113,7 @@ public:
 HTMLParser::HTMLParser(Document* doc) 
     : document(doc)
     , current(0)
-    , currentIsReferenced(false)
+    , didRefCurrent(false)
     , blockStack(0)
     , m_fragment(false)
 {
@@ -220,7 +123,7 @@ HTMLParser::HTMLParser(Document* doc)
 HTMLParser::HTMLParser(DocumentFragment* frag)
     : document(frag->document())
     , current(0)
-    , currentIsReferenced(false)
+    , didRefCurrent(false)
     , blockStack(0)
     , m_fragment(true)
 {
@@ -257,15 +160,15 @@ void HTMLParser::reset()
     discard_until = nullAtom;
 }
 
-void HTMLParser::setCurrent(Node *newCurrent) 
+void HTMLParser::setCurrent(NodenewCurrent) 
 {
-    bool newCurrentIsReferenced = newCurrent && newCurrent != doc();
-    if (newCurrentIsReferenced
+    bool didRefNewCurrent = newCurrent && newCurrent != doc();
+    if (didRefNewCurrent
         newCurrent->ref(); 
-    if (currentIsReferenced
+    if (didRefCurrent
         current->deref(); 
     current = newCurrent;
-    currentIsReferenced = newCurrentIsReferenced;
+    didRefCurrent = didRefNewCurrent;
 }
 
 PassRefPtr<Node> HTMLParser::parseToken(Token *t)
@@ -361,27 +264,35 @@ bool HTMLParser::insertNode(Node *n, bool flat)
         
     // let's be stupid and just try to insert it.
     // this should work if the document is well-formed
-    Node *newNode = current->addChild(n);
-    if (newNode) {
-        // don't push elements without end tags (e.g., <img>) on the stack
-        bool parentAttached = current->attached();
-        if (tagPriority > 0 && !flat) {
-            pushBlock(localName, tagPriority);
-            if (newNode == current)
-                popBlock(localName);
-            else
-                setCurrent(newNode);
-            if (parentAttached && !n->attached() && !m_fragment)
-                n->attach();
-        } else {
-            if (parentAttached && !n->attached() && !m_fragment)
-                n->attach();
-            n->closeRenderer();
+    Node* newNode = current->addChild(n);
+    if (!newNode)
+        return handleError(n, flat, localName, tagPriority); // Try to handle the error.
+
+    // don't push elements without end tags (e.g., <img>) on the stack
+    bool parentAttached = current->attached();
+    if (tagPriority > 0 && !flat) {
+        pushBlock(localName, tagPriority);
+        if (newNode == current)
+            popBlock(localName);
+        else {
+            // The pushBlock function transfers ownership of current to the block stack
+            // so we're guaranteed that didRefCurrent is false. The code below is an
+            // optimized version of setCurrent that takes advantage of that fact and also
+            // assumes that newNode is neither 0 nor a pointer to the document.
+            ASSERT(!didRefCurrent);
+            newNode->ref(); 
+            current = newNode;
+            didRefCurrent = true;
         }
+        if (parentAttached && !n->attached() && !m_fragment)
+            n->attach();
+    } else {
+        if (parentAttached && !n->attached() && !m_fragment)
+            n->attach();
+        n->closeRenderer();
+    }
 
-        return true;
-    } else
-        return handleError(n, flat, localName, tagPriority); // Try to handle the error.
+    return true;
 }
 
 bool HTMLParser::handleError(Node* n, bool flat, const AtomicString& localName, int tagPriority)
@@ -965,7 +876,7 @@ void HTMLParser::popNestedHeaderTag()
         }
         if (currNode && !isInline(currNode))
             return;
-        currNode = curr->node.get();
+        currNode = curr->node;
     }
 }
 
@@ -1076,9 +987,9 @@ void HTMLParser::handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem* elem)
 
     if (!curr || !maxElem || !isAffectedByResidualStyle(maxElem->tagName)) return;
 
-    Node* residualElem = prev->node.get();
-    Node* blockElem = prevMaxElem ? prevMaxElem->node.get() : current;
-    Node* parentElem = elem->node.get();
+    Node* residualElem = prev->node;
+    Node* blockElem = prevMaxElem ? prevMaxElem->node : current;
+    Node* parentElem = elem->node;
 
     // Check to see if the reparenting that is going to occur is allowed according to the DOM.
     // FIXME: We should either always allow it or perform an additional fixup instead of
@@ -1098,7 +1009,9 @@ void HTMLParser::handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem* elem)
             HTMLStackElem* nextElem = currElem->next;
             if (!isResidualStyleTag(currElem->tagName)) {
                 prevElem->next = nextElem;
+                prevElem->derefNode();
                 prevElem->node = currElem->node;
+                prevElem->didRefNode = currElem->didRefNode;
                 delete currElem;
             }
             else
@@ -1119,18 +1032,22 @@ void HTMLParser::handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem* elem)
         while (currElem->node != residualElem) {
             if (isResidualStyleTag(currElem->node->localName())) {
                 // Create a clone of this element.
-                RefPtr<Node> currNode = currElem->node->cloneNode(false);
+                // We call release to get a raw pointer since we plan to hand over ownership to currElem.
+                Node* currNode = currElem->node->cloneNode(false).release();
 
                 // Change the stack element's node to point to the clone.
+                // The stack element adopts the reference we obtained above by calling release().
+                currElem->derefNode();
                 currElem->node = currNode;
+                currElem->didRefNode = true;
                 
                 // Attach the previous node as a child of this new node.
                 if (prevNode)
                     currNode->appendChild(prevNode, ec);
                 else // The new parent for the block element is going to be the innermost clone.
-                    parentElem = currNode.get();
+                    parentElem = currNode;
                                 
-                prevNode = currNode.get();
+                prevNode = currNode;
             }
             
             currElem = currElem->next;
@@ -1190,7 +1107,9 @@ void HTMLParser::handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem* elem)
         currElem = currElem->next;
     }
     prevElem->next = elem->next;
+    prevElem->derefNode();
     prevElem->node = elem->node;
+    prevElem->didRefNode = elem->didRefNode;
     delete elem;
     
     // Step 7: Reopen intermediate inlines, e.g., <b><p><i>Foo</b>Goo</p>.
@@ -1200,8 +1119,7 @@ void HTMLParser::handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem* elem)
     while (curr && curr != maxElem) {
         // We will actually schedule this tag for reopening
         // after we complete the close of this entire block.
-        Node* currNode = current;
-        if (isResidualStyleTag(curr->tagName)) {
+        if (isResidualStyleTag(curr->tagName))
             // We've overloaded the use of stack elements and are just reusing the
             // struct with a slightly different meaning to the variables.  Instead of chaining
             // from innermost to outermost, we build up a list of all the tags we need to reopen
@@ -1210,11 +1128,7 @@ void HTMLParser::handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem* elem)
             // We also set curr->node to be the actual element that corresponds to the ID stored in
             // curr->id rather than the node that you should pop to when the element gets pulled off
             // the stack.
-            popOneBlock(false);
-            curr->node = currNode;
-            curr->next = residualStyleStack;
-            residualStyleStack = curr;
-        }
+            moveOneBlockToStack(residualStyleStack);
         else
             popOneBlock();
 
@@ -1261,15 +1175,16 @@ void HTMLParser::reopenResidualStyleTags(HTMLStackElem* elem, Node* malformedTab
         
         // Advance to the next tag that needs to be reopened.
         HTMLStackElem* next = elem->next;
+        elem->derefNode();
         delete elem;
         elem = next;
     }
 }
 
-void HTMLParser::pushBlock(const AtomicString& tagName, int _level)
+void HTMLParser::pushBlock(const AtomicString& tagName, int level)
 {
-    HTMLStackElem *Elem = new HTMLStackElem(tagName, _level, current, blockStack);
-    blockStack = Elem;
+    blockStack = new HTMLStackElem(tagName, level, current, didRefCurrent, blockStack);
+    didRefCurrent = false;
 }
 
 void HTMLParser::popBlock(const AtomicString& _tagName)
@@ -1326,8 +1241,7 @@ void HTMLParser::popBlock(const AtomicString& _tagName)
 
             // Schedule this tag for reopening
             // after we complete the close of this entire block.
-            Node* currNode = current;
-            if (isAffectedByStyle && isResidualStyleTag(Elem->tagName)) {
+            if (isAffectedByStyle && isResidualStyleTag(Elem->tagName))
                 // We've overloaded the use of stack elements and are just reusing the
                 // struct with a slightly different meaning to the variables.  Instead of chaining
                 // from innermost to outermost, we build up a list of all the tags we need to reopen
@@ -1336,11 +1250,7 @@ void HTMLParser::popBlock(const AtomicString& _tagName)
                 // We also set Elem->node to be the actual element that corresponds to the ID stored in
                 // Elem->id rather than the node that you should pop to when the element gets pulled off
                 // the stack.
-                popOneBlock(false);
-                Elem->next = residualStyleStack;
-                Elem->node = currNode;
-                residualStyleStack = Elem;
-            }
+                moveOneBlockToStack(residualStyleStack);
             else
                 popOneBlock();
             Elem = blockStack;
@@ -1350,7 +1260,7 @@ void HTMLParser::popBlock(const AtomicString& _tagName)
     reopenResidualStyleTags(residualStyleStack, malformedTableParent);
 }
 
-void HTMLParser::popOneBlock(bool delBlock)
+inline HTMLStackElem* HTMLParser::popOneBlockCommon()
 {
     HTMLStackElem* elem = blockStack;
 
@@ -1360,13 +1270,47 @@ void HTMLParser::popOneBlock(bool delBlock)
         current->closeRenderer();
 
     blockStack = elem->next;
-    setCurrent(elem->node.get());
+    current = elem->node;
+    didRefCurrent = elem->didRefNode;
 
     if (elem->strayTableContent)
         inStrayTableContent--;
-    
-    if (delBlock)
-        delete elem;
+
+    return elem;
+}
+
+void HTMLParser::popOneBlock()
+{
+    // Store the current node before popOneBlockCommon overwrites it.
+    Node* lastCurrent = current;
+    bool didRefLastCurrent = didRefCurrent;
+
+    delete popOneBlockCommon();
+
+    if (didRefLastCurrent)
+        lastCurrent->deref();
+}
+
+void HTMLParser::moveOneBlockToStack(HTMLStackElem*& head)
+{
+    // We'll be using the stack element we're popping, but for the current node.
+    // See the two callers for details.
+
+    // Store the current node before popOneBlockCommon overwrites it.
+    Node* lastCurrent = current;
+    bool didRefLastCurrent = didRefCurrent;
+
+    // Pop the block, but don't deref the current node as popOneBlock does because
+    // we'll be using the pointer in the new stack element.
+    HTMLStackElem* elem = popOneBlockCommon();
+
+    // Transfer the current node into the stack element.
+    // No need to deref the old elem->node because popOneBlockCommon transferred
+    // it into the current/didRefCurrent fields.
+    elem->node = lastCurrent;
+    elem->didRefNode = didRefLastCurrent;
+    elem->next = head;
+    head = elem;
 }
 
 void HTMLParser::popInlineBlocks()
index 2c83ed1c6e7f0ecfad9b046d26be8def249b348d..a4dfb508463e88fccca2a4730d22f84fd779d3d2 100644 (file)
@@ -76,6 +76,7 @@ public:
 
 private:
     void setCurrent(Node* newCurrent);
+    void derefCurrent();
     void setSkipMode(const QualifiedName& qName) { discard_until = qName.localName(); }
 
     Document* document;
@@ -108,17 +109,19 @@ private:
     bool insertNode(Node *n, bool flat = false);
     bool handleError(Node* n, bool flat, const AtomicString& localName, int tagPriority);
     
-    // The currently active element (the one new elements will be added to).  Can be a DocumentFragment, a Document or an Element.
+    // The currently active element (the one new elements will be added to). Can be a document fragment, a document or an element.
     Node* current;
-
-    bool currentIsReferenced;
+    // We can't ref a document, but we don't want to constantly check if a node is a document just to decide whether to deref.
+    bool didRefCurrent;
 
     HTMLStackElem *blockStack;
 
     void pushBlock(const AtomicString& tagName, int _level);
     void popBlock(const AtomicString& tagName);
     void popBlock(const QualifiedName& qName) { return popBlock(qName.localName()); } // Convenience function for readability.
-    void popOneBlock(bool delBlock = true);
+    void popOneBlock();
+    void moveOneBlockToStack(HTMLStackElem*& head);
+    inline HTMLStackElem* popOneBlockCommon();
     void popInlineBlocks();
 
     void freeBlock();
@@ -127,8 +130,8 @@ private:
 
     bool isResidualStyleTag(const AtomicString& tagName);
     bool isAffectedByResidualStyle(const AtomicString& tagName);
-    void handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem* elem);
-    void reopenResidualStyleTags(HTMLStackElem* elem, Node* malformedTableParent);
+    void handleResidualStyleCloseTagAcrossBlocks(HTMLStackElem*);
+    void reopenResidualStyleTags(HTMLStackElem*, Node* malformedTableParent);
 
     bool allowNestedRedundantTag(const AtomicString& tagName);