Bug 140139

Summary: Investigate the character type of repeated string instead of checking is8Bit flag
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Yusuke Suzuki
Reported 2015-01-06 11:53:54 PST
Investigate the character type of repeated string instead of checking is8Bit flag
Attachments
Patch (3.02 KB, patch)
2015-01-06 11:57 PST, Yusuke Suzuki
no flags
Patch (3.02 KB, patch)
2015-01-06 12:07 PST, Yusuke Suzuki
no flags
Patch (2.95 KB, patch)
2015-01-07 06:34 PST, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2015-01-06 11:57:59 PST
Yusuke Suzuki
Comment 2 2015-01-06 12:02:39 PST
Comment on attachment 244082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244082&action=review In this patch, I've fixed the issue pointed at [1]. 1: https://bugs.webkit.org/show_bug.cgi?id=140047#c22 > Source/JavaScriptCore/runtime/StringPrototype.cpp:679 > +static inline JSValue repeatCharacter(ExecState* exec, CharacterType character, unsigned repeatCount) Change name to `repeatCharacter`. > Source/JavaScriptCore/runtime/StringPrototype.cpp:725 > + if (!(character & ~0xff)) Checking character is 8bit or not. This check is the same to the guart which is used in the StringImpl / StringBuiler[1]. 1: http://trac.webkit.org/browser/trunk/Source/WTF/wtf/text/StringImpl.h#L1135
Yusuke Suzuki
Comment 3 2015-01-06 12:07:51 PST
Darin Adler
Comment 4 2015-01-06 14:12:01 PST
Comment on attachment 244087 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244087&action=review > Source/JavaScriptCore/runtime/StringPrototype.cpp:727 > + return JSValue::encode(repeatCharacter<LChar>(exec, static_cast<LChar>(character), repeatCount)); > + return JSValue::encode(repeatCharacter<UChar>(exec, character, repeatCount)); I don’t think the <LChar> and <UChar> are needed after repeatCharacter; I think template type deduction will take care of this.
Yusuke Suzuki
Comment 5 2015-01-07 06:33:06 PST
Comment on attachment 244087 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244087&action=review Thanks for your review :) >> Source/JavaScriptCore/runtime/StringPrototype.cpp:727 >> + return JSValue::encode(repeatCharacter<UChar>(exec, character, repeatCount)); > > I don’t think the <LChar> and <UChar> are needed after repeatCharacter; I think template type deduction will take care of this. Agreed. `static_cast` ensures the instantiated result of the template function. I'll fix it and update the patch.
Yusuke Suzuki
Comment 6 2015-01-07 06:34:24 PST
WebKit Commit Bot
Comment 7 2015-01-07 21:09:27 PST
Comment on attachment 244154 [details] Patch Clearing flags on attachment: 244154 Committed r178098: <http://trac.webkit.org/changeset/178098>
WebKit Commit Bot
Comment 8 2015-01-07 21:09:30 PST
All reviewed patches have been landed. Closing bug.
Yusuke Suzuki
Comment 9 2015-01-08 08:33:20 PST
Thank you for your r+ and cq+ :D
Note You need to log in before you can comment on or make changes to this bug.