Bug 132796 – QualifiedName should use RefPtr<QualifiedNameImpl> internally.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Andreas Kling]

QualifiedName should use RefPtr<QualifiedNameImpl> internally. This'll be simpler and we can make ~QualifiedName() inline.

[Andreas Kling]

Created attachment 231250 [details]
Patch

[WebKit Commit Bot]

Attachment 231250 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/QualifiedName.h:74:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/QualifiedName.h:81:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 231250 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231250&action=review

These are the Windows build errors:

     1>..\dom\QualifiedName.cpp(100): error C2734: 'WebCore::anyName' : const object must be initialized if not extern
     1>..\dom\QualifiedName.cpp(100): warning C4211: nonstandard extension used : redefined extern to static
     1>..\dom\QualifiedName.cpp(100): error C2512: 'WebCore::QualifiedName' : no appropriate default constructor available

Looks good as long as you resolve that before landing. I think QNAME_DEFAULT_CONSTRUCTOR was doing some good on Windows.

> Source/WebCore/dom/QualifiedName.cpp:81
>      QualifiedNameComponents components = { p.impl(), l.impl(), n.isEmpty() ? nullAtom.impl() : n.impl() };

Isn’t nullAtom.impl() the same thing as nullptr?

> Source/WebCore/dom/QualifiedName.cpp:83
> +    m_impl = addResult.isNewEntry ? adoptRef(*addResult.iterator) : *addResult.iterator;

Can we make an inline helper function to do this work? Then we can use construction instead of assignment for m_impl, and possibly even save a branch.

I also think we could go nuts and use words for the three arguments instead of letters.

> Source/WebCore/dom/QualifiedName.h:74
> +    explicit QualifiedName(WTF::HashTableDeletedValueType) : m_impl(WTF::HashTableDeletedValue) { }

Are the explicit WTF prefixes needed here? Fine to leave them in if they are indeed needed.

> Source/WebCore/dom/QualifiedName.h:76
> +    ~QualifiedName() { }

I believe the best way to get this effect, a public empty destructor that gets inlined, is to omit explicit declaration or definition of the destructor entirely.

> Source/WebCore/dom/QualifiedName.h:78
>      QualifiedName() : m_impl(0) { }

Maybe nullptr this while you are in the neighborhood.

[Andreas Kling]

Committed r170816: <http://trac.webkit.org/changeset/170816>