Bug 55766

Summary: JavaField should not expose JavaString in its API
Product: WebKit Reporter: Steve Block <steveblock>
Component: WebCore Misc.Assignee: Steve Block <steveblock>
Status: RESOLVED FIXED    
Severity: Normal CC: andreip, jorlow, steveblock
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Other   
Bug Depends on:    
Bug Blocks: 55383    
Attachments:
Description Flags
Patch 1 jorlow: review+

Steve Block
Reported 2011-03-04 04:32:07 PST
JavaField, which is used by the Java bridge to represent a field of a Java class, uses JavaString for the return value of its name() method. JavaString is a utility class intended to convert from the JNI string type jstring to WTF::String and char*. There's no need for JavaField to expose this utility type in its API. Instead it should just expose WTF::String, using JavaString internally where appropriate. See also Bug 55765.
Attachments
Patch 1 (13.59 KB, patch)
2011-03-30 06:40 PDT, Steve Block 		jorlow: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Steve Block
Comment 1 2011-03-30 06:37:15 PDT
Note that V8 and JSC use different implementations of JavaField. This bug tracks fixing V8's implementation only.
Steve Block
Comment 2 2011-03-30 06:40:09 PDT
Created attachment 87523 [details] Patch 1
Andrei Popescu
Comment 3 2011-03-30 11:53:54 PDT
LGTM > Source/WebCore/bridge/jni/v8/JavaClassV8.cpp:56 > } Why the { } ? > Source/WebCore/bridge/jni/v8/JavaFieldJobjectV8.h:45 > + virtual const char* typeClassName() const { return m_typeClassName.utf8(); } I wish we didn't have this inconsistency: one method returns a String, the next returns a char*...
Jeremy Orlow
Comment 4 2011-03-30 13:17:11 PDT
Comment on attachment 87523 [details] Patch 1 rubber stamp cause of andreip's LGTM
Steve Block
Comment 5 2011-03-31 02:46:30 PDT
> Why the { } ? Looks like an oversight from when this class was copied from the JSC version, which uses these brackets to scope a JSLock. Will remove on landing. > I wish we didn't have this inconsistency: one method returns a String, the next returns a char*... Agreed. Tracking in Bug 57443 and will fix.
Steve Block
Comment 6 2011-03-31 03:05:10 PDT
Committed r82554: <http://trac.webkit.org/changeset/82554>
Note You need to log in before you can comment on or make changes to this bug.