Bug 174501 – [WTF] Newly added AtomicStringImpl should use BufferInternal static string if StringImpl is static

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[WTF] Newly added AtomicStringImpl should use BufferInternal static string if StringImpl::requiresCopy returns false

[Yusuke Suzuki]

Created attachment 315418 [details]
Patch

[Build Bot]

Comment on attachment 315418 [details]
Patch

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

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 315422 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Yusuke Suzuki]

Created attachment 315423 [details]
Patch

[Darin Adler]

Comment on attachment 315423 [details]
Patch

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

> Source/WTF/wtf/text/AtomicStringImpl.cpp:128
>      const CharacterType* s;

Seems like this should be named "characters" or "string" rather than a single letter. Worth coming back to and cleaning up some day.

[Yusuke Suzuki]

Comment on attachment 315423 [details]
Patch

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

>> Source/WTF/wtf/text/AtomicStringImpl.cpp:128
>>      const CharacterType* s;
> 
> Seems like this should be named "characters" or "string" rather than a single letter. Worth coming back to and cleaning up some day.

Yeah, "characters" sounds fine.

[Yusuke Suzuki]

Created attachment 315434 [details]
Patch for landing

[Build Bot]

Comment on attachment 315423 [details]
Patch

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

New failing tests:
storage/indexeddb/modern/new-database-after-user-delete.html

[Build Bot]

Created attachment 315439 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Yusuke Suzuki]

Committed r219510: <http://trac.webkit.org/changeset/219510>

[Matt Lewis]

This is causing an api failure on iOS.

https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Debug%20WK2%20%28Tests%29/builds/2725/steps/run-api-tests/logs/stdio

Tests that failed:
  WTF.StringImplNullSymbolToAtomicString

[Yusuke Suzuki]

Comment on attachment 315434 [details]
Patch for landing

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

> Source/WTF/wtf/text/AtomicStringImpl.cpp:423
> +    ASSERT_WITH_MESSAGE(string.length(), "Empty string should be already handled.");

There is one more length() == 0 string: null symbol! We need to keep this lines.
After rolling out my patch, I'll revert this part partially and reland it.

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #12)
> Comment on attachment 315434 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=315434&action=review
> 
> > Source/WTF/wtf/text/AtomicStringImpl.cpp:423
> > +    ASSERT_WITH_MESSAGE(string.length(), "Empty string should be already handled.");
> 
> There is one more length() == 0 string: null symbol! We need to keep this
> lines.
> After rolling out my patch, I'll revert this part partially and reland it.

With assertion and comments.

[Yusuke Suzuki]

Created attachment 315543 [details]
Patch for landing after stabilizing is done

Patch for landing

[Yusuke Suzuki]

Committed r219725: <http://trac.webkit.org/changeset/219725>