Bug 95888 – More fixes for String::operator+=() in Debug mode

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Patrick R. Gansterer]

More fixes for String::operator+=() in Debug mode

[Patrick R. Gansterer]

Created attachment 162325 [details]
Patch

[Patrick R. Gansterer]

Comment on attachment 162325 [details]
Patch

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

> Source/WebCore/editing/VisibleSelection.cpp:678
>          result = "<none>";

won't work

[Adam Barth]

Comment on attachment 162325 [details]
Patch

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

> Source/WebCore/dom/Element.cpp:1421
> +    strncpy(buffer, result.toString().utf8().data(), length - 1);

We should explicitly NUL terminate buffer in case we hit the length limit, but that's something for another patch.

> Source/WebCore/dom/Node.cpp:2105
> +        if (!attr.isEmpty())
> +            string.append(attrDesc + attr);

It's more efficient to do this in two operations.  Having the + as a separate operation creates an intermediate buffer that's not needed.

> Source/WebCore/dom/Node.cpp:2198
> -                traverseTreeAndMark(indent + "\t", youngerShadowRoot, markedNode1, markedLabel1, markedNode2, markedLabel2);
> +                traverseTreeAndMark(indent.toString() + '\t', youngerShadowRoot, markedNode1, markedLabel1, markedNode2, markedLabel2);

It's more efficient to append the '\t' and then call toString.

> Source/WebCore/dom/Node.cpp:2200
> -            traverseTreeAndMark(indent + "\t", oldestShadowRoot, markedNode1, markedLabel1, markedNode2, markedLabel2);
> +            traverseTreeAndMark(indent.toString() + '\t', oldestShadowRoot, markedNode1, markedLabel1, markedNode2, markedLabel2);

ditto

> Source/WebCore/dom/Node.cpp:2222
> +    if (s.length())

if (!s.isEmpty())

[Patrick R. Gansterer]

Created attachment 162329 [details]
Patch

[Patrick R. Gansterer]

Comment on attachment 162329 [details]
Patch

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

> Source/WebCore/dom/Node.cpp:2105
> +            string.append(attrDesc + attr);

string is an out-parameter
are two String::append() calls better than one append and operator+()?

[Adam Barth]

Comment on attachment 162329 [details]
Patch

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

>> Source/WebCore/dom/Node.cpp:2105
>> +            string.append(attrDesc + attr);
> 
> string is an out-parameter
> are two String::append() calls better than one append and operator+()?

Can we change the outparameter type to StringBuilder?

[Patrick R. Gansterer]

Created attachment 162333 [details]
Patch

[Adam Barth]

Comment on attachment 162333 [details]
Patch

Clearing flags on attachment: 162333

Committed r127706: <http://trac.webkit.org/changeset/127706>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

*** Bug 95945 has been marked as a duplicate of this bug. ***