[webkit-reviews] review requested: [Bug 99363] StringImpl::reverseFind() with a single match character isn't optimal for mixed 8/16 bit cases : [Attachment 168832] Patch with review updates

[bugzilla-daemon at webkit.org]

Michael Saboff <msaboff at apple.com> has asked  for review:
Bug 99363: StringImpl::reverseFind() with a single match character isn't
optimal for mixed 8/16 bit cases
https://bugs.webkit.org/show_bug.cgi?id=99363

Attachment 168832: Patch with review updates
https://bugs.webkit.org/attachment.cgi?id=168832&action=review

------- Additional Comments from Michael Saboff <msaboff at apple.com>
(In reply to comment #5)
> (From update of attachment 168778 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=168778&action=review
> 
> > Source/WTF/wtf/text/StringImpl.cpp:1035
> > +	     UChar matchCharacter = matchString->is8Bit() ?
matchString->characters8()[0] : matchString->characters16()[0];
> 
> StringImpl's operator[] does that for you.

Fixed.	I think I added operator[].

> > Source/WTF/wtf/text/StringImpl.cpp:-1135
> > -	     if (is8Bit() && matchString->is8Bit())
> > -		 return WTF::reverseFind(characters8(), ourLength,
matchString->characters8()[0], index);
> > -	     return WTF::reverseFind(characters(), ourLength,
matchString->characters()[0], index);
> 
> This was intentional as the reverseFind(LChar*, UChar) was not implemented.
Then I forgot about it.
> 
> Does WTF::reverseFind() has this combination now? I don't see it in
StringImpl.h.

I updated reverseFind() to be like find().

> > Source/WTF/wtf/text/StringImpl.cpp:1141
> > +	     UChar matchCharacter = matchString->is8Bit() ?
matchString->characters8()[0] : matchString->characters16()[0];
> 
> see StringImpl's operator[].