Bug 38525

Summary:

JavaInstanceJSC.cpp and JNIUtilityPrivate.cpp need to include jni_jsobject.h for jlong_to_pt() and ptr_to_jlong()

Product:

WebKit

Reporter:

Steve Block <steveblock>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

android-webkit-unforking, ap, darin, steveblock

Priority:

Version:

528+ (Nightly build)

Hardware:

Android

OS:

Android

Attachments:

Description	Flags
Patch	none
Patch	none

Steve Block

Reported 2010-05-04 08:56:44 PDT

Change http://trac.webkit.org/changeset/55250 added code to JavaInstanceJSC.cpp and JNIUtilityPrivate.cpp which uses jlong_to_pt() and ptr_to_jlong(). These methods are provided in jni_jsobject.h but this header is not included directly. This causes build errors on Android. Furthermore, jni_jsobject.h includes Mac-specific code which is not guarded by PLATFORM(MAC).

Attachments
Patch (2.48 KB, patch) 2010-05-04 10:04 PDT, Steve Block	no flags	Details Formatted Diff Diff
Patch (2.43 KB, patch) 2010-05-04 13:54 PDT, Steve Block	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Steve Block

Comment 1 2010-05-04 10:04:11 PDT

Created attachment 55024 [details] Patch

Darin Adler

Comment 2 2010-05-04 10:10:34 PDT

Comment on attachment 55024 [details] Patch > + * bridge/jni/jni_jsobject.h: Guard Mac-specific code with PLATFORM(MAC) Why are you making this change? The entire file is inside #if ENABLE(MAC_JAVA_BRIDGE).

Steve Block

Comment 3 2010-05-04 10:16:39 PDT

> Why are you making this change? The entire file is inside #if > ENABLE(MAC_JAVA_BRIDGE). Android also defines MAC_JAVA_BRIDGE, which is used to protect all parts of the Java bridge, not just those parts specific to Mac. (The guard is probably poorly named. I can open a separate bug to fix that?) Without the additional PLATFORM(MAC) guard, the build fails on Android.

Darin Adler

Comment 4 2010-05-04 10:25:22 PDT

Comment on attachment 55024 [details] Patch > #define jlong_to_impptr(a) (static_cast<JSC::JSObject*>(((void*)(uintptr_t)(a)))) > #define ptr_to_jlong(a) ((jlong)(uintptr_t)(a)) > > +#if PLATFORM(MAC) > + > +#include <CoreFoundation/CoreFoundation.h> Normally in WebKit, conditional includes go up at the top of the file with the other includes, not down below things like macros. I also think this include is unneeded. r=me

Steve Block

Comment 5 2010-05-04 13:48:56 PDT

I've filed Bug 38544 to track the renaming of MAC_JAVA_BRIDGE.

Steve Block

Comment 6 2010-05-04 13:54:14 PDT

Created attachment 55043 [details] Patch

Steve Block

Comment 7 2010-05-04 13:55:36 PDT

> I also think this include is unneeded. I think you're right. I've uploaded a new patch without this include to check the build on the buildbots.

Steve Block

Comment 8 2010-05-04 14:17:27 PDT

Builders look OK. Landed manually as http://trac.webkit.org/changeset/58775 Closing bug as resolved fixed.

Note You need to log in before you can comment on or make changes to this bug.