Bug 210049 - ProcessAssertion should use ASCIILiteral for its reason
Summary: ProcessAssertion should use ASCIILiteral for its reason
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 209990
  Show dependency treegraph
 
Reported: 2020-04-06 08:27 PDT by Chris Dumez
Modified: 2020-04-06 11:41 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.32 KB, patch)
2020-04-06 08:29 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2020-04-06 08:27:32 PDT
ProcessAssertion should use ASCIILiteral for its reason, instead of a String.
Comment 1 Chris Dumez 2020-04-06 08:29:40 PDT
Created attachment 395568 [details]
Patch
Comment 2 Alex Christensen 2020-04-06 09:45:30 PDT
Why?
Comment 3 Chris Dumez 2020-04-06 09:47:48 PDT
(In reply to Alex Christensen from comment #2)
> Why?

https://bugs.webkit.org/show_bug.cgi?id=209984#c3
Comment 4 Alex Christensen 2020-04-06 10:17:19 PDT
Comment on attachment 395568 [details]
Patch

This seems silly to me.  The API takes an NSString.  Using stringWithCString and NSASCIIStringEncoding doesn't improve anything.  This is just documenting that all the strings we use happen to be ASCII right now.
Comment 5 Darin Adler 2020-04-06 10:31:35 PDT
(In reply to Alex Christensen from comment #4)
> This seems silly to me.  The API takes an NSString.  Using stringWithCString
> and NSASCIIStringEncoding doesn't improve anything.  This is just
> documenting that all the strings we use happen to be ASCII right now.

Seems great to me. Why turn literals into WTF::String just to turn them into NSString later? These are labels from the code, not arbitrary strings.
Comment 6 EWS 2020-04-06 11:07:31 PDT
Committed r259579: <https://trac.webkit.org/changeset/259579>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395568 [details].
Comment 7 Radar WebKit Bug Importer 2020-04-06 11:08:17 PDT
<rdar://problem/61350171>
Comment 8 Alex Christensen 2020-04-06 11:41:10 PDT
I guess that's true.  This reduces a few WTF::String constructions.