Bug 33558

Summary: [Android] JavaString uses JSC-specific types
Product: WebKit Reporter: Steve Block <steveblock>
Component: WebCore Misc.Assignee: Steve Block <steveblock>
Severity: Normal CC: android-webkit-unforking, steveblock
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Android   
OS: Android   
Bug Depends on:    
Bug Blocks: 32154    
Description Flags
Patch 1 for Bug 33558
Patch 2 for Bug 33558 abarth: review+

Description Steve Block 2010-01-12 16:24:12 PST
JavaString, defined in WebCore/bridge/jni/jni_runtime.h, uses JSC-specific types. This causes problems on Android, where we can build with JSC or V8. See Bug 32154, which this bug blocks.

The code should re-factored to separate the JSC-specific internals from the script-independent code.
Comment 1 Steve Block 2010-01-12 16:42:44 PST
Created attachment 46412 [details]
Patch 1 for Bug 33558

This moves the JSC-specific implementation of JavaString to a private implementation class. A later change will add the V8 equivalent. Also modifies JavaField::name and JavaMethod::name to return const JavaString&, rather than UString::Rep*, which is JSC-specific.

Note that JavaString retains one JSC-specific method, operator UString. This is required for JSC only to allow UString::rep to be used to represent the JavaString. To avoid this anomaly, we could move the entire JavaString class to JSC and V8-specific files, but this means losing the sharing of code between the two.
Comment 2 Steve Block 2010-01-18 06:14:35 PST
Created attachment 46815 [details]
Patch 2 for Bug 33558

Corrects naming of JavaStringJSC.h
Comment 3 Adam Barth 2010-01-18 10:01:04 PST
Comment on attachment 46815 [details]
Patch 2 for Bug 33558

Ok.  I'm sad about the c-style casts, but I see that they were there in the original.  Also, there are style errors in the old code, but that's probably not worth worrying about at the moment.
Comment 4 Steve Block 2010-01-18 18:58:41 PST
Landed manually as http://trac.webkit.org/changeset/53449

Will fix style errors in existing code with a later patch

Closing bug as resolved