Bug 98300 - OpaqueJSString doesn't optimally handle 8 bit strings
Summary: OpaqueJSString doesn't optimally handle 8 bit strings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 98417 98433 98693 98801 98805
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-03 14:16 PDT by Michael Saboff
Modified: 2022-02-27 23:24 PST (History)
1 user (show)

See Also:


Attachments
Patch (3.81 KB, patch)
2012-10-03 14:45 PDT, Michael Saboff
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2012-10-03 14:16:09 PDT
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.
Comment 1 Michael Saboff 2012-10-03 14:45:53 PDT
Created attachment 166963 [details]
Patch
Comment 2 Geoffrey Garen 2012-10-03 14:47:08 PDT
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.
Comment 3 Michael Saboff 2012-10-03 16:40:09 PDT
Committed r130344: <http://trac.webkit.org/changeset/130344>
Comment 4 Chris Dumez 2012-10-04 00:04:04 PDT
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
Comment 5 Chris Dumez 2012-10-04 00:15:52 PDT
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?
Comment 6 Michael Saboff 2012-10-04 09:17:02 PDT
(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.