WebCore:
authorcblu <cblu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Sep 2004 21:44:02 +0000 (21:44 +0000)
committercblu <cblu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Sep 2004 21:44:02 +0000 (21:44 +0000)
Made markup copying 5 times faster. Unfortunately, this still doesn't fix:
<rdar://problem/3794799> Tiger8A252: copying a bunch o' text is so slow it seems like a hang

        Reviewed by rjw.

        * khtml/dom/dom_string.h:
        * khtml/xml/dom2_rangeimpl.cpp:
        (DOM::RangeImpl::toHTML): serialize the range by iterating through the range
        * khtml/xml/dom_nodeimpl.cpp:
        (NodeImpl::startMarkup): new, factored out from recursive_toString
        (NodeImpl::endMarkup): ditto
        (NodeImpl::recursive_toString): call factored out methods
        * khtml/xml/dom_nodeimpl.h:

WebKit:

        Reviewed by rjw.

        * WebView.subproj/WebHTMLView.m:
        (-[WebHTMLView _selectedArchive]): added timing code for copying markup

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/dom/dom_string.h
WebCore/khtml/xml/dom2_rangeimpl.cpp
WebCore/khtml/xml/dom_nodeimpl.cpp
WebCore/khtml/xml/dom_nodeimpl.h
WebKit/ChangeLog
WebKit/WebView.subproj/WebHTMLView.m

index 55310e1904f7d2ff978277f81abc89db03c15c50..70c33a3083e81a2a5134c3d720da651d92a5288c 100644 (file)
@@ -1,3 +1,19 @@
+2004-09-24  Chris Blumenberg  <cblu@apple.com>
+
+       Made markup copying 5 times faster. Unfortunately, this still doesn't fix:
+       <rdar://problem/3794799> Tiger8A252: copying a bunch o' text is so slow it seems like a hang
+
+        Reviewed by rjw.
+
+        * khtml/dom/dom_string.h:
+        * khtml/xml/dom2_rangeimpl.cpp:
+        (DOM::RangeImpl::toHTML): serialize the range by iterating through the range
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::startMarkup): new, factored out from recursive_toString
+        (NodeImpl::endMarkup): ditto
+        (NodeImpl::recursive_toString): call factored out methods
+        * khtml/xml/dom_nodeimpl.h:
+
 === Safari-165 ===
 
 2004-09-24  Ken Kocienda  <kocienda@apple.com>
index 00da8a2d21ba6f6f568564efc1125a327060350d..999c63f38561e1b4f1c06c46c71893d9fe09b198 100644 (file)
@@ -120,9 +120,10 @@ public:
     operator NSString *() const;
 #endif
 
-private:
+#ifndef NDEBUG
     // For debugging only, leaks memory.
     const char *ascii() const;
+#endif
 
 protected:
     DOMStringImpl *impl;
index 944a0e0b4d9ea128d0c46d415d72bbf25cf30a3e..89e0307bcd097acf3df40d8c586ae367a377cab3 100644 (file)
@@ -824,14 +824,119 @@ DOMString RangeImpl::toHTML(QPtrList<NodeImpl> *nodes) const
 {
     int exceptionCode;
     NodeImpl *commonAncestor = commonAncestorContainer(exceptionCode);
-    NodeImpl *commonAncestorBlock = commonAncestor ? commonAncestor->enclosingBlockFlowElement() : 0;
-    if (commonAncestorBlock == 0)
-        return "";
+    NodeImpl *commonAncestorBlock = 0;
+    if (commonAncestor != 0) {
+        commonAncestorBlock = commonAncestor->enclosingBlockFlowElement();
+    }
+    if (commonAncestorBlock == 0) {
+        return "";    
+    }
+    
+#ifdef USE_NODEIMPL_ALGORITHM
     NodeImpl::Id id = commonAncestorBlock->id();
     bool onlyIncludeChildren = (id != ID_TABLE && id != ID_OL && id != ID_UL);
     return commonAncestorBlock->recursive_toHTML(onlyIncludeChildren, true, this, nodes);
+#else    
+    
+    QStringList markups;
+    NodeImpl *pastEnd = pastEndNode();
+    NodeImpl *lastClosed = 0;
+    QPtrList<NodeImpl> ancestorsToClose;
+    
+    // Iterate through the nodes of the range.
+    for (NodeImpl *n = startNode(); n != pastEnd; n = n->traverseNextNode()) {
+        
+        // Add the node to the markup.
+        markups.append(n->startMarkup(this));
+        if (nodes) {
+            nodes->append(n);
+        }
+        
+        if (n->firstChild() == 0) {
+            // Node has no children, add its close tag now.
+            markups.append(n->endMarkup());
+            lastClosed = n;
+            
+            // Check if the node is the last leaf of a tree.
+            NodeImpl *parent = n->parentNode();
+            if (parent != 0 && n == parent->lastChild()) {
+                if (!ancestorsToClose.isEmpty()) {
+                    // Close up the ancestors.
+                    QPtrListIterator<NodeImpl> ancestorsToCloseIt(ancestorsToClose);
+                    NodeImpl *next = n->traverseNextNode();
+                    NodeImpl *ancestor;
+                    int removeCount = 0;
+                    for (ancestorsToCloseIt.toLast(); (ancestor = ancestorsToCloseIt.current()) != 0; --ancestorsToCloseIt) {
+                        if (next != pastEnd) {
+                            // Not at the end of the range, close ancestors up to sibling of next node.
+                            if (ancestor != next->parentNode()) {
+                                markups.append(ancestor->endMarkup());
+                                lastClosed = ancestor;
+                                removeCount++;
+                            } else {
+                                break;
+                            }
+                        } else {
+                            // At the end of the range, close all pending ancestors.
+                            markups.append(ancestor->endMarkup());
+                            lastClosed = ancestor;
+                            removeCount++;
+                        }
+                    }
+                    for (int i = removeCount; i > 0; i--) {
+                        ancestorsToClose.removeLast();
+                    }
+                } else {
+                    // No ancestors to close, but need to add ancestors not in range since next node is in another tree. 
+                    NodeImpl *next = n->traverseNextNode();
+                    if (next != pastEnd && n != next->parentNode()) {
+                        while (parent != 0 && parent != next->parentNode()) {
+                            markups.prepend(parent->startMarkup(this));
+                            markups.append(parent->endMarkup());
+                            if (nodes) {
+                                nodes->append(parent);
+                            }                            
+                            lastClosed = parent;
+                            parent = parent->parentNode();
+                        }
+                    }
+                }
+            }
+        } else {
+            // Node is an ancestor, set it to close eventually.
+            ancestorsToClose.append(n);
+        }
+    }
+    
+    // Add ancestors up to the common ancestor block so inline ancestors such as FONT and B are part of the markup.
+    NodeImpl *ancestor = lastClosed->parentNode();
+    while (ancestor) {
+        bool breakAtEnd = false;
+        if (commonAncestorBlock == ancestor) {
+            NodeImpl::Id id = ancestor->id();
+            // Tables and lists must be part of the markup to avoid broken structures. 
+            if (id == ID_TABLE || id == ID_OL || id == ID_UL) {
+                breakAtEnd = true;
+            } else {
+                break;
+            }
+        }
+        markups.prepend(ancestor->startMarkup(this));
+        markups.append(ancestor->endMarkup());
+        if (nodes) {
+            nodes->append(ancestor);
+        }        
+        if (breakAtEnd) {
+            break;
+        }
+        ancestor = ancestor->parentNode();
+    }
+    
+    return markups.join("");
+#endif
 }
 
+
 DOMString RangeImpl::text() const
 {
     if (m_detached)
index c4df555d7882dfdb1809e682fd9d2aee654634c6..d3bc9faaba89c17de5bf1d7706f09402bbdc5e54 100644 (file)
@@ -280,8 +280,47 @@ static QString escapeHTML( const QString& in )
     return s;
 }
 
-QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyIncludeChildren, bool includeSiblings, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes)
+QString NodeImpl::startMarkup(const DOM::RangeImpl *range) const
+{
+    if (nodeType() == Node::TEXT_NODE) {
+        DOMString str = nodeValue().copy();
+        if (range) {
+            int exceptionCode;
+            if (this == range->endContainer(exceptionCode)) {
+                str.truncate(range->endOffset(exceptionCode));
+            }
+            if (this == range->startContainer(exceptionCode)) {
+                str.remove(0, range->startOffset(exceptionCode));
+            }
+        }
+        Id parentID = parentNode()->id();
+        bool dontEscape = (parentID == ID_SCRIPT || parentID == ID_TEXTAREA || parentID == ID_STYLE);
+        return dontEscape ? str.string() : escapeHTML(str.string());        
+    } else {
+        QString markup = QChar('<') + nodeName().string();
+        if (nodeType() == Node::ELEMENT_NODE) {
+            const ElementImpl *el = static_cast<const ElementImpl *>(this);
+            NamedAttrMapImpl *attrs = el->attributes();
+            unsigned long length = attrs->length();
+            for (unsigned int i=0; i<length; i++) {
+                AttributeImpl *attr = attrs->attributeItem(i);
+                markup += " " + getDocument()->attrName(attr->id()).string() + "=\"" + attr->value().string() + "\"";
+            }
+        }
+        markup += isHTMLElement() ? ">" : "/>";
+        return markup;
+    }
+}
+
+QString NodeImpl::endMarkup(void) const
+{
+    if ((!isHTMLElement() || endTag[id()] != FORBIDDEN) && nodeType() != Node::TEXT_NODE && nodeType() != Node::DOCUMENT_NODE) {
+        return "</" + nodeName().string() + ">";
+    }
+    return "";
+}
 
+QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyIncludeChildren, bool includeSiblings, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes)
 {
     QString me = "";
 
@@ -311,35 +350,7 @@ QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyInclude
             if (nodes) {
                 nodes->append(current);
             }
-            // Copy who I am into the me string
-            if (current->nodeType() == Node::TEXT_NODE) {
-                DOMString str = current->nodeValue().copy();
-                if (range) {
-                    int exceptionCode;
-                    if (current == range->endContainer(exceptionCode)) {
-                        str.truncate(range->endOffset(exceptionCode));
-                    }
-                    if (current == range->startContainer(exceptionCode)) {
-                        str.remove(0, range->startOffset(exceptionCode));
-                    }
-                }
-                Id parentID = current->parentNode()->id();
-                bool dontEscape = (parentID == ID_SCRIPT || parentID == ID_TEXTAREA || parentID == ID_STYLE);
-                me += dontEscape ? str.string() : escapeHTML(str.string());
-            } else if (current->nodeType() != Node::DOCUMENT_NODE) {
-                // If I am an element, not a text
-                me += QChar('<') + current->nodeName().string();
-                if (current->nodeType() == Node::ELEMENT_NODE) {
-                    const ElementImpl *el = static_cast<const ElementImpl *>(current);
-                    NamedAttrMapImpl *attrs = el->attributes();
-                    unsigned long length = attrs->length();
-                    for (unsigned int i=0; i<length; i++) {
-                        AttributeImpl *attr = attrs->attributeItem(i);
-                        me += " " + current->getDocument()->attrName(attr->id()).string() + "=\"" + attr->value().string() + "\"";
-                    }
-                }
-                me += current->isHTMLElement() ? ">" : "/>";
-            }
+            me += current->startMarkup(range);
         }
         
         if (!current->isHTMLElement() || endTag[current->id()] != FORBIDDEN) {
@@ -348,8 +359,8 @@ QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyInclude
                 me += recursive_toString(n, false, true, range, nodes);
             }
             // Print my ending tag
-            if (include && current->nodeType() != Node::TEXT_NODE && current->nodeType() != Node::DOCUMENT_NODE) {
-                me += "</" + current->nodeName().string() + ">";
+            if (include) {
+                me += current->endMarkup();
             }
         }
         
index d271d33549d8e16bb8e4796ae35db0334d1ab5f9..19b3f3ea26bc83d7024f7a8c89149709fd88074e 100644 (file)
@@ -259,6 +259,8 @@ public:
     virtual bool isMouseFocusable() const;
     
     virtual bool isInline() const;
+    QString startMarkup(const DOM::RangeImpl *range) const;
+    QString endMarkup(void) const;
     virtual QString toHTML() const;
     QString recursive_toHTML(bool onlyIncludeChildren=false, bool includeSiblings = false, const DOM::RangeImpl *range=NULL, QPtrList<NodeImpl> *nodes=NULL) const;
     static QString recursive_toString(const NodeImpl *startNode, bool onlyIncludeChildren, bool includeSiblings, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes);
index a6956744d0140525c68797d9d82c8f9284058925..c6d2094cbfcdec9812af739389a6577a9a38fade 100644 (file)
@@ -1,3 +1,10 @@
+2004-09-24  Chris Blumenberg  <cblu@apple.com>
+
+        Reviewed by rjw.
+
+        * WebView.subproj/WebHTMLView.m:
+        (-[WebHTMLView _selectedArchive]): added timing code for copying markup
+
 === Safari-165 ===
 
 2004-09-24  Chris Blumenberg  <cblu@apple.com>
index 213dcaac4c4ba9871bf3ecf26f4fb0bf6785cf8b..3216ee876118c0b291682daa71b0998f4aebf333 100644 (file)
@@ -833,7 +833,15 @@ static WebHTMLView *lastHitView = nil;
 - (WebArchive *)_selectedArchive
 {
     NSArray *nodes;
+#if !LOG_DISABLED        
+    double start = CFAbsoluteTimeGetCurrent();
+#endif
     NSString *markupString = [[self _bridge] markupStringFromRange:[self _selectedRange] nodes:&nodes];
+#if !LOG_DISABLED
+    double duration = CFAbsoluteTimeGetCurrent() - start;
+    LOG(Timing, "copying markup took %f seconds.", duration);
+#endif
+    
     return [[self _dataSource] _archiveWithMarkupString:markupString nodes:nodes];
 }