Bug 98300 – OpaqueJSString doesn't optimally handle 8 bit strings

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

OpaqueJSString's can be created with a String or with a UChar*, length pair.  The String version of create() will up convert an 8 bit string.

[Michael Saboff]

Created attachment 166963 [details]
Patch

[Geoffrey Garen]

Comment on attachment 166963 [details]
Patch

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

r=me

> Source/JavaScriptCore/API/OpaqueJSString.h:67
> +        // Make a copy fo the source string

Typo: should be "of". Please put a period at the end of the comment sentence.

[Michael Saboff]

Committed r130344: <http://trac.webkit.org/changeset/130344>

[Chris Dumez]

Hi,

This patch seems to have caused a behavior change for the following code:

JSRetainPtr<JSStringRef> response;
response.adopt(JSStringCreateWithUTF8CString(0));
String responseString = response->string();
responseString.isNull() // Used to return true, now returns false

[Chris Dumez]

Comment on attachment 166963 [details]
Patch

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

> Source/JavaScriptCore/API/OpaqueJSString.h:62
> +        m_string = emptyString();

Why this? Shouldn't m_string stay null here instead of assigning an empty string to it?

[Michael Saboff]

(In reply to comment #5)
> (From update of attachment 166963 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166963&action=review
> 
> > Source/JavaScriptCore/API/OpaqueJSString.h:62
> > +        m_string = emptyString();
> 
> Why this? Shouldn't m_string stay null here instead of assigning an empty string to it?

That change is likely the cause of your problem.  I filed https://bugs.webkit.org/show_bug.cgi?id=98417 "After r130344, OpaqueJSString() creates a empty string which should be a null string".  Will fix this today.