Last Comment Bug 517355 - Restore OJI, Liveconnect and the JEP on the 1.9.2 branch on OS X
: Restore OJI, Liveconnect and the JEP on the 1.9.2 branch on OS X
Status: VERIFIED FIXED
: verified1.9.2
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: 1.9.2 Branch
: All Mac OS X
: P1 normal with 1 vote (vote)
: mozilla1.9.2
Assigned To: Steven Michaud [:smichaud] (Retired)
:
:
Mentors:
Depends on: 523625 513979 521124 521135 521338 521599 521675 521818 523129 547025
Blocks: 485984 511884 519708 521624 535594
  Show dependency treegraph
 
Reported: 2009-09-17 16:35 PDT by Steven Michaud [:smichaud] (Retired)
Modified: 2010-03-11 06:18 PST (History)
27 users (show)
jaas: blocking1.9.2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed


Attachments
Patch v1.0, full (1.80 MB, application/gzip)
2009-09-17 16:51 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.0, without JEP (401.64 KB, application/gzip)
2009-09-17 17:01 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.0, changed files only (164.55 KB, patch)
2009-09-17 17:02 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Splinter Review
Patch v1.1, full (1.80 MB, application/gzip)
2009-09-29 12:54 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.1, without JEP (396.54 KB, application/gzip)
2009-09-29 12:55 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.1, changed files only (145.87 KB, patch)
2009-09-29 12:56 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Splinter Review
Patch v1.2, full (1.80 MB, application/gzip)
2009-10-01 09:35 PDT, Steven Michaud [:smichaud] (Retired)
jaas: review+
jst: review+
jwalden+bmo: review+
Details
Patch v1.2, without JEP (396.74 KB, application/gzip)
2009-10-01 09:36 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.2, changed files only (146.59 KB, patch)
2009-10-01 09:37 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Splinter Review
Patch v1.3, full (1.80 MB, application/gzip)
2009-10-06 21:05 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.3, changed files only (155.15 KB, patch)
2009-10-06 21:10 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Splinter Review
Gdb stack of js trace crash (11.43 KB, text/plain)
2009-10-06 21:14 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.4, full (1.80 MB, application/gzip)
2009-10-08 11:31 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Patch v1.4, changed files only (155.15 KB, patch)
2009-10-08 11:32 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Splinter Review

Description Steven Michaud [:smichaud] (Retired) 2009-09-17 16:35:46 PDT
This bug has been spun off from bug 510035.

I fear we're heading for a Java train wreck on OS X on the 1.9.2
branch (what will become Firefox 3.6).  We currently don't have a
usable Java plugin for the 1.9.2 branch (or the trunk) on OS X, and
none is likely to be available before next year -- some time between
January and March, 2010, or possibly even later.  This is well after
the planned release of Firefox 3.6 (currently scheduled for November
of this year).

There are really only two feasible ways of dealing with this problem:

1) Postpone the 3.6 release until after Apple releases its port of
   Sun's Java Plugin2.

2) Restore OJI, Liveconnect and the JEP on the 1.9.2 branch.

I have no control and little influence over whether or not we choose
option 1.  But I *can* show that option 2, though somewhat clumsy, is
feasible.  In my next comment I'll post a patch that restores OJI, the
JEP and Liveconnect on the 1.9.2 branch.

A developer preview of Apple's Java Plugin2 is actually present on OS
X 10.6.X, and on OS X 10.5.8 if you've installed a recent Java update
(Update 4 or Update 5).  But it's not release quality -- Apple
acknowledges this and doesn't provide a GUI for ordinary users to turn
it on.  "Not release quality" is in fact quite an understatement --
see bug 510035 comment #41.  There's no way we can tell people to use
that Java plugin in a Firefox release.
Comment 1 Steven Michaud [:smichaud] (Retired) 2009-09-17 16:51:19 PDT
Created attachment 401338 [details]
Patch v1.0, full

Here's a preliminary patch.  It's quite large, so I'm going to make it
available in three different copies -- the full patch, a full patch
without the JEP, and a patch containing only revisions to existing
files.

The full patch is the one you'd want to apply.  But the part
corresponding to the JEP is a bunch of base64-encoded files, some of
them quite large -- so this patch isn't exactly readable.

(If you don't want to bother building from the full patch, a tryserver
build should be available in a few hours.)

The full patch without the JEP contains many "new" files (those
containing the implementations of OJI and Liveconnect).  It contains
nothing but text files, so it's somewhat more readable.

But most readable of all is the copy of the patch that contains only
revisions to existing files.
Comment 2 Steven Michaud [:smichaud] (Retired) 2009-09-17 17:01:50 PDT
Created attachment 401340 [details]
Patch v1.0, without JEP
Comment 3 Steven Michaud [:smichaud] (Retired) 2009-09-17 17:02:50 PDT
Created attachment 401342 [details] [diff] [review]
Patch v1.0, changed files only
Comment 4 Steven Michaud [:smichaud] (Retired) 2009-09-17 17:17:35 PDT
I should say a little about my patch:

I didn't restore things to the way they were before OJI, Liveconnect
and the JEP were removed -- there have been too many changes in the
underlying code for that to be possible (for example lots of
interfaces have been changed or removed).  Intead I made existing
objects also (conditionally) support the old interfaces.

I wrapped all my changes with define macros, so that none of them have
any effect on other platforms than OS X.

Even on the Mac, I haven't made any changes in how ordinary NPAPI
plugins are handled.

My restoration of OJI and Liveconnect are only meant to support the
JEP, and have only been tested with the JEP (on OS X 10.5.8 and
10.6.1).  If other OJI plugins existed they might cause trouble
... but I don't believe any others do exist.
Comment 5 Steven Michaud [:smichaud] (Retired) 2009-09-17 17:47:57 PDT
Here's a tryserver build made with the full copy of my v1.0 patch:

https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-restoreOJI/restoreOJI-macosx.dmg

But it somehow doesn't bundle the JEP.  So (to test it) you'll need to download a copy of JEP 0.9.7.2 from http://javaplugin.sourceforge.net and copy that to /Library/Internet Plug-Ins.

I don't know why the JEP didn't get bundled -- possibly the tryserver doesn't like base64-encoded binary files in a patch.
Comment 6 Steven Michaud [:smichaud] (Retired) 2009-09-17 19:50:32 PDT
My tryserver build (based on my v1.0 patch) passed all tests (save for
a spurious "broken pipe" error on the Linux unit test machine).

My local build passed all tests locally, except for some reftest
failures that also happen without my patch (and so are presumably due
to problems with the tests).
Comment 7 Steven Michaud [:smichaud] (Retired) 2009-09-29 12:54:03 PDT
Created attachment 403558 [details]
Patch v1.1, full

Here's a new patch, with some minor changes.  Once again I'm going to
post three copies of it.  A tryserver build should follow in a few
hours.

The previous patch made *almost* no changes except on the Mac, with
one very small exception -- three "extern C" JavaScript methods were
explicitly exported on all platforms.  Now these methods are only
explicitly exported on the Mac (only when OJI is defined) (this is
needed by LiveConnect).

My v1.1 patch now refuses to load any XPCOM plugin but the Java
Embedding Plugin.  If a plugin fails to load as an NPAPI plugin, the
code now checks if it's a Java plugin, an XPCOM plugin (with an
NSGetFactory() entry point), and if the plugin's name contains "Java
Embedding Plugin".  Only if all three tests pass does the plugin get
loaded.

I've also made a few other changes to tighten things up.  For example
I now always try to load a plugin as an NPAPI plugin before I try
loading it as an OJI plugin (so a plugin that supports both APIs would
have a chance to be loaded as an NPAPI plugin).  And I've made a few
more of the "old" interfaces' methods return errors if no XPCOM plugin
has been loaded or instantiated.

There's no way to stop the "old" interfaces being present -- which
might confuse plugins/extensions that test for their presence.  But,
where possible, I've made these interfaces inoperable unless they're
being used by an XPCOM plugin (i.e. by the JEP, which is the only
XPCOM plugin that can get loaded/instantiated).
Comment 8 Steven Michaud [:smichaud] (Retired) 2009-09-29 12:55:19 PDT
Created attachment 403561 [details]
Patch v1.1, without JEP
Comment 9 Steven Michaud [:smichaud] (Retired) 2009-09-29 12:56:28 PDT
Created attachment 403563 [details] [diff] [review]
Patch v1.1, changed files only
Comment 10 Steven Michaud [:smichaud] (Retired) 2009-09-29 12:58:31 PDT
Comment on attachment 403558 [details]
Patch v1.1, full

Josh, I'm asking you to review this patch.  But if you think parts of
it should also be reviewed by other people, please add them.
Comment 11 Steven Michaud [:smichaud] (Retired) 2009-09-29 15:05:34 PDT
Here's a tryserver build made with my full v1.1 patch:

https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla517355-v1.1/bugzilla517355-v1.1-macosx.dmg

Once again it doesn't (for some reason) bundle the JEP.  So to test it
you'll you'll need to download JEP 0.9.7.2 from
http://javaplugin.sourceforge.net and copy its binaries
(JavaEmbeddingPlugin.bundle and MRJPlugin.plugin) to /Library/Internet
Plug-Ins (or to the Contents/MacOS/plugins directory of my tryserver
build's bundle).
Comment 12 Steven Michaud [:smichaud] (Retired) 2009-09-29 15:24:15 PDT
My v1.1 build passed all the tryserver tests except for two apparently
spurious failures on a Linux box ("Linux try hg unit test") and a
Windows box ("WINNT 5.2 try hg unit test").
Comment 13 Mike Beltzner [:beltzner, not reading bugmail] 2009-09-30 07:32:36 PDT
Need this for beta.
Comment 14 Steven Michaud [:smichaud] (Retired) 2009-09-30 09:49:56 PDT
This patch needs at least one more revision.

PPC Macs running OS X 10.4.X (and possibly also 10.5.X) have an
ancient Apple plugin (Java Applet.plugin) that supports Java 1.3.1.
As things now stand (with my current v1.1 patch), this plugin (where
present) gets used instead of the JEP.

I've opened bug 519734 to get Java Applet.plugin blocklisted on the
1.9.2 branch.  But I need to figure out why 1.9.2-branch FF now
prefers the Java Applet.plugin over the JEP (as earlier versions of
the browser never did).

I'll be working on this today and (probably) into tomorrow.
Comment 15 Smokey Ardisson (offline for a while; not following bugs - do not email) 2009-09-30 18:10:55 PDT
(In reply to comment #14)
> PPC Macs running OS X 10.4.X (and possibly also 10.5.X) have an
> ancient Apple plugin (Java Applet.plugin) that supports Java 1.3.1.
> As things now stand (with my current v1.1 patch), this plugin (where
> present) gets used instead of the JEP.

Isn't this because OJI & friends aren't there, so the part of the JEP distro that handles Java 1.4 and above (even if installed manually) "can't be loaded"?  That's the way I read bug 510035 comment 4, 9, etc., and specifically bug 510035 comment 12: 

> I'd forgotten that the MRJPlugin.plugin included with the JEP is able
> to use Java 1.3.1 (where present), and when doing so doesn't use the
> OJI API.

Or have you done subsequent tests on your tryserver builds with the JEP and OJI restored and are still seeing only Java 1.3.1 loaded; comment 14 isn't clear about that.
Comment 16 Steven Michaud [:smichaud] (Retired) 2009-09-30 19:06:22 PDT
> I'd forgotten that the MRJPlugin.plugin included with the JEP is
> able to use Java 1.3.1 (where present), and when doing so doesn't
> use the OJI API.

This comment of mine is (I now think) simply wrong.  Sorry :-(

The JEP's MRJPlugin.plugin *is* able to use Java 1.3.1 when
JavaEmbeddingPlugin.plugin isn't present.  But it still needs OJI.

Scott Field at bug 510035 was definitely running Apple's Java 1.3.1.
But I'm now pretty sure this was because he'd loaded Apple's Java
Applet.plugin -- i.e. he wasn't getting it via the JEP's
MRJPlugin.plugin.

> Or have you done subsequent tests on your tryserver builds with the
> JEP and OJI restored and are still seeing only Java 1.3.1 loaded;

Yes.

But there's more to the story.  This only happens (as I now realize)
with builds (made from my patch) that don't bundle the JEP.  In other
words, 1.9.2-branch Firefox (with my patch) only prefers Java
Applet.plugin over the JEP when both are installed to
/Library/Internet Plug-Ins/ (and no JEP exists in the distro's
Contents/MacOS/plugins directory).  This is because of a bug that's
been introduced on the 1.9.2 branch (and also the trunk) -- FF now
chooses the *older* of two plugins that support the same MIME type.

Tomorrow I'll have more to say about all this.
Comment 17 Steven Michaud [:smichaud] (Retired) 2009-09-30 19:10:53 PDT
> FF now chooses the *older* of two plugins that support the same MIME
> type.

And have been installed to the same location (e.g. both installed to
/Library/Internet Plug-Ins/, or both installed to the distro's
Contents/MacOS/plugins/).
Comment 18 Steven Michaud [:smichaud] (Retired) 2009-10-01 09:35:19 PDT
Created attachment 404057 [details]
Patch v1.2, full

Here's a new patch with just one change:

I discovered that 'make package' doesn't include the JEP in the
resulting distro.  (This is presumably the reason my previous patches'
tryserver builds didn't bundle the JEP.)

To fix this I needed to make a change to
browser/installer/package-manifest.in.

As I mentioned previously (comment #16), the problem I described in
comment #14 doesn't happen when the JEP is bundled with a 1.9.2-branch
distro (in its Contents/MacOS/plugins/ directory).  There's definitely
a bug on the trunk and 1.9.2 branch that makes the browser prefer
older plugins for a given MIME type, but it doesn't effect the JEP in
the default case (when it's bundled with FF).  So fixing this bug
doesn't depend on fixing the preference-order bug.

So I'll address the preference-order bug elsewhere -- in another bug.

I'm doing a tryserver build.  But the tryservers are severely backed
up, so it'll be quite a while before I can post a link here.
Comment 19 Steven Michaud [:smichaud] (Retired) 2009-10-01 09:36:22 PDT
Created attachment 404058 [details]
Patch v1.2, without JEP
Comment 20 Steven Michaud [:smichaud] (Retired) 2009-10-01 09:37:10 PDT
Created attachment 404059 [details] [diff] [review]
Patch v1.2, changed files only
Comment 21 Steven Michaud [:smichaud] (Retired) 2009-10-01 14:00:57 PDT
Here's a tryserver build made with my full v1.2 patch:

https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla517355-v1.2/bugzilla517355-v1.2-macosx.dmg

This one *does* bundle the JEP.

My v1.2 patch passed all the tryserver tests except for three
apparently spurious failures -- two timeouts (on "WINNT 5.2 try hg
unit test" and "OS X 10.5.2 try hg unit test") and a "connection ended
unexpectedly" (on "Linux try hg").
Comment 22 Marcia Knous [:marcia - use ni] 2009-10-01 14:29:16 PDT
Steven: Forgive my ignorance - I assume this tryserver build is supposed to show the JEP in about:plugins?

(In reply to comment #21)
> Here's a tryserver build made with my full v1.2 patch:
> 
> https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla517355-v1.2/bugzilla517355-v1.2-macosx.dmg
> 
> This one *does* bundle the JEP.
> 
> My v1.2 patch passed all the tryserver tests except for three
> apparently spurious failures -- two timeouts (on "WINNT 5.2 try hg
> unit test" and "OS X 10.5.2 try hg unit test") and a "connection ended
> unexpectedly" (on "Linux try hg").
Comment 23 Steven Michaud [:smichaud] (Retired) 2009-10-01 15:33:29 PDT
> I assume this tryserver build is supposed to show the JEP in
> about:plugins?

It should ... but now I see that it doesn't.

You *do* see JavaEmbeddingPlugin.bundle and MRJPlugin.plugin when you
right-click on the app bundle, choose "Show Package Contents" and
browse to Contents/MacOS/plugins.  But now I see that the tryserver
didn't "build" them properly -- these two bundles are missing all
their "binaries"!!!  This must (presumably) be because the tryservers
can't deal with base64-encoded binaries in patches.

'make package' works properly with my local build (made with my v1.2
patch) -- when you install the app bundle from the resulting *.dmg
file, it contains all the needed binaries and works correctly.

For now I guess you've got to test in the following way:

1) Remove JavaEmbeddingPlugin.bundle and MRJPlugin.plugin from the
   Contents/MacOS/plugins/ directory of the tryserver build you
   downloaded and installed.

2) Download JEP 0.9.7.2 from http://javaplugin.sourceforge.net/ and
   drag its binaries (JavaEmbeddingPlugin.bundle and MRJPlugin.plugin)
   to your tryserver download's Contents/MacOS/plugins/ directory.

(I missed the lost binaries problem because I already have a copy of
the JEP in my /Library/Internet Plug-Ins/ directory, and (on my
system) the tryserver download silently failed over to using that.)
Comment 24 Steven Michaud [:smichaud] (Retired) 2009-10-01 16:03:02 PDT
For what it's worth, here's a link to a build I made locally with 'make package':

http://people.mozilla.com/~stmichaud/bmo/firefox-517355-v1.2.dmg
Comment 25 Steven Michaud [:smichaud] (Retired) 2009-10-01 16:07:20 PDT
It's too bad the tryservers won't let you do "push to try" on the
1.9.2 branch (only on the trunk).  That's the only way I can think of
to get around the problem of the tryservers not being able to deal
with binaries in patches.
Comment 26 Steven Michaud [:smichaud] (Retired) 2009-10-01 16:20:21 PDT
> a bug that's been introduced on the 1.9.2 branch (and also the
> trunk) -- FF now chooses the *older* of two plugins that support the
> same MIME type.

I've opened bug 520085 to deal with this.
Comment 27 Josh Aas 2009-10-02 13:42:11 PDT
Comment on attachment 404057 [details]
Patch v1.2, full

I've looked over this and didn't see anything wrong but there is a lot here. The best thing to do is land it ASAP for testing but please get review from jst first.

To be clear since I'm marking this r+, this is an inherently dangerous patch. Steven did a good job writing it as far as I can tell, but this is a lot of code that nobody understood well even when it was in the tree the first time.
Comment 28 Johnny Stenback (:jst, jst@mozilla.com) 2009-10-06 13:36:18 PDT
Comment on attachment 404057 [details]
Patch v1.2, full

Looks good to me. I asked Waldo to glance over the JS engine API changes here as well.
Comment 29 Jeff Walden [:Waldo] (remove +bmo to email) 2009-10-06 14:06:43 PDT
#endif // __cplusplus

Speaking only for JS code, I'm sure we want these to be C-style /* */ comments, as they likely cause build warnings (might not even build on particularly pedantic compilers).

Line 298-ish of the new jsfun.cpp has this:

...
}
#ifdef OJI
JS_END_EXTERN_C
#endif

#ifdef OJI
JS_BEGIN_EXTERN_C
JS_EXPORT_API(void)
#else
void
#endif
js_PutArgsObject(JSContext *cx, JSStackFrame *fp)
...

The end/begin here is pointless, right?  Please get rid of it.

The comment /* Allow inclusion from LiveConnect C files */ (passim) should include a trailing period, throughout, because it's not a sentence fragment.

With those changes js/src looks fine.
Comment 30 Steven Michaud [:smichaud] (Retired) 2009-10-06 14:14:55 PDT
> The end/begin here is pointless, right?

What do you mean?  JS_BEGIN_EXTERN_C and JS_END_EXTERN_C?

Are you telling me that's pointless?  I don't know myself.
Comment 31 Jeff Walden [:Waldo] (remove +bmo to email) 2009-10-06 14:18:43 PDT
I believe it should be; those macros are just gunk that expands to

extern "C" {

and

}

if __cplusplus and nothing otherwise, so I think they can be omitted.  It'd only be necessary if there were something between the two, but there's only whitespace there after the preprocessor does its thing.
Comment 32 Steven Michaud [:smichaud] (Retired) 2009-10-06 14:20:23 PDT
Oh now I see (I think).

You're just saying I shouldn't have a JS_END_EXTERN_C just before a JS_BEGIN_EXTERN_C.  I should consolidate the two blocks.
Comment 33 Jeff Walden [:Waldo] (remove +bmo to email) 2009-10-06 14:28:08 PDT
What I'm saying is that this:

...
}
#ifdef OJI
JS_END_EXTERN_C
#endif

#ifdef OJI
JS_BEGIN_EXTERN_C
JS_EXPORT_API(void)
#else
void
#endif
js_PutArgsObject(JSContext *cx, JSStackFrame *fp)
...

expands to this in C++ with OJI:

...
}
}

extern "C" {
JS_EXPORT_API(void)
js_PutArgsObject(JSContext *cx, JSStackFrame *fp)
...

but since there's nothing between the } that closed the extern "C" started way above and the extern "C" that starts a new such block, there's no reason to close and reopen -- just do:

...
}

#ifdef OJI
JS_EXPORT_API(void)
#else
void
#endif
js_PutArgsObject(JSContext *cx, JSStackFrame *fp)
...
Comment 34 Steven Michaud [:smichaud] (Retired) 2009-10-06 14:40:57 PDT
I understand now.  Will do.
Comment 35 Steven Michaud [:smichaud] (Retired) 2009-10-06 21:05:18 PDT
Created attachment 404993 [details]
Patch v1.3, full

I've run into trouble.

There've been many changes to JS code since my v1.2 patch, and I've
had to make changes to JS header files (and sometimes to *.cpp) files
to get Liveconnect to compile and link properly.  But now I crash in a
JS trace stack every time I load a Java applet.

I don't *think* this is caused by anything in my patch (even the
newest one).  Rather I suspect there's been some kind of change to JS
tracing in the last week, and that's the source of the trouble.  But I
don't know a whole lot about JS code, so I'm asking for help from the
JS people.

Andreas Gal, your name popped up.  So I'm cc-ing you on this bug.

Thanks in advance for whatever suggestions you can come up with!

I'll post a stack trace of my crashes in a later comment.
Comment 36 Steven Michaud [:smichaud] (Retired) 2009-10-06 21:10:16 PDT
Created attachment 404994 [details] [diff] [review]
Patch v1.3, changed files only
Comment 37 Robert Sayre 2009-10-06 21:11:48 PDT
(In reply to comment #35)
> Created an attachment (id=404993) [details]
> Patch v1.3, full
> 
> I've run into trouble.

...

>  But now I crash in a
> JS trace stack every time I load a Java applet.

Attach the stacks here.
Comment 38 Steven Michaud [:smichaud] (Retired) 2009-10-06 21:14:23 PDT
Created attachment 404995 [details]
Gdb stack of js trace crash
Comment 39 Robert Sayre 2009-10-06 21:16:51 PDT
(In reply to comment #38)
> Created an attachment (id=404995) [details]
> Gdb stack of js trace crash

That's a GC bug, confusing also called "tracer". not related to the JIT.
Comment 40 Andreas Gal :gal 2009-10-06 21:29:00 PDT
  2688         else
  2689             JS_TraceChildren(trc, thing, kind);
  2690     } else {

Not sure how we end up at pc == 0x00000000 with that. Maybe that gets inlined?

  2416         /* If obj has no map, it must be a newborn. */
  2417         JSObject *obj = (JSObject *) thing;
  2418         if (!obj->map)
  2419             break;
  2420         obj->map->ops->trace(trc, obj);

Maybe ops->trace is NULL? A bunch of wrappers are on the call stack. Maybe one of them isn't implemented right?
  2421         break;
Comment 41 Steven Michaud [:smichaud] (Retired) 2009-10-07 08:13:51 PDT
I'll be using 'hg bisect' on this.  I should know more later today.
Comment 42 Damon Sicore (:damons) 2009-10-07 15:21:52 PDT
Steven, update here?
Comment 43 Steven Michaud [:smichaud] (Retired) 2009-10-07 15:33:49 PDT
I'm closing in on the guilty patch.  I should have it within an hour.

I have no idea how close I am to a fix, though.  I'll know more once I've identified the patch that triggered the crashes.
Comment 44 Steven Michaud [:smichaud] (Retired) 2009-10-07 15:53:41 PDT
'hg bisect' found the patch that triggers my crashes:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/042313efb6ac
"bug 511425 - removal of JSObjectOps.(get|set)RequiredSlot"
Igor Bukanov <igor@mir2.org>

I'll look through the patch for something more specific.  But I'm likely to need help from JS developers.
Comment 45 Mike Shaver (:shaver -- probably not reading bugmail closely) 2009-10-07 15:55:01 PDT
The first comment in bug 511425 makes me think that you should just back it out on 1.9.2 to start.
Comment 46 Andreas Gal :gal 2009-10-07 16:15:32 PDT
#45: thats the plan, mrbkap agrees too, working on it
Comment 47 Igor Bukanov 2009-10-08 05:09:03 PDT
(In reply to comment #45)
> The first comment in bug 511425 makes me think that you should just back it out
> on 1.9.2 to start.

This is not necessary, implementing JSObjectOps.trace in liveconnect is enough to fix this.
Comment 48 Blake Kaplan (:mrbkap) 2009-10-08 10:05:57 PDT
(In reply to comment #47)
> This is not necessary, implementing JSObjectOps.trace in liveconnect is enough
> to fix this.

Beat you to that one: bug 521135 has that patch.
Comment 49 Andreas Gal :gal 2009-10-08 10:45:08 PDT
Its not clear that that is enough though. We are crashing in the cycle collector now.
Comment 50 Steven Michaud [:smichaud] (Retired) 2009-10-08 11:31:38 PDT
Created attachment 405296 [details]
Patch v1.4, full

Here's a new revision to my patch, which updates it to current code.

It still crashes, of course -- so I won't do a tryserver build.

But here's a user repository with the patch on top of it:
http://hg.mozilla.org/users/smichaud_pobox.com/bugzilla517355/
Comment 51 Steven Michaud [:smichaud] (Retired) 2009-10-08 11:32:32 PDT
Created attachment 405297 [details] [diff] [review]
Patch v1.4, changed files only
Comment 52 Steven Michaud [:smichaud] (Retired) 2009-10-08 16:44:53 PDT
I've spun off bug 521338 to follow gal's and mrbkap's current work on fixing the crashes.  mrbkap has been adding patches to my user repository (http://hg.mozilla.org/users/smichaud_pobox.com/bugzilla517355/).
Comment 53 Steven Michaud [:smichaud] (Retired) 2009-10-09 11:15:29 PDT
After a couple hours of testing, I've concluded that the patches at
bug 521338 completely fix the problems triggered by the patch for bug
511425 (the crash I reported at comment #38, and another crash
reported at bug 521338 comment #1).

The patches at bug 521338 still need to be reviewed.  But once that
happens I think they can land on the 1.9.2 branch, either on top of or
in combination with my patch for this bug.

(My v1.4 patch will once again need to be updated to current code, but
that shouldn't be difficult -- there haven't been many changes since I
posted it.)
Comment 54 Steven Michaud [:smichaud] (Retired) 2009-10-09 13:40:55 PDT
Landed on the 1.9.2 branch, together with mrbkap's patch for bug
521338:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/3861fd3d6671
Comment 55 Steven Michaud [:smichaud] (Retired) 2009-10-09 16:14:20 PDT
Dbaron just pointed out to me that some OJI/Liveconnect-related leaks
were triggered by this landing:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6-Unittest/1255123815.1255126224.11988.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6-Unittest/1255124353.1255126125.10906.gz

I'll try to figure out what's going on next week (starting Monday).
Comment 56 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2009-10-10 10:15:38 PDT
Steven, do you have a collection of web pages which use Java Applets and we can use for testing?
Comment 57 Steven Michaud [:smichaud] (Retired) 2009-10-10 10:47:56 PDT
Probably the best place to start is bug 371084 comment #3.  Four or
five of those applets have by now disappeared from the web, but the
rest give you quite a bit to work with.

Also look at Sun's demo applets at
http://java.sun.com/applets/jdk/1.4/index.html.  The ones I use most
often are the Clock (of course), plus ArcTest, ImageMap and Molecule
Viewer's "example3".

ImageMap is currently partially broken (in Namoroka after my patch) --
clicking on the developer's face doesn't take you to
http://java.sun.com/.  This may be a bug in the JEP, which I'll be
working on as I have the time.

Clicking on the developer's face?  Don't ask me, I didn't write it :-)
Comment 58 Blake Kaplan (:mrbkap) 2009-10-10 12:46:11 PDT
(In reply to comment #55)
> Dbaron just pointed out to me that some OJI/Liveconnect-related leaks
> were triggered by this landing:

Is there a bug filed on the leak?
Comment 59 Steven Michaud [:smichaud] (Retired) 2009-10-10 14:20:33 PDT
Now there is -- bug 521599.
Comment 60 Marcia Knous [:marcia - use ni] 2009-10-13 14:18:31 PDT
Verified fixed on the 1.9.2 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2b1pre) Gecko/20091013 Namoroka/3.6b1pre as well as the equivalent build on 10.5 and 10.4.
Comment 61 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2009-10-14 07:51:29 PDT
This bug is 1.9.2 only so switching to verified fixed too.
Comment 62 Steven Michaud [:smichaud] (Retired) 2009-10-19 10:05:55 PDT
(Following up comment #57)

> ImageMap is currently partially broken (in Namoroka after my patch)
> -- clicking on the developer's face doesn't take you to
> http://java.sun.com/.  This may be a bug in the JEP, which I'll be
> working on as I have the time.

I've found out this is a Firefox bug, and I've opened bug 523129.

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