Have Document::parseQualifiedName() deal with AtomicStrings

Have Document::parseQualifiedName() deal with AtomicStrings

Have Document::parseQualifiedName() deal with AtomicStrings instead of Strings as
its purpose is to extract a prefix and localName that the caller can use to
construct a QualifiedName object. The QualifiedName constructor takes
AtomicStrings in argument so it does not make sense for parseQualifiedName() to
construct and return Strings. The caller will always need to convert these to
AtomicStrings, which has a cost.

R=abarth, eseidel
BUG=323739
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163355

[eseidel, 2013-12-06 17:02:19]

Lgtm
On Dec 6, 2013 8:28 AM, <ch.dumez@samsung.com> wrote:

> Reviewers: abarth, eseidel,
>
> Description:
> Have Document::parseQualifiedName() deal with AtomicStrings
>
> Have Document::parseQualifiedName() deal with AtomicStrings instead of
> Strings
> as
> its purpose is to extract a prefix and localName that the caller can use to
> construct a QualifiedName object. The QualifiedName constructor takes
> AtomicStrings in argument so it does not make sense for
> parseQualifiedName() to
> construct and return Strings. The caller will always need to convert these
> to
> AtomicStrings, which has a cost.
>
> R=abarth, eseidel
> BUG=323739
>
> Please review this at https://codereview.chromium.org/108483002/
>
> SVN Base: https://chromium.googlesource.com/chromium/blink.git@master
>
> Affected files (+31, -30 lines):
>   M Source/core/dom/DOMImplementation.h
>   M Source/core/dom/DOMImplementation.cpp
>   M Source/core/dom/Document.h
>   M Source/core/dom/Document.cpp
>   M Source/core/dom/Element.cpp
>   M Source/core/editing/EditorCommand.cpp
>   M Source/core/svg/animation/SVGSMILElement.cpp
>
>
> Index: Source/core/dom/DOMImplementation.cpp
> diff --git a/Source/core/dom/DOMImplementation.cpp b/Source/core/dom/
> DOMImplementation.cpp
> index 33efde51610da46c82bbf76ac0842c31ca9ddf09..
> ff898a52dea55fc0f6fe0adb3536cf06815d005b 100644
> --- a/Source/core/dom/DOMImplementation.cpp
> +++ b/Source/core/dom/DOMImplementation.cpp
> @@ -178,10 +178,10 @@ bool DOMImplementation::hasFeature(const String&
> feature, const String& version)
>      return true;
>  }
>
> -PassRefPtr<DocumentType> DOMImplementation::createDocumentType(const
> String& qualifiedName,
> +PassRefPtr<DocumentType> DOMImplementation::createDocumentType(const
> AtomicString& qualifiedName,
>      const String& publicId, const String& systemId, ExceptionState&
> exceptionState)
>  {
> -    String prefix, localName;
> +    AtomicString prefix, localName;
>      if (!Document::parseQualifiedName(qualifiedName, prefix, localName,
> exceptionState))
>          return 0;
>
> @@ -193,8 +193,8 @@ DOMImplementation* DOMImplementation::getInterface(const
> String& /*feature*/)
>      return 0;
>  }
>
> -PassRefPtr<Document> DOMImplementation::createDocument(const String&
> namespaceURI,
> -    const String& qualifiedName, DocumentType* doctype, ExceptionState&
> exceptionState)
> +PassRefPtr<Document> DOMImplementation::createDocument(const
> AtomicString& namespaceURI,
> +    const AtomicString& qualifiedName, DocumentType* doctype,
> ExceptionState& exceptionState)
>  {
>      RefPtr<Document> doc;
>      DocumentInit init = DocumentInit::fromContext(m_
> document.contextDocument());
> Index: Source/core/dom/DOMImplementation.h
> diff --git a/Source/core/dom/DOMImplementation.h b/Source/core/dom/
> DOMImplementation.h
> index 76852d3ef05c00bd27116705c0e4542a59f95fd9..
> b710d2ad22cc3b0def330820cb9566362f3f3f0b 100644
> --- a/Source/core/dom/DOMImplementation.h
> +++ b/Source/core/dom/DOMImplementation.h
> @@ -50,8 +50,8 @@ public:
>
>      // DOM methods & attributes for DOMImplementation
>      static bool hasFeature(const String& feature, const String& version);
> -    PassRefPtr<DocumentType> createDocumentType(const String&
> qualifiedName, const String& publicId, const String& systemId,
> ExceptionState&);
> -    PassRefPtr<Document> createDocument(const String& namespaceURI, const
> String& qualifiedName, DocumentType*, ExceptionState&);
> +    PassRefPtr<DocumentType> createDocumentType(const AtomicString&
> qualifiedName, const String& publicId, const String& systemId,
> ExceptionState&);
> +    PassRefPtr<Document> createDocument(const AtomicString& namespaceURI,
> const AtomicString& qualifiedName, DocumentType*, ExceptionState&);
>
>      DOMImplementation* getInterface(const String& feature);
>
> Index: Source/core/dom/Document.cpp
> diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
> index aefd0fc7983607d401e33fd9115884ece3f02d80..
> 99255d7538e03ad2787790177fb749afd2765506 100644
> --- a/Source/core/dom/Document.cpp
> +++ b/Source/core/dom/Document.cpp
> @@ -709,9 +709,9 @@ PassRefPtr<Element> Document::createElement(const
> AtomicString& localName, const
>      return element;
>  }
>
> -PassRefPtr<Element> Document::createElementNS(const AtomicString&
> namespaceURI, const String& qualifiedName, const AtomicString&
> typeExtension, ExceptionState& exceptionState)
> +PassRefPtr<Element> Document::createElementNS(const AtomicString&
> namespaceURI, const AtomicString& qualifiedName, const AtomicString&
> typeExtension, ExceptionState& exceptionState)
>  {
> -    String prefix, localName;
> +    AtomicString prefix, localName;
>      if (!parseQualifiedName(qualifiedName, prefix, localName,
> exceptionState))
>          return 0;
>
> @@ -1019,9 +1019,9 @@ NamedFlowCollection* Document::namedFlows()
>      return m_namedFlows.get();
>  }
>
> -PassRefPtr<Element> Document::createElementNS(const String&
> namespaceURI, const String& qualifiedName, ExceptionState& exceptionState)
> +PassRefPtr<Element> Document::createElementNS(const AtomicString&
> namespaceURI, const AtomicString& qualifiedName, ExceptionState&
> exceptionState)
>  {
> -    String prefix, localName;
> +    AtomicString prefix, localName;
>      if (!parseQualifiedName(qualifiedName, prefix, localName,
> exceptionState))
>          return 0;
>
> @@ -3878,7 +3878,7 @@ bool Document::isValidName(const String& name)
>  }
>
>  template<typename CharType>
> -static bool parseQualifiedNameInternal(const String& qualifiedName,
> const CharType* characters, unsigned length, String& prefix, String&
> localName, ExceptionState& exceptionState)
> +static bool parseQualifiedNameInternal(const AtomicString&
> qualifiedName, const CharType* characters, unsigned length, AtomicString&
> prefix, AtomicString& localName, ExceptionState& exceptionState)
>  {
>      bool nameStart = true;
>      bool sawColon = false;
> @@ -3910,15 +3910,16 @@ static bool parseQualifiedNameInternal(const
> String& qualifiedName, const CharTy
>      }
>
>      if (!sawColon) {
> -        prefix = String();
> +        prefix = nullAtom;
>          localName = qualifiedName;
>      } else {
> -        prefix = qualifiedName.substring(0, colonPos);
> +        prefix = AtomicString(characters, colonPos);
>          if (prefix.isEmpty()) {
>              exceptionState.throwUninformativeAndGenericDO
> MException(NamespaceError);
>              return false;
>          }
> -        localName = qualifiedName.substring(colonPos + 1);
> +        int prefixStart = colonPos + 1;
> +        localName = AtomicString(characters + prefixStart, length -
> prefixStart);
>      }
>
>      if (localName.isEmpty()) {
> @@ -3929,7 +3930,7 @@ static bool parseQualifiedNameInternal(const
> String& qualifiedName, const CharTy
>      return true;
>  }
>
> -bool Document::parseQualifiedName(const String& qualifiedName, String&
> prefix, String& localName, ExceptionState& exceptionState)
> +bool Document::parseQualifiedName(const AtomicString& qualifiedName,
> AtomicString& prefix, AtomicString& localName, ExceptionState&
> exceptionState)
>  {
>      unsigned length = qualifiedName.length();
>
> @@ -4146,14 +4147,14 @@ WeakPtr<Document> Document::contextDocument()
>      return WeakPtr<Document>(0);
>  }
>
> -PassRefPtr<Attr> Document::createAttribute(const String& name,
> ExceptionState& exceptionState)
> +PassRefPtr<Attr> Document::createAttribute(const AtomicString& name,
> ExceptionState& exceptionState)
>  {
> -    return createAttributeNS(String(), name, exceptionState, true);
> +    return createAttributeNS(nullAtom, name, exceptionState, true);
>  }
>
> -PassRefPtr<Attr> Document::createAttributeNS(const String& namespaceURI,
> const String& qualifiedName, ExceptionState& exceptionState, bool
> shouldIgnoreNamespaceChecks)
> +PassRefPtr<Attr> Document::createAttributeNS(const AtomicString&
> namespaceURI, const AtomicString& qualifiedName, ExceptionState&
> exceptionState, bool shouldIgnoreNamespaceChecks)
>  {
> -    String prefix, localName;
> +    AtomicString prefix, localName;
>      if (!parseQualifiedName(qualifiedName, prefix, localName,
> exceptionState))
>          return 0;
>
> Index: Source/core/dom/Document.h
> diff --git a/Source/core/dom/Document.h b/Source/core/dom/Document.h
> index 4e83e442691b9f6f57911f38fcd66266701d2ca7..
> b1cc65c6999f96acd7cbb4abd21dca8c0e7219d0 100644
> --- a/Source/core/dom/Document.h
> +++ b/Source/core/dom/Document.h
> @@ -290,11 +290,11 @@ public:
>      PassRefPtr<Comment> createComment(const String& data);
>      PassRefPtr<CDATASection> createCDATASection(const String& data,
> ExceptionState&);
>      PassRefPtr<ProcessingInstruction> createProcessingInstruction(const
> String& target, const String& data, ExceptionState&);
> -    PassRefPtr<Attr> createAttribute(const String& name, ExceptionState&);
> -    PassRefPtr<Attr> createAttributeNS(const String& namespaceURI, const
> String& qualifiedName, ExceptionState&, bool shouldIgnoreNamespaceChecks =
> false);
> +    PassRefPtr<Attr> createAttribute(const AtomicString& name,
> ExceptionState&);
> +    PassRefPtr<Attr> createAttributeNS(const AtomicString& namespaceURI,
> const AtomicString& qualifiedName, ExceptionState&, bool
> shouldIgnoreNamespaceChecks = false);
>      PassRefPtr<Node> importNode(Node* importedNode, ExceptionState& ec) {
> return importNode(importedNode, true, ec); }
>      PassRefPtr<Node> importNode(Node* importedNode, bool deep,
> ExceptionState&);
> -    PassRefPtr<Element> createElementNS(const String& namespaceURI, const
> String& qualifiedName, ExceptionState&);
> +    PassRefPtr<Element> createElementNS(const AtomicString& namespaceURI,
> const AtomicString& qualifiedName, ExceptionState&);
>      PassRefPtr<Element> createElement(const QualifiedName&, bool
> createdByParser);
>
>      PassRefPtr<DOMNamedFlowCollection> webkitGetNamedFlows();
> @@ -754,7 +754,7 @@ public:
>      // The following breaks a qualified name into a prefix and a local
> name.
>      // It also does a validity check, and returns false if the qualified
> name
>      // is invalid.  It also sets ExceptionCode when name is invalid.
> -    static bool parseQualifiedName(const String& qualifiedName, String&
> prefix, String& localName, ExceptionState&);
> +    static bool parseQualifiedName(const AtomicString& qualifiedName,
> AtomicString& prefix, AtomicString& localName, ExceptionState&);
>
>      // Checks to make sure prefix and namespace do not conflict (per DOM
> Core 3)
>      static bool hasValidNamespaceForElements(const QualifiedName&);
> @@ -944,7 +944,7 @@ public:
>      FastTextAutosizer* fastTextAutosizer();
>
>      PassRefPtr<Element> createElement(const AtomicString& localName,
> const AtomicString& typeExtension, ExceptionState&);
> -    PassRefPtr<Element> createElementNS(const AtomicString& namespaceURI,
> const String& qualifiedName, const AtomicString& typeExtension,
> ExceptionState&);
> +    PassRefPtr<Element> createElementNS(const AtomicString& namespaceURI,
> const AtomicString& qualifiedName, const AtomicString& typeExtension,
> ExceptionState&);
>      ScriptValue registerElement(WebCore::ScriptState*, const
> AtomicString& name, ExceptionState&);
>      ScriptValue registerElement(WebCore::ScriptState*, const
> AtomicString& name, const Dictionary& options, ExceptionState&,
> CustomElement::NameSet validNames = CustomElement::StandardNames);
>      CustomElementRegistrationContext* registrationContext() { return
> m_registrationContext.get(); }
> Index: Source/core/dom/Element.cpp
> diff --git a/Source/core/dom/Element.cpp b/Source/core/dom/Element.cpp
> index cca4eab671f19e85afd63d5b385d881983ffa15d..
> f0b7b12554dac75334e698614c4d248274eebca2 100644
> --- a/Source/core/dom/Element.cpp
> +++ b/Source/core/dom/Element.cpp
> @@ -2000,7 +2000,7 @@ PassRefPtr<Attr> Element::removeAttributeNode(Attr*
> attr, ExceptionState& except
>
>  bool Element::parseAttributeName(QualifiedName& out, const AtomicString&
> namespaceURI, const AtomicString& qualifiedName, ExceptionState&
> exceptionState)
>  {
> -    String prefix, localName;
> +    AtomicString prefix, localName;
>      if (!Document::parseQualifiedName(qualifiedName, prefix, localName,
> exceptionState))
>          return false;
>      ASSERT(!exceptionState.hadException());
> Index: Source/core/editing/EditorCommand.cpp
> diff --git a/Source/core/editing/EditorCommand.cpp b/Source/core/editing/
> EditorCommand.cpp
> index 46ae8af3358cad6465d5ecc7da9b1723b1f17096..
> 8c6291af1e013a5190945fdca9431aaa2e9e45aa 100644
> --- a/Source/core/editing/EditorCommand.cpp
> +++ b/Source/core/editing/EditorCommand.cpp
> @@ -436,8 +436,8 @@ static bool executeFormatBlock(Frame& frame, Event*,
> EditorCommandSource, const
>      if (tagName[0] == '<' && tagName[tagName.length() - 1] == '>')
>          tagName = tagName.substring(1, tagName.length() - 2);
>
> -    String localName, prefix;
> -    if (!Document::parseQualifiedName(tagName, prefix, localName,
> IGNORE_EXCEPTION))
> +    AtomicString localName, prefix;
> +    if (!Document::parseQualifiedName(AtomicString(tagName), prefix,
> localName, IGNORE_EXCEPTION))
>          return false;
>      QualifiedName qualifiedTagName(prefix, localName, xhtmlNamespaceURI);
>
> Index: Source/core/svg/animation/SVGSMILElement.cpp
> diff --git a/Source/core/svg/animation/SVGSMILElement.cpp
> b/Source/core/svg/animation/SVGSMILElement.cpp
> index c8a3014aa6b709f073034e01da592986037b42e5..
> f79ef568573eb80b04096efbbee8c03eae19ce73 100644
> --- a/Source/core/svg/animation/SVGSMILElement.cpp
> +++ b/Source/core/svg/animation/SVGSMILElement.cpp
> @@ -239,7 +239,7 @@ void SVGSMILElement::buildPendingResource()
>      }
>  }
>
> -static inline QualifiedName constructQualifiedName(const SVGElement*
> svgElement, const String& attributeName)
> +static inline QualifiedName constructQualifiedName(const SVGElement*
> svgElement, const AtomicString& attributeName)
>  {
>      ASSERT(svgElement);
>      if (attributeName.isEmpty())
> @@ -247,8 +247,8 @@ static inline QualifiedName
> constructQualifiedName(const SVGElement* svgElement,
>      if (!attributeName.contains(':'))
>          return QualifiedName(nullAtom, attributeName, nullAtom);
>
> -    String prefix;
> -    String localName;
> +    AtomicString prefix;
> +    AtomicString localName;
>      if (!Document::parseQualifiedName(attributeName, prefix, localName,
> ASSERT_NO_EXCEPTION))
>          return anyQName();
>
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-12-06 17:04:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/108483002/1

[commit-bot: I haz the power, 2013-12-06 18:01:05]

Change committed as 163355