Bug 65175 – Fix AtomicString vs. String usage in XPath code

• Status: NEW
• Component: WebCore Misc.

[Patrick R. Gansterer]

Fix AtomicString vs. String usage in XPath code

[Patrick R. Gansterer]

Created attachment 101989 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/xml/XPathStep.h:61:  Missing space inside { }.  [whitespace/braces] [5]
Source/WebCore/xml/XPathStep.h:62:  Missing space inside { }.  [whitespace/braces] [5]
Total errors found: 2 in 15 files


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

[Gyuyoung Kim]

Comment on attachment 101989 [details]
Patch

Attachment 101989 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9249423

[Collabora GTK+ EWS bot]

Comment on attachment 101989 [details]
Patch

Attachment 101989 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9249424

[Early Warning System Bot]

Comment on attachment 101989 [details]
Patch

Attachment 101989 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9244428

[WebKit Review Bot]

Comment on attachment 101989 [details]
Patch

Attachment 101989 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9245430

[WebKit Review Bot]

Comment on attachment 101989 [details]
Patch

Attachment 101989 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9249430

[Patrick R. Gansterer]

Comment on attachment 101989 [details]
Patch

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

> Source/WebCore/xml/NativeXPathNSResolver.cpp:53
> +    return m_node ? m_node->lookupNamespaceURI(prefix) : nullAtom;

The build break is caused by this line. This change depends on an other patch, which changes the return value of lookupNamespaceURI to AtomicString too.
Please ignore the build break on the EWS for now.

[WebKit Review Bot]

Comment on attachment 101989 [details]
Patch

Attachment 101989 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9248436

[Alexey Proskuryakov]

Comment on attachment 101989 [details]
Patch

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

I see that the patch adds a number of String->AtomicString conversions, but I don't see which ones are removed. Could you please explain that in ChangeLog, for the benefit of lazy reviewers and everyone else reading patches?

> Source/WebCore/ChangeLog:8
> +        Use AtomicString instead of String to avoid unnecessary type conversations.

conversations - > conversions.

[Brent Fulgham]

Could you link the bug ID with the patch that is required before this one can build properly?

[Eric Seidel (no email)]

Comment on attachment 101989 [details]
Patch

We should do this in smaller pieces, or with more explanation in the ChangeLog.