From 91437969ddb28bc81b3652986a67aae7e2465db2 Mon Sep 17 00:00:00 2001 From: Kulanthaivel Palanichamy Date: Wed, 15 Aug 2012 17:14:48 -0700 Subject: [WebKit] Browsermark DomAdvSearch and DomSearch optimizations This patch is a combination of the following optimizations. Optimize appendCharactersReplacingEntities [Performance] Optimize innerHTML and outerHTML https://bugs.webkit.org/show_bug.cgi?id=81214 Optimize Dromaeo/dom-query.html by caching NodeRareData on Document https://bugs.webkit.org/show_bug.cgi?id=90059 Optimize Dromaeo/dom-attr.html by speeding up Element::getAttribute() https://bugs.webkit.org/show_bug.cgi?id=90174 Change argument types of Element::getAttribute*() from String to AtomicString https://bugs.webkit.org/show_bug.cgi?id=90246 Conflicts: Source/WebCore/ChangeLog Source/WebCore/dom/Document.cpp Source/WebCore/dom/Document.h Conflicts: Source/WebCore/ChangeLog Change-Id: I9ca3d2e883155e2ad08c3cb0c5912b935ebe9406 --- Source/JavaScriptCore/wtf/text/StringBuilder.h | 29 ++++ Source/WebCore/ChangeLog | 148 +++++++++++++++++ Source/WebCore/dom/Document.cpp | 9 ++ Source/WebCore/dom/Document.h | 7 +- Source/WebCore/dom/Element.cpp | 22 +-- Source/WebCore/dom/Element.h | 16 +- Source/WebCore/dom/NamedNodeMap.cpp | 10 +- Source/WebCore/dom/NamedNodeMap.h | 14 +- Source/WebCore/dom/Node.cpp | 16 +- Source/WebCore/editing/MarkupAccumulator.cpp | 212 ++++++++++++++----------- Source/WebCore/editing/MarkupAccumulator.h | 39 ++--- Source/WebCore/editing/markup.cpp | 48 +++--- 12 files changed, 397 insertions(+), 173 deletions(-) diff --git a/Source/JavaScriptCore/wtf/text/StringBuilder.h b/Source/JavaScriptCore/wtf/text/StringBuilder.h index f10af64..e891932 100644 --- a/Source/JavaScriptCore/wtf/text/StringBuilder.h +++ b/Source/JavaScriptCore/wtf/text/StringBuilder.h @@ -43,6 +43,9 @@ public: void append(const String& string) { + if (!string.length()) + return; + // If we're appending to an empty string, and there is not buffer // (in case reserveCapacity has been called) then just retain the // string. @@ -54,6 +57,22 @@ public: append(string.characters(), string.length()); } + void append(const StringBuilder& other) + { + if (!other.m_length) + return; + + // If we're appending to an empty string, and there is not a buffer (reserveCapacity has not been called) + // then just retain the string. + if (!m_length && !m_buffer && !other.m_string.isNull()) { + m_string = other.m_string; + m_length = other.m_length; + return; + } + + append(other.characters(), other.m_length); + } + void append(const char* characters) { if (characters) @@ -114,6 +133,16 @@ public: return m_buffer->characters()[i]; } + const UChar* characters() const + { + if (!m_length) + return 0; + if (!m_string.isNull()) + return m_string.characters(); + ASSERT(m_buffer); + return m_buffer->characters(); + } + void clear() { m_length = 0; diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 42d6cd1..c877435 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,151 @@ +2012-03-16 Kentaro Hara + + [Performance] Optimize innerHTML and outerHTML + https://bugs.webkit.org/show_bug.cgi?id=81214 + + Reviewed by Adam Barth. + + This patch makes innerHTML and outerHTML 2.4 times faster. + + Performance test: https://bugs.webkit.org/attachment.cgi?id=132034 + The performance test measures body.innerHTML for 3000 lines of HTML, + which is copied from the HTML spec. + + - Chromium/Mac without the patch + div.innerHTML: 1658.6 ms + div.outerHTML: 4859.6 ms + body.innerHTML: 640.2 ms + body.outerHTML: 641.8 ms + + - Chromium/Mac with the patch + div.innerHTML: 751.0 ms + div.outerHTML: 2096.0 ms + body.innerHTML: 271.2 ms + body.outerHTML: 271.2 ms + + - Chromium/Linux without the patch + div.innerHTML: 950.4 ms + div.outerHTML: 2257.8 ms + body.innerHTML: 452.8 ms + body.outerHTML: 457.6 ms + + - Chromium/Linux with the patch + div.innerHTML: 582.4 ms + div.outerHTML: 1283.0 ms + body.innerHTML: 233.0 ms + body.outerHTML: 233.4 ms + + - AppleWebKit/Mac without the patch + div.innerHTML: 900.6 ms + div.outerHTML: 2245.2 ms + body.innerHTML: 462.6 ms + body.outerHTML: 468.0 ms + + - AppleWebKit/Mac with the patch + div.innerHTML: 529.8 ms + div.outerHTML: 1090.2 ms + body.innerHTML: 239.2 ms + body.outerHTML: 239.2 ms + + This patch applies the following two optimizations: + + (a) Remove redundant copies between Vector and StringBuilders + in MarkupAccumulator::serializeNodes(), MarkupAccumulator::appendStartTag(), + and MarkupAccumulator::appendEndTag(). + + (Previous behavior) + - Create a StringBuilder for each tag. + - Append a created string in each StringBuilder to Vector, + parsing the DOM tree. + - After the parsing, allocate a StringBuilder whose size is the sum + of all Strings in Vector. + - Append all Strings in Vector to the StringBuilder. + (New behavior) + - Allocate a StringBuilder with a default buffer size. + - Append created strings to the StringBuilder, incrementally parsing + the DOM tree. + + (b) Optimize stringBuilder.append(). + (b-1) Replace stringBuilder.append("A") with stringBuilder.append('A'). + stringBuilder.append("A") requires to cast the characters to LChar*, + and then call strlen("A"). stringBuilder.append('A') is faster. + (b-2) Replace stringBuilder.append("AB") with stringBuilder.append('A') + and stringBuilder.append('B'). In my experiment, appending characters + one by one is faster than appending the characters at a breath if the + number of characters is less than 3. + (b-3) Hard-code a string length; i.e. replace stringBuilder.append("ABCDE") + with stringBuilder.append("ABCDE", 5). While the former requires to call + strlen("ABCDE"), the latter does not. + + (a) improves performance by 170% ~ 200%. (b) improves performance by 30 ~ 40%. + + Tests: fast/dom/Range/range-extract-contents.html + fast/dom/serialize-nodes.xhtml + fast/dom/XMLSerializer.html + and all other tests that use innerHTML or outerHTML. + No change in the test results. + + * editing/MarkupAccumulator.cpp: + (WebCore::MarkupAccumulator::serializeNodes): + (WebCore::MarkupAccumulator::appendString): + (WebCore::MarkupAccumulator::appendStartTag): + (WebCore::MarkupAccumulator::appendEndTag): + (WebCore::MarkupAccumulator::concatenateMarkup): + (WebCore::MarkupAccumulator::appendQuotedURLAttributeValue): + (WebCore::MarkupAccumulator::appendComment): + (WebCore::MarkupAccumulator::appendDocumentType): + (WebCore::MarkupAccumulator::appendProcessingInstruction): + (WebCore::MarkupAccumulator::appendOpenTag): + (WebCore::MarkupAccumulator::appendAttribute): + (WebCore::MarkupAccumulator::appendCDATASection): + * editing/MarkupAccumulator.h: + (MarkupAccumulator): + +2012-06-27 Kentaro Hara + + Performance: Optimize Dromaeo/dom-query.html by caching NodeRareData on Document + https://bugs.webkit.org/show_bug.cgi?id=90059 + + Reviewed by Ryosuke Niwa. + + This patch improves performance of document.getElementsBy*(). + e.g. the patch makes Dromaeo/dom-query.html 5.4% faster. + + Dromaeo/dom-query.html without the patch (Chromium/Linux): + 784714 runs/s, 765947 runs/s, 803109 runs/s, 804450 runs/s + + Dromaeo/dom-query.html with the patch (Chromium/Linux): + 839245 runs/s, 829867 runs/s, 811032 runs/s, 847486 runs/s + + Based on the assumption that document.getElementsByClassName(), + document.getElementsByTagName() and document.getElementsByName() + would be used frequently in the real world, this patch implements + a fast path for Document methods that require to access NodeRareData. + Specifically, this patch caches a pointer to NodeRareData on Document, + by which Document can access NodeRareData without looking up a HashMap. + + The only performance concern is the overhead of the isDocumentNode() check + that this patch added to Node::ensureRareData. However, I could not + observe any performance regression caused by the overhead. + + No tests. No change in behavior. + + * dom/Document.cpp: + (WebCore::Document::Document): + (WebCore::Document::setCachedRareData): I didn't inline this method, + since the inlining slightly regressed performance for some reason. + (WebCore): + * dom/Document.h: + (WebCore): + (WebCore::Document::cachedRareData): + (Document): + (~Document): Moved 'm_document = 0' to the tail of the destructor, + since isDocumentNode() has to return true in clearRareData() that is called + in ~Document(). + * dom/Node.cpp: + (WebCore::Node::ensureRareData): + (~Node): Moved the assertion into clearRareData(). + 2012-04-09 James Robinson Remove partially implemented per-Element visibility checks from requestAnimationFrame logic diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp index 60ecdb9..f319cac 100644 --- a/Source/WebCore/dom/Document.cpp +++ b/Source/WebCore/dom/Document.cpp @@ -110,6 +110,7 @@ #include "NestingLevelIncrementer.h" #include "NodeFilter.h" #include "NodeIterator.h" +#include "NodeRareData.h" #include "NodeWithIndex.h" #include "OverflowEvent.h" #include "Page.h" @@ -424,6 +425,7 @@ Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML) , m_sawElementsInKnownNamespaces(false) , m_usingGeolocation(false) , m_eventQueue(EventQueue::create(this)) + , m_documentRareData(0) #if ENABLE(WML) , m_containsWMLContent(false) #endif @@ -573,6 +575,13 @@ Document::~Document() if (m_implementation) m_implementation->ownerDocumentDestroyed(); + + if (hasRareData()) { + ASSERT(m_documentRareData); + delete m_documentRareData; + m_documentRareData = 0; + clearFlag(HasRareDataFlag); + } } void Document::removedLastRef() diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h index 44f3f93..f94ce7a 100644 --- a/Source/WebCore/dom/Document.h +++ b/Source/WebCore/dom/Document.h @@ -104,6 +104,7 @@ class MediaQueryMatcher; class MouseEventWithHitTestResults; class NodeFilter; class NodeIterator; +class NodeRareData; class Page; class PlatformMouseEvent; class ProcessingInstruction; @@ -1109,12 +1110,14 @@ public: ContentSecurityPolicy* contentSecurityPolicy() { return m_contentSecurityPolicy.get(); } + NodeRareData* documentRareData() const { return m_documentRareData; }; + void setDocumentRareData(NodeRareData* rareData) { m_documentRareData = rareData; } + protected: Document(Frame*, const KURL&, bool isXHTML, bool isHTML); void clearXMLVersion() { m_xmlVersion = String(); } - private: friend class IgnoreDestructiveWriteCountIncrementer; @@ -1376,6 +1379,8 @@ private: RefPtr m_eventQueue; + NodeRareData* m_documentRareData; + #if ENABLE(WML) bool m_containsWMLContent; #endif diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp index 5fb6cdc..4b9de49 100644 --- a/Source/WebCore/dom/Element.cpp +++ b/Source/WebCore/dom/Element.cpp @@ -622,7 +622,7 @@ static inline bool shouldIgnoreAttributeCase(const Element* e) return e && e->document()->isHTMLDocument() && e->isHTMLElement(); } -const AtomicString& Element::getAttribute(const String& name) const +const AtomicString& Element::getAttribute(const AtomicString& name) const { bool ignoreCase = shouldIgnoreAttributeCase(this); @@ -645,7 +645,7 @@ const AtomicString& Element::getAttribute(const String& name) const return nullAtom; } -const AtomicString& Element::getAttributeNS(const String& namespaceURI, const String& localName) const +const AtomicString& Element::getAttributeNS(const AtomicString& namespaceURI, const AtomicString& localName) const { return getAttribute(QualifiedName(nullAtom, localName, namespaceURI)); } @@ -1471,11 +1471,11 @@ void Element::setAttributeNS(const AtomicString& namespaceURI, const AtomicStrin setAttribute(qName, value, ec); } -void Element::removeAttribute(const String& name, ExceptionCode& ec) +void Element::removeAttribute(const AtomicString& name, ExceptionCode& ec) { InspectorInstrumentation::willModifyDOMAttr(document(), this); - String localName = shouldIgnoreAttributeCase(this) ? name.lower() : name; + AtomicString localName = shouldIgnoreAttributeCase(this) ? name.lower() : name; if (m_attributeMap) { m_attributeMap->removeNamedItem(localName, ec); @@ -1486,21 +1486,21 @@ void Element::removeAttribute(const String& name, ExceptionCode& ec) InspectorInstrumentation::didModifyDOMAttr(document(), this); } -void Element::removeAttributeNS(const String& namespaceURI, const String& localName, ExceptionCode& ec) +void Element::removeAttributeNS(const AtomicString& namespaceURI, const AtomicString& localName, ExceptionCode& ec) { removeAttribute(QualifiedName(nullAtom, localName, namespaceURI), ec); } -PassRefPtr Element::getAttributeNode(const String& name) +PassRefPtr Element::getAttributeNode(const AtomicString& name) { NamedNodeMap* attrs = attributes(true); if (!attrs) return 0; - String localName = shouldIgnoreAttributeCase(this) ? name.lower() : name; + AtomicString localName = shouldIgnoreAttributeCase(this) ? name.lower() : name; return static_pointer_cast(attrs->getNamedItem(localName)); } -PassRefPtr Element::getAttributeNodeNS(const String& namespaceURI, const String& localName) +PassRefPtr Element::getAttributeNodeNS(const AtomicString& namespaceURI, const AtomicString& localName) { NamedNodeMap* attrs = attributes(true); if (!attrs) @@ -1508,7 +1508,7 @@ PassRefPtr Element::getAttributeNodeNS(const String& namespaceURI, const S return static_pointer_cast(attrs->getNamedItem(QualifiedName(nullAtom, localName, namespaceURI))); } -bool Element::hasAttribute(const String& name) const +bool Element::hasAttribute(const AtomicString& name) const { NamedNodeMap* attrs = attributes(true); if (!attrs) @@ -1516,11 +1516,11 @@ bool Element::hasAttribute(const String& name) const // This call to String::lower() seems to be required but // there may be a way to remove it. - String localName = shouldIgnoreAttributeCase(this) ? name.lower() : name; + AtomicString localName = shouldIgnoreAttributeCase(this) ? name.lower() : name; return attrs->getAttributeItem(localName, false); } -bool Element::hasAttributeNS(const String& namespaceURI, const String& localName) const +bool Element::hasAttributeNS(const AtomicString& namespaceURI, const AtomicString& localName) const { NamedNodeMap* attrs = attributes(true); if (!attrs) diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h index 97265e9..79815dd 100644 --- a/Source/WebCore/dom/Element.h +++ b/Source/WebCore/dom/Element.h @@ -128,11 +128,11 @@ public: bool hasAttributes() const; - bool hasAttribute(const String& name) const; - bool hasAttributeNS(const String& namespaceURI, const String& localName) const; + bool hasAttribute(const AtomicString& name) const; + bool hasAttributeNS(const AtomicString& namespaceURI, const AtomicString& localName) const; - const AtomicString& getAttribute(const String& name) const; - const AtomicString& getAttributeNS(const String& namespaceURI, const String& localName) const; + const AtomicString& getAttribute(const AtomicString& name) const; + const AtomicString& getAttributeNS(const AtomicString& namespaceURI, const AtomicString& localName) const; void setAttribute(const AtomicString& name, const AtomicString& value, ExceptionCode&); void setAttributeNS(const AtomicString& namespaceURI, const AtomicString& qualifiedName, const AtomicString& value, ExceptionCode&, FragmentScriptingPermission = FragmentScriptingAllowed); @@ -176,11 +176,11 @@ public: // Returns the absolute bounding box translated into screen coordinates: IntRect screenRect() const; - void removeAttribute(const String& name, ExceptionCode&); - void removeAttributeNS(const String& namespaceURI, const String& localName, ExceptionCode&); + void removeAttribute(const AtomicString& name, ExceptionCode&); + void removeAttributeNS(const AtomicString& namespaceURI, const AtomicString& localName, ExceptionCode&); - PassRefPtr getAttributeNode(const String& name); - PassRefPtr getAttributeNodeNS(const String& namespaceURI, const String& localName); + PassRefPtr getAttributeNode(const AtomicString& name); + PassRefPtr getAttributeNodeNS(const AtomicString& namespaceURI, const AtomicString& localName); PassRefPtr setAttributeNode(Attr*, ExceptionCode&); PassRefPtr setAttributeNodeNS(Attr*, ExceptionCode&); PassRefPtr removeAttributeNode(Attr*, ExceptionCode&); diff --git a/Source/WebCore/dom/NamedNodeMap.cpp b/Source/WebCore/dom/NamedNodeMap.cpp index 6fa30bf..253bc53 100644 --- a/Source/WebCore/dom/NamedNodeMap.cpp +++ b/Source/WebCore/dom/NamedNodeMap.cpp @@ -54,7 +54,7 @@ NamedNodeMap::~NamedNodeMap() detachAttributesFromElement(); } -PassRefPtr NamedNodeMap::getNamedItem(const String& name) const +PassRefPtr NamedNodeMap::getNamedItem(const AtomicString& name) const { Attribute* a = getAttributeItem(name, shouldIgnoreAttributeCase(m_element)); if (!a) @@ -63,12 +63,12 @@ PassRefPtr NamedNodeMap::getNamedItem(const String& name) const return a->createAttrIfNeeded(m_element); } -PassRefPtr NamedNodeMap::getNamedItemNS(const String& namespaceURI, const String& localName) const +PassRefPtr NamedNodeMap::getNamedItemNS(const AtomicString& namespaceURI, const AtomicString& localName) const { return getNamedItem(QualifiedName(nullAtom, localName, namespaceURI)); } -PassRefPtr NamedNodeMap::removeNamedItem(const String& name, ExceptionCode& ec) +PassRefPtr NamedNodeMap::removeNamedItem(const AtomicString& name, ExceptionCode& ec) { Attribute* a = getAttributeItem(name, shouldIgnoreAttributeCase(m_element)); if (!a) { @@ -79,7 +79,7 @@ PassRefPtr NamedNodeMap::removeNamedItem(const String& name, ExceptionCode return removeNamedItem(a->name(), ec); } -PassRefPtr NamedNodeMap::removeNamedItemNS(const String& namespaceURI, const String& localName, ExceptionCode& ec) +PassRefPtr NamedNodeMap::removeNamedItemNS(const AtomicString& namespaceURI, const AtomicString& localName, ExceptionCode& ec) { return removeNamedItem(QualifiedName(nullAtom, localName, namespaceURI), ec); } @@ -171,7 +171,7 @@ void NamedNodeMap::copyAttributesToVector(Vector >& copy) copy = m_attributes; } -Attribute* NamedNodeMap::getAttributeItemSlowCase(const String& name, bool shouldIgnoreAttributeCase) const +Attribute* NamedNodeMap::getAttributeItemSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const { unsigned len = length(); diff --git a/Source/WebCore/dom/NamedNodeMap.h b/Source/WebCore/dom/NamedNodeMap.h index c3c2cd9..f4aedb0 100644 --- a/Source/WebCore/dom/NamedNodeMap.h +++ b/Source/WebCore/dom/NamedNodeMap.h @@ -46,11 +46,11 @@ public: // Public DOM interface. - PassRefPtr getNamedItem(const String& name) const; - PassRefPtr removeNamedItem(const String& name, ExceptionCode&); + PassRefPtr getNamedItem(const AtomicString& name) const; + PassRefPtr removeNamedItem(const AtomicString& name, ExceptionCode&); - PassRefPtr getNamedItemNS(const String& namespaceURI, const String& localName) const; - PassRefPtr removeNamedItemNS(const String& namespaceURI, const String& localName, ExceptionCode&); + PassRefPtr getNamedItemNS(const AtomicString& namespaceURI, const AtomicString& localName) const; + PassRefPtr removeNamedItemNS(const AtomicString& namespaceURI, const AtomicString& localName, ExceptionCode&); PassRefPtr getNamedItem(const QualifiedName& name) const; PassRefPtr removeNamedItem(const QualifiedName& name, ExceptionCode&); @@ -111,8 +111,8 @@ private: void detachAttributesFromElement(); void detachFromElement(); - Attribute* getAttributeItem(const String& name, bool shouldIgnoreAttributeCase) const; - Attribute* getAttributeItemSlowCase(const String& name, bool shouldIgnoreAttributeCase) const; + Attribute* getAttributeItem(const AtomicString& name, bool shouldIgnoreAttributeCase) const; + Attribute* getAttributeItemSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const; void clearAttributes(); int declCount() const; @@ -135,7 +135,7 @@ inline Attribute* NamedNodeMap::getAttributeItem(const QualifiedName& name) cons // We use a boolean parameter instead of calling shouldIgnoreAttributeCase so that the caller // can tune the behavior (hasAttribute is case sensitive whereas getAttribute is not). -inline Attribute* NamedNodeMap::getAttributeItem(const String& name, bool shouldIgnoreAttributeCase) const +inline Attribute* NamedNodeMap::getAttributeItem(const AtomicString& name, bool shouldIgnoreAttributeCase) const { unsigned len = length(); bool doSlowCheck = shouldIgnoreAttributeCase; diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp index 1e56278..5b173bf 100644 --- a/Source/WebCore/dom/Node.cpp +++ b/Source/WebCore/dom/Node.cpp @@ -394,7 +394,7 @@ Node::~Node() else { if (m_document && rareData()->nodeLists()) m_document->removeNodeListCache(); - + NodeRareData::NodeRareDataMap& dataMap = NodeRareData::rareDataMap(); NodeRareData::NodeRareDataMap::iterator it = dataMap.find(this); ASSERT(it != dataMap.end()); @@ -534,7 +534,9 @@ void Node::setTreeScopeRecursively(TreeScope* newTreeScope) NodeRareData* Node::rareData() const { ASSERT(hasRareData()); - return NodeRareData::rareDataFromMap(this); + NodeRareData* data = isDocumentNode() ? static_cast(this)->documentRareData() : NodeRareData::rareDataFromMap(this); + ASSERT(data); + return data; } NodeRareData* Node::ensureRareData() @@ -542,9 +544,15 @@ NodeRareData* Node::ensureRareData() if (hasRareData()) return rareData(); - ASSERT(!NodeRareData::rareDataMap().contains(this)); NodeRareData* data = createRareData(); - NodeRareData::rareDataMap().set(this, data); + if (isDocumentNode()) { + // Fast path for a Document. A Document knows a pointer to NodeRareData. + ASSERT(!static_cast(this)->documentRareData()); + static_cast(this)->setDocumentRareData(data); + } else { + ASSERT(!NodeRareData::rareDataMap().contains(this)); + NodeRareData::rareDataMap().set(this, data); + } setFlag(HasRareDataFlag); return data; } diff --git a/Source/WebCore/editing/MarkupAccumulator.cpp b/Source/WebCore/editing/MarkupAccumulator.cpp index f4489c5..8ef0594 100644 --- a/Source/WebCore/editing/MarkupAccumulator.cpp +++ b/Source/WebCore/editing/MarkupAccumulator.cpp @@ -1,6 +1,7 @@ /* * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved. * Copyright (C) 2009, 2010 Google Inc. All rights reserved. + * Copyright (C) 2012, The Linux Foundation All rights reserved * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -43,34 +44,57 @@ namespace WebCore { using namespace HTMLNames; -void appendCharactersReplacingEntities(Vector& out, const UChar* content, size_t length, EntityMask entityMask) +void appendCharactersReplacingEntities(StringBuilder& result, const UChar* content, size_t length, EntityMask entityMask) { - DEFINE_STATIC_LOCAL(const String, ampReference, ("&")); - DEFINE_STATIC_LOCAL(const String, ltReference, ("<")); - DEFINE_STATIC_LOCAL(const String, gtReference, (">")); - DEFINE_STATIC_LOCAL(const String, quotReference, (""")); - DEFINE_STATIC_LOCAL(const String, nbspReference, (" ")); - - static const EntityDescription entityMaps[] = { - { '&', ampReference, EntityAmp }, - { '<', ltReference, EntityLt }, - { '>', gtReference, EntityGt }, - { '"', quotReference, EntityQuot }, - { noBreakSpace, nbspReference, EntityNbsp }, - }; - size_t positionAfterLastEntity = 0; - for (size_t i = 0; i < length; ++i) { - for (size_t m = 0; m < WTF_ARRAY_LENGTH(entityMaps); ++m) { - if (content[i] == entityMaps[m].entity && entityMaps[m].mask & entityMask) { - out.append(content + positionAfterLastEntity, i - positionAfterLastEntity); - append(out, entityMaps[m].reference); + if (entityMask & EntityNbsp) { + for (size_t i = 0; i < length; ++i) { + UChar c = content[i]; + if (c == noBreakSpace) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append(" ", 6); + positionAfterLastEntity = i + 1; + } else if (c == '&' && EntityAmp & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append("&", 5); + positionAfterLastEntity = i + 1; + } else if (c == '<' && EntityLt & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append("<", 4); + positionAfterLastEntity = i + 1; + } else if (c == '>' && EntityGt & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append(">", 4); + positionAfterLastEntity = i + 1; + } else if (c == '"' && EntityQuot & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append(""", 6); + positionAfterLastEntity = i + 1; + } + } + }else if (entityMask) { + for (size_t i = 0; i < length; ++i) { + UChar c = content[i]; + if (c == '&' && EntityAmp & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append("&", 5); + positionAfterLastEntity = i + 1; + } else if (c == '<' && EntityLt & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append("<", 4); + positionAfterLastEntity = i + 1; + } else if (c == '>' && EntityGt & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append(">", 4); + positionAfterLastEntity = i + 1; + } else if (c == '"' && EntityQuot & entityMask) { + result.append(content + positionAfterLastEntity, i - positionAfterLastEntity); + result.append(""", 6); positionAfterLastEntity = i + 1; - break; } } } - out.append(content + positionAfterLastEntity, length - positionAfterLastEntity); + result.append(content + positionAfterLastEntity, length - positionAfterLastEntity); } MarkupAccumulator::MarkupAccumulator(Vector* nodes, EAbsoluteURLs shouldResolveURLs, const Range* range) @@ -86,11 +110,8 @@ MarkupAccumulator::~MarkupAccumulator() String MarkupAccumulator::serializeNodes(Node* node, Node* nodeToSkip, EChildrenOnly childrenOnly) { - Vector out; serializeNodesWithNamespaces(node, nodeToSkip, childrenOnly, 0); - out.reserveInitialCapacity(length()); - concatenateMarkup(out); - return String::adopt(out); + return m_markup.toString(); } void MarkupAccumulator::serializeNodesWithNamespaces(Node* node, Node* nodeToSkip, EChildrenOnly childrenOnly, const Namespaces* namespaces) @@ -116,23 +137,19 @@ void MarkupAccumulator::serializeNodesWithNamespaces(Node* node, Node* nodeToSki void MarkupAccumulator::appendString(const String& string) { - m_succeedingMarkup.append(string); + m_markup.append(string); } void MarkupAccumulator::appendStartTag(Node* node, Namespaces* namespaces) { - Vector markup; - appendStartMarkup(markup, node, namespaces); - appendString(String::adopt(markup)); + appendStartMarkup(m_markup, node, namespaces); if (m_nodes) m_nodes->append(node); } void MarkupAccumulator::appendEndTag(Node* node) { - Vector markup; - appendEndMarkup(markup, node); - appendString(String::adopt(markup)); + appendEndMarkup(m_markup, node); } size_t MarkupAccumulator::totalLength(const Vector& strings) @@ -143,35 +160,31 @@ size_t MarkupAccumulator::totalLength(const Vector& strings) return length; } -// FIXME: This is a very inefficient way of accumulating the markup. -// We're converting results of appendStartMarkup and appendEndMarkup from Vector to String -// and then back to Vector and again to String here. -void MarkupAccumulator::concatenateMarkup(Vector& out) +void MarkupAccumulator::concatenateMarkup(StringBuilder& result) { - for (size_t i = 0; i < m_succeedingMarkup.size(); ++i) - append(out, m_succeedingMarkup[i]); + result.append(m_markup); } -void MarkupAccumulator::appendAttributeValue(Vector& result, const String& attribute, bool documentIsHTML) +void MarkupAccumulator::appendAttributeValue(StringBuilder& result, const String& attribute, bool documentIsHTML) { appendCharactersReplacingEntities(result, attribute.characters(), attribute.length(), documentIsHTML ? EntityMaskInHTMLAttributeValue : EntityMaskInAttributeValue); } -void MarkupAccumulator::appendQuotedURLAttributeValue(Vector& result, const String& urlString) +void MarkupAccumulator::appendQuotedURLAttributeValue(StringBuilder& result, const String& urlString) { - UChar quoteChar = '\"'; + UChar quoteChar = '"'; String strippedURLString = urlString.stripWhiteSpace(); if (protocolIsJavaScript(strippedURLString)) { // minimal escaping for javascript urls if (strippedURLString.contains('"')) { if (strippedURLString.contains('\'')) - strippedURLString.replace('\"', """); + strippedURLString.replace('"', """); else quoteChar = '\''; } result.append(quoteChar); - append(result, strippedURLString); + result.append(strippedURLString); result.append(quoteChar); return; } @@ -182,7 +195,7 @@ void MarkupAccumulator::appendQuotedURLAttributeValue(Vector& result, con result.append(quoteChar); } -void MarkupAccumulator::appendNodeValue(Vector& out, const Node* node, const Range* range, EntityMask entityMask) +void MarkupAccumulator::appendNodeValue(StringBuilder& out, const Node* node, const Range* range, EntityMask entityMask) { String str = node->nodeValue(); const UChar* characters = str.characters(); @@ -229,7 +242,7 @@ bool MarkupAccumulator::shouldAddNamespaceAttribute(const Attribute& attribute, return true; } -void MarkupAccumulator::appendNamespace(Vector& result, const AtomicString& prefix, const AtomicString& namespaceURI, Namespaces& namespaces) +void MarkupAccumulator::appendNamespace(StringBuilder& result, const AtomicString& prefix, const AtomicString& namespaceURI, Namespaces& namespaces) { namespaces.checkConsistency(); if (namespaceURI.isEmpty()) @@ -241,10 +254,10 @@ void MarkupAccumulator::appendNamespace(Vector& result, const AtomicStrin if (foundNS != namespaceURI.impl()) { namespaces.set(pre, namespaceURI.impl()); result.append(' '); - append(result, xmlnsAtom.string()); + result.append(xmlnsAtom.string()); if (!prefix.isEmpty()) { result.append(':'); - append(result, prefix); + result.append(prefix); } result.append('='); @@ -259,66 +272,75 @@ EntityMask MarkupAccumulator::entityMaskForText(Text* text) const const QualifiedName* parentName = 0; if (text->parentElement()) parentName = &static_cast(text->parentElement())->tagQName(); - + if (parentName && (*parentName == scriptTag || *parentName == styleTag || *parentName == xmpTag)) return EntityMaskInCDATA; return text->document()->isHTMLDocument() ? EntityMaskInHTMLPCDATA : EntityMaskInPCDATA; } -void MarkupAccumulator::appendText(Vector& out, Text* text) +void MarkupAccumulator::appendText(StringBuilder& result, Text* text) { - appendNodeValue(out, text, m_range, entityMaskForText(text)); + appendNodeValue(result, text, m_range, entityMaskForText(text)); } -void MarkupAccumulator::appendComment(Vector& out, const String& comment) +void MarkupAccumulator::appendComment(StringBuilder& result, const String& comment) { // FIXME: Comment content is not escaped, but XMLSerializer (and possibly other callers) should raise an exception if it includes "-->". - append(out, ""); + static const char commentBegin[] = ""; + result.append(commentEnd, sizeof(commentEnd) - 1); } -void MarkupAccumulator::appendDocumentType(Vector& result, const DocumentType* n) +void MarkupAccumulator::appendDocumentType(StringBuilder& result, const DocumentType* n) { if (n->name().isEmpty()) return; - append(result, "name()); + static const char doctypeString[] = "name()); if (!n->publicId().isEmpty()) { - append(result, " PUBLIC \""); - append(result, n->publicId()); - append(result, "\""); + static const char publicString[] = " PUBLIC \""; + result.append(publicString, sizeof(publicString) - 1); + result.append(n->publicId()); + result.append('"'); if (!n->systemId().isEmpty()) { - append(result, " \""); - append(result, n->systemId()); - append(result, "\""); + result.append(' '); + result.append('"'); + result.append(n->systemId()); + result.append('"'); } } else if (!n->systemId().isEmpty()) { - append(result, " SYSTEM \""); - append(result, n->systemId()); - append(result, "\""); + static const char systemString[] = " SYSTEM \""; + result.append(systemString, sizeof(systemString) - 1); + result.append(n->systemId()); + result.append('"'); } if (!n->internalSubset().isEmpty()) { - append(result, " ["); - append(result, n->internalSubset()); - append(result, "]"); + result.append(' '); + result.append('['); + result.append(n->internalSubset()); + result.append(']'); } - append(result, ">"); + result.append('>'); } -void MarkupAccumulator::appendProcessingInstruction(Vector& out, const String& target, const String& data) +void MarkupAccumulator::appendProcessingInstruction(StringBuilder& result, const String& target, const String& data) { // FIXME: PI data is not escaped, but XMLSerializer (and possibly other callers) this should raise an exception if it includes "?>". - append(out, ""); + result.append('<'); + result.append('?'); + result.append(target); + result.append(' '); + result.append(data); + result.append('?'); + result.append('>'); } -void MarkupAccumulator::appendElement(Vector& out, Element* element, Namespaces* namespaces) +void MarkupAccumulator::appendElement(StringBuilder& out, Element* element, Namespaces* namespaces) { appendOpenTag(out, element, namespaces); @@ -330,15 +352,15 @@ void MarkupAccumulator::appendElement(Vector& out, Element* element, Name appendCloseTag(out, element); } -void MarkupAccumulator::appendOpenTag(Vector& out, Element* element, Namespaces* namespaces) +void MarkupAccumulator::appendOpenTag(StringBuilder& out, Element* element, Namespaces* namespaces) { out.append('<'); - append(out, element->nodeNamePreservingCase()); + out.append(element->nodeNamePreservingCase()); if (!element->document()->isHTMLDocument() && namespaces && shouldAddNamespaceElement(element)) appendNamespace(out, element->prefix(), element->namespaceURI(), *namespaces); } -void MarkupAccumulator::appendCloseTag(Vector& out, Element* element) +void MarkupAccumulator::appendCloseTag(StringBuilder& out, Element* element) { if (shouldSelfClose(element)) { if (element->isHTMLElement()) @@ -348,16 +370,16 @@ void MarkupAccumulator::appendCloseTag(Vector& out, Element* element) out.append('>'); } -void MarkupAccumulator::appendAttribute(Vector& out, Element* element, const Attribute& attribute, Namespaces* namespaces) +void MarkupAccumulator::appendAttribute(StringBuilder& out, Element* element, const Attribute& attribute, Namespaces* namespaces) { bool documentIsHTML = element->document()->isHTMLDocument(); out.append(' '); if (documentIsHTML) - append(out, attribute.name().localName()); + out.append(attribute.name().localName()); else - append(out, attribute.name().toString()); + out.append(attribute.name().toString()); out.append('='); @@ -369,24 +391,26 @@ void MarkupAccumulator::appendAttribute(Vector& out, Element* element, co else appendQuotedURLAttributeValue(out, attribute.value()); } else { - out.append('\"'); + out.append('"'); appendAttributeValue(out, attribute.value(), documentIsHTML); - out.append('\"'); + out.append('"'); } if (!documentIsHTML && namespaces && shouldAddNamespaceAttribute(attribute, *namespaces)) appendNamespace(out, attribute.prefix(), attribute.namespaceURI(), *namespaces); } -void MarkupAccumulator::appendCDATASection(Vector& out, const String& section) +void MarkupAccumulator::appendCDATASection(StringBuilder& result, const String& section) { // FIXME: CDATA content is not escaped, but XMLSerializer (and possibly other callers) should raise an exception if it includes "]]>". - append(out, ""); + static const char cdataBegin[] = ""; + result.append(cdataEnd, sizeof(cdataEnd) - 1); } -void MarkupAccumulator::appendStartMarkup(Vector& result, const Node* node, Namespaces* namespaces) +void MarkupAccumulator::appendStartMarkup(StringBuilder& result, const Node* node, Namespaces* namespaces) { if (namespaces) namespaces->checkConsistency(); @@ -443,7 +467,7 @@ bool MarkupAccumulator::elementCannotHaveEndTag(const Node* node) { if (!node->isHTMLElement()) return false; - + // FIXME: ieForbidsInsertHTML may not be the right function to call here // ieForbidsInsertHTML is used to disallow setting innerHTML/outerHTML // or createContextualFragment. It does not necessarily align with @@ -451,14 +475,14 @@ bool MarkupAccumulator::elementCannotHaveEndTag(const Node* node) return static_cast(node)->ieForbidsInsertHTML(); } -void MarkupAccumulator::appendEndMarkup(Vector& result, const Node* node) +void MarkupAccumulator::appendEndMarkup(StringBuilder& result, const Node* node) { if (!node->isElementNode() || shouldSelfClose(node) || (!node->hasChildNodes() && elementCannotHaveEndTag(node))) return; result.append('<'); result.append('/'); - append(result, static_cast(node)->nodeNamePreservingCase()); + result.append(static_cast(node)->nodeNamePreservingCase()); result.append('>'); } diff --git a/Source/WebCore/editing/MarkupAccumulator.h b/Source/WebCore/editing/MarkupAccumulator.h index 0bfc6e6..ba28288 100644 --- a/Source/WebCore/editing/MarkupAccumulator.h +++ b/Source/WebCore/editing/MarkupAccumulator.h @@ -30,6 +30,7 @@ #include "markup.h" #include #include +#include namespace WebCore { @@ -76,28 +77,28 @@ protected: void appendStartTag(Node*, Namespaces* = 0); void appendEndTag(Node*); static size_t totalLength(const Vector&); - size_t length() const { return totalLength(m_succeedingMarkup); } - void concatenateMarkup(Vector& out); - void appendAttributeValue(Vector& result, const String& attribute, bool documentIsHTML); - void appendQuotedURLAttributeValue(Vector& result, const String& urlString); - void appendNodeValue(Vector& out, const Node*, const Range*, EntityMask); + size_t length() const { return m_markup.length(); } + void concatenateMarkup(StringBuilder& out); + void appendAttributeValue(StringBuilder& result, const String& attribute, bool documentIsHTML); + void appendQuotedURLAttributeValue(StringBuilder& result, const String& urlString); + void appendNodeValue(StringBuilder& out, const Node*, const Range*, EntityMask); bool shouldAddNamespaceElement(const Element*); bool shouldAddNamespaceAttribute(const Attribute&, Namespaces&); - void appendNamespace(Vector& result, const AtomicString& prefix, const AtomicString& namespaceURI, Namespaces&); + void appendNamespace(StringBuilder& result, const AtomicString& prefix, const AtomicString& namespaceURI, Namespaces&); EntityMask entityMaskForText(Text* text) const; - virtual void appendText(Vector& out, Text*); - void appendComment(Vector& out, const String& comment); - void appendDocumentType(Vector& result, const DocumentType*); - void appendProcessingInstruction(Vector& out, const String& target, const String& data); - virtual void appendElement(Vector& out, Element*, Namespaces*); - void appendOpenTag(Vector& out, Element* element, Namespaces*); - void appendCloseTag(Vector& out, Element* element); - void appendAttribute(Vector& out, Element* element, const Attribute&, Namespaces*); - void appendCDATASection(Vector& out, const String& section); - void appendStartMarkup(Vector& result, const Node*, Namespaces*); + virtual void appendText(StringBuilder& out, Text*); + void appendComment(StringBuilder& out, const String& comment); + void appendDocumentType(StringBuilder& result, const DocumentType*); + void appendProcessingInstruction(StringBuilder& out, const String& target, const String& data); + virtual void appendElement(StringBuilder& out, Element*, Namespaces*); + void appendOpenTag(StringBuilder& out, Element* element, Namespaces*); + void appendCloseTag(StringBuilder& out, Element* element); + void appendAttribute(StringBuilder& out, Element* element, const Attribute&, Namespaces*); + void appendCDATASection(StringBuilder& out, const String& section); + void appendStartMarkup(StringBuilder& result, const Node*, Namespaces*); bool shouldSelfClose(const Node*); bool elementCannotHaveEndTag(const Node* node); - void appendEndMarkup(Vector& result, const Node*); + void appendEndMarkup(StringBuilder&, const Node*); bool shouldResolveURLs() { return m_shouldResolveURLs == AbsoluteURLs; } @@ -107,12 +108,12 @@ protected: private: void serializeNodesWithNamespaces(Node*, Node* nodeToSkip, EChildrenOnly, const Namespaces*); - Vector m_succeedingMarkup; + StringBuilder m_markup; const bool m_shouldResolveURLs; }; // FIXME: This method should be integrated with MarkupAccumulator. -void appendCharactersReplacingEntities(Vector& out, const UChar* content, size_t length, EntityMask entityMask); +void appendCharactersReplacingEntities(StringBuilder& out, const UChar* content, size_t length, EntityMask entityMask); } diff --git a/Source/WebCore/editing/markup.cpp b/Source/WebCore/editing/markup.cpp index 5ef3d7d..a02ff03 100644 --- a/Source/WebCore/editing/markup.cpp +++ b/Source/WebCore/editing/markup.cpp @@ -130,12 +130,12 @@ public: String takeResults(); private: - virtual void appendText(Vector& out, Text*); + virtual void appendText(StringBuilder& out, Text*); String renderedText(const Node*, const Range*); String stringValueForRange(const Node*, const Range*); void removeExteriorStyles(CSSMutableStyleDeclaration*); - void appendElement(Vector& out, Element* element, bool addDisplayInline, RangeFullySelectsNode); - void appendElement(Vector& out, Element* element, Namespaces*) { appendElement(out, element, false, DoesFullySelectNode); } + void appendElement(StringBuilder& out, Element* element, bool addDisplayInline, RangeFullySelectsNode); + void appendElement(StringBuilder& out, Element* element, Namespaces*) { appendElement(out, element, false, DoesFullySelectNode); } bool shouldAnnotate() { return m_shouldAnnotate == AnnotateForInterchange; } @@ -145,12 +145,12 @@ private: void StyledMarkupAccumulator::wrapWithNode(Node* node, bool convertBlocksToInlines, RangeFullySelectsNode rangeFullySelectsNode) { - Vector markup; + StringBuilder markup; if (node->isElementNode()) appendElement(markup, static_cast(node), convertBlocksToInlines && isBlock(const_cast(node)), rangeFullySelectsNode); else appendStartMarkup(markup, node, 0); - m_reversedPrecedingMarkup.append(String::adopt(markup)); + m_reversedPrecedingMarkup.append(markup.toString()); appendEndTag(node); if (m_nodes) m_nodes->append(node); @@ -165,30 +165,30 @@ void StyledMarkupAccumulator::wrapWithStyleNode(CSSStyleDeclaration* style, Docu DEFINE_STATIC_LOCAL(const String, divClose, ("")); DEFINE_STATIC_LOCAL(const String, styleSpanOpen, ("")); - Vector openTag; - append(openTag, isBlock ? divStyle : styleSpanOpen); + StringBuilder openTag; + openTag.append(isBlock ? divStyle : styleSpanOpen); appendAttributeValue(openTag, style->cssText(), document->isHTMLDocument()); openTag.append('\"'); openTag.append('>'); - m_reversedPrecedingMarkup.append(String::adopt(openTag)); + m_reversedPrecedingMarkup.append(openTag.toString()); appendString(isBlock ? divClose : styleSpanClose); } String StyledMarkupAccumulator::takeResults() { - Vector result; - result.reserveInitialCapacity(totalLength(m_reversedPrecedingMarkup) + length()); + StringBuilder result; + result.reserveCapacity(totalLength(m_reversedPrecedingMarkup) + length()); for (size_t i = m_reversedPrecedingMarkup.size(); i > 0; --i) - append(result, m_reversedPrecedingMarkup[i - 1]); + result.append(m_reversedPrecedingMarkup[i - 1]); concatenateMarkup(result); // We remove '\0' characters because they are not visibly rendered to the user. - return String::adopt(result).replace(0, ""); + return result.toString().replace(0, ""); } -void StyledMarkupAccumulator::appendText(Vector& out, Text* text) +void StyledMarkupAccumulator::appendText(StringBuilder& out, Text* text) { if (!shouldAnnotate() || (text->parentElement() && text->parentElement()->tagQName() == textareaTag)) { MarkupAccumulator::appendText(out, text); @@ -197,9 +197,9 @@ void StyledMarkupAccumulator::appendText(Vector& out, Text* text) bool useRenderedText = !enclosingNodeWithTag(firstPositionInNode(text), selectTag); String content = useRenderedText ? renderedText(text, m_range) : stringValueForRange(text, m_range); - Vector buffer; + StringBuilder buffer; appendCharactersReplacingEntities(buffer, content.characters(), content.length(), EntityMaskInPCDATA); - append(out, convertHTMLTextToInterchangeFormat(String::adopt(buffer), text)); + out.append(convertHTMLTextToInterchangeFormat(buffer.toString(), text)); } String StyledMarkupAccumulator::renderedText(const Node* node, const Range* range) @@ -252,7 +252,7 @@ static PassRefPtr styleFromMatchedRulesForElement(El return style.release(); } -void StyledMarkupAccumulator::appendElement(Vector& out, Element* element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode) +void StyledMarkupAccumulator::appendElement(StringBuilder& out, Element* element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode) { bool documentIsHTML = element->document()->isHTMLDocument(); appendOpenTag(out, element, 0); @@ -303,7 +303,7 @@ void StyledMarkupAccumulator::appendElement(Vector& out, Element* element removeExteriorStyles(style.get()); if (style->length() > 0) { DEFINE_STATIC_LOCAL(const String, stylePrefix, (" style=\"")); - append(out, stylePrefix); + out.append(stylePrefix); appendAttributeValue(out, style->cssText(), documentIsHTML); out.append('\"'); } @@ -819,7 +819,7 @@ PassRefPtr createFragmentFromText(Range* context, const String if (s.isEmpty() && i + 1 == numLines) { // For last line, use the "magic BR" rather than a P. element = createBreakElement(document); - element->setAttribute(classAttr, AppleInterchangeNewline); + element->setAttribute(classAttr, AppleInterchangeNewline); } else { if (useClonesOfEnclosingBlock) element = block->cloneElementWithoutChildren(); @@ -905,13 +905,13 @@ String createFullMarkup(const Range* range) String urlToMarkup(const KURL& url, const String& title) { - Vector markup; - append(markup, ""); + StringBuilder markup; + markup.append(""); appendCharactersReplacingEntities(markup, title.characters(), title.length(), EntityMaskInPCDATA); - append(markup, ""); - return String::adopt(markup); + markup.append(""); + return markup.toString(); } } -- cgit v1.1