Closed Bug 445391 Opened 12 years ago Closed 12 years ago
Re-enable OJI for Firefox 3
Given that there's unlikely to be a NPRuntime enabled Java plugin available for the Mac in time for our release of Firefox 3.1 we need to re-enable OJI for 3.1. Once 3.1 is out the door and a Java plugin that works w/o OJI exists for the Mac too we can disable it again and finally remove it from the tree, but for now we don't really have a choice other than to re-enable OJI.
This re-enables OJI :( The bulk of this change is to the JS engine due to its header files no longer compiling cleanly on windows when included from C code. Apparently inline is not recognized as a keyword in C when using devstudio (9), so this replaces two static inline functions with macros (yay!) and moves one into a .cpp file as it wasn't used anywhere else. I'm all ears if there's a better way to do this...
Comment on attachment 331246 [details] [diff] [review] Re-enable OJI by default. Igor, could you do the review here? Thanks, /be
Comment on attachment 331246 [details] [diff] [review] Re-enable OJI by default. >diff -r eb6abd26a711 configure.in >--- a/configure.in Thu Jul 24 15:11:08 2008 -0700 >+++ b/configure.in Thu Jul 24 18:07:11 2008 -0700 >@@ -4359,7 +4359,7 @@ > MOZ_NO_XPCOM_OBSOLETE= > MOZ_NO_FAST_LOAD= > MOZ_MEDIA= >-MOZ_OJI= >+MOZ_OJI=1 > MOZ_PERMISSIONS=1 > MOZ_PLACES= > MOZ_PLAINTEXT_EDITOR_ONLY= >@@ -5162,10 +5162,10 @@ > dnl ======================================================== > dnl = Open JVM Interface (OJI) support > dnl ======================================================== >-MOZ_ARG_ENABLE_BOOL(oji, >-[ --enable-oji Enable Open JVM Integration support], >- MOZ_OJI=1, >- MOZ_OJI=) >+MOZ_ARG_DISABLE_BOOL(oji, >+[ --disable-oji Disable Open JVM Integration support], >+ MOZ_OJI=, >+ MOZ_OJI=1) > if test -n "$MOZ_OJI"; then > AC_DEFINE(OJI) > fi >diff -r eb6abd26a711 js/src/jsfun.cpp >--- a/js/src/jsfun.cpp Thu Jul 24 15:11:08 2008 -0700 >+++ b/js/src/jsfun.cpp Thu Jul 24 18:07:11 2008 -0700 >@@ -2247,7 +2247,7 @@ > > js_ReportValueError3(cx, error, > (fp && fp->regs && >- StackBase(fp) <= vp && vp < fp->regs->sp) >+ STACK_BASE(fp) <= vp && vp < fp->regs->sp) > ? vp - fp->regs->sp > : (flags & JSV2F_SEARCH_STACK) > ? JSDVG_SEARCH_STACK >diff -r eb6abd26a711 js/src/jsinterp.cpp >--- a/js/src/jsinterp.cpp Thu Jul 24 15:11:08 2008 -0700 >+++ b/js/src/jsinterp.cpp Thu Jul 24 18:07:11 2008 -0700 >@@ -81,6 +81,18 @@ > > /* jsinvoke_cpp___ indicates inclusion from jsinvoke.cpp. */ > #if !JS_LONE_INTERPRET ^ defined jsinvoke_cpp___ >+ >+static inline uintN >+GlobalVarCount(JSStackFrame *fp) This is a wrong place for GlobalVarCount. This section on Linux and MAC does not compile in the same compilation unit as js_Interpret that uses the function. So this function should also be turned into a macro.
(In reply to comment #1) > Apparently inline is not recognized as a keyword in C when using devstudio (9). Alternatively we can define something like js_static_inline that under C++ and C99 compilers will be defined as #define js_static_inline static inline but for MSVC in C mode as #define js_static_inline static __inline Note also that SpiderMonkey defines JS_INLINE, but this is to ensure that one absolutely wants to inline the function. This is not necessary with the inline functions declared in the header since in a debug build it is nice to have them as functions to be able to step through them in the debugger.
This version uses __inline when MSVC is in C mode and __inline__ for GCC to support code that compiles with -ansi with GCC. The patch also allows to redefine JS_INLINE and JS_ALWAYS_INLINE macros for performance testing. Would it work on Windows?
The new version consistently uses defined XXX , not defined(XXX), in the preprocessor conditionals.
Comment on attachment 331292 [details] [diff] [review] using __inline with MSVC in C mode v1b Works wonderfully, awesome!. Bsmedberg, wanna glance over the configure.in changes here?
Attachment #331292 - Flags: superreview?(benjamin) → superreview+
The previous version of the patch has a redundant static specifier in JS_ALWAYS_INLINE definition with GCC. This version fixes this.
Comment on attachment 331529 [details] [diff] [review] using __inline with MSVC in C mode v2 As I mentioned in another bug: I like the idea of separating ALWAYS_INLINE and INLINE, so this patch makes me very happy.
Attachment #331529 - Flags: review?(crowder) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
This was a regression with the patch on bug 397080. Running the testcase from bug 444881 I cannot the any remaining issues with the Java Embedding Plugin 0.9.6.4 (MRJ Plugin version 1.0-JEP-0.9.6.4). Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080811020618 Minefield/3.1a2pre ID:20080811020618
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.