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

Steve Block
Reported 2011-02-25 07:52:27 PST
JNI code should include <jni.h> directly, rather than as <JavaVM/jni.h>
Attachments
Patch (3.03 KB, patch)
2011-02-25 07:56 PST, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (5.47 KB, patch)
2011-02-28 04:09 PST, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (3.34 KB, patch)
2011-02-28 07:03 PST, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (4.97 KB, patch)
2011-02-28 09:18 PST, Steve Block 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Steve Block
Comment 1 2011-02-25 07:53:08 PST
JavaVM/jni.h seems to be a valid path on Mac, but not on other platforms.
Steve Block
Comment 2 2011-02-25 07:56:40 PST
Created attachment 83815 [details] Patch
Darin Adler
Comment 3 2011-02-25 08:11:14 PST
Is <jni.h> a valid path on Mac?
WebKit Review Bot
Comment 4 2011-02-25 10:50:31 PST
Attachment 83815 [details] did not build on mac: Build output: http://queues.webkit.org/results/8031380
Alexey Proskuryakov
Comment 5 2011-02-25 21:42:40 PST
Comment on attachment 83815 [details] Patch r- since this breaks Mac build.
Steve Block
Comment 6 2011-02-28 04:09:32 PST
Created attachment 84038 [details] Patch
Steve Block
Comment 7 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.
WebKit Review Bot
Comment 8 2011-02-28 06:32:51 PST
Attachment 84038 [details] did not build on mac: Build output: http://queues.webkit.org/results/8076245
Steve Block
Comment 9 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.
Steve Block
Comment 10 2011-02-28 07:03:29 PST
Created attachment 84054 [details] Patch
Steve Block
Comment 11 2011-02-28 09:18:36 PST
Created attachment 84072 [details] Patch
Jeremy Orlow
Comment 12 2011-02-28 15:27:06 PST
Comment on attachment 84072 [details] Patch r=me
WebKit Commit Bot
Comment 13 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.
WebKit Commit Bot
Comment 14 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>
WebKit Commit Bot
Comment 15 2011-02-28 17:02:52 PST
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 16 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
Note You need to log in before you can comment on or make changes to this bug.