Bug 55219

Summary: JNI code should include <jni.h> on non-OSX platforms.
Product: WebKit Reporter: Steve Block <steveblock>
Component: New BugsAssignee: Steve Block <steveblock>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, darin, eric, jorlow, steveblock, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 55383, 55387    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Steve Block 2011-02-25 07:52:27 PST
JNI code should include <jni.h> directly, rather than as <JavaVM/jni.h>
Comment 1 Steve Block 2011-02-25 07:53:08 PST
JavaVM/jni.h seems to be a valid path on Mac, but not on other platforms.
Comment 2 Steve Block 2011-02-25 07:56:40 PST
Created attachment 83815 [details]
Patch
Comment 3 Darin Adler 2011-02-25 08:11:14 PST
Is <jni.h> a valid path on Mac?
Comment 4 WebKit Review Bot 2011-02-25 10:50:31 PST
Attachment 83815 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8031380
Comment 5 Alexey Proskuryakov 2011-02-25 21:42:40 PST
Comment on attachment 83815 [details]
Patch

r- since this breaks Mac build.
Comment 6 Steve Block 2011-02-28 04:09:32 PST
Created attachment 84038 [details]
Patch
Comment 7 Steve Block 2011-02-28 04:10:17 PST
Comment on attachment 84038 [details]
Patch

Not yet ready for review, just checking that Mac build is OK.
Comment 8 WebKit Review Bot 2011-02-28 06:32:51 PST
Attachment 84038 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8076245
Comment 9 Steve Block 2011-02-28 07:02:57 PST
It seems that setting the include path manually for jni.h causes problems on OSX, as this is a frameworks header and needs some defines to be set up.

Instead I think it's best to use a conditional include based on the OS.
Comment 10 Steve Block 2011-02-28 07:03:29 PST
Created attachment 84054 [details]
Patch
Comment 11 Steve Block 2011-02-28 09:18:36 PST
Created attachment 84072 [details]
Patch
Comment 12 Jeremy Orlow 2011-02-28 15:27:06 PST
Comment on attachment 84072 [details]
Patch

r=me
Comment 13 WebKit Commit Bot 2011-02-28 16:59:00 PST
The commit-queue encountered the following flaky tests while processing attachment 84072 [details]:

media/invalid-media-url-crash.html bug 51138 (authors: inferno@chromium.org and jamesr@chromium.org)
The commit-queue is continuing to process your patch.
Comment 14 WebKit Commit Bot 2011-02-28 17:02:46 PST
Comment on attachment 84072 [details]
Patch

Clearing flags on attachment: 84072

Committed r79947: <http://trac.webkit.org/changeset/79947>
Comment 15 WebKit Commit Bot 2011-02-28 17:02:52 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 WebKit Review Bot 2011-02-28 18:08:04 PST
http://trac.webkit.org/changeset/79947 might have broken Qt Linux Release
The following tests are not passing:
fast/events/tabindex-focus-blur-all.html
fast/frames/iframe-plugin-load-remove-document-crash.html
fast/frames/sandboxed-iframe-attribute-parsing.html
fast/layers/clip-rects-transformed.html
fast/replaced/object-with-non-empty-classid-triggers-fallback.html
plugins/createScriptableObject-before-start.html
plugins/destroy-on-setwindow.html
plugins/destroy-plugin-from-callback.html
plugins/destroy-stream-twice.html
plugins/document-open.html
plugins/evaluate-js-after-removing-plugin-element.html
plugins/get-file-url.html
plugins/get-url-that-the-resource-load-delegate-will-disallow.html
plugins/get-url-with-javascript-destroying-plugin.html
plugins/geturl-replace-query.html
plugins/geturlnotify-during-document-teardown.html
plugins/instance-available-before-stylesheets-loaded-object.html
plugins/invalid-mime-with-valid-extension-shows-missing-plugin.html
plugins/js-from-destroy.html