http://lxr.mozilla.org/seamonkey/source/modules/oji/public/nsIJVMPluginInstance. h#54 There is some need for this interface to be defined in xpidl so that it can make use of the thread proxying stuff I wrote. Steve.Katz@east.Sun.COM can fill in the requirement details.

All interfaces that might be used by a plugin or OJI plugin to request a service of the browser asyncronously should be defined in such a way that the proxying stuff can be used.

Reassign.

Steve, Would you please fill in the requirements for this request? Thanks.

The requierments are that plugins and OJI plugins be provided with some way to cause the main browser thread to execute code on its [the plugins] behalf (since the browser (at least the UI) is not thread safe. It would appear that extending the nsISupports proxy system to include those interfaces that a plugin or OJI plugin might call is the "correct" way. Beyond that, I have no idea what you are asking me for.

The reason I asked you to fill in the requirements is two-fold: 1) At the beginning of the comments above, there is a line saying, "Steve.Katz@east.Sun.COM can fill in the requirement details". I suppose the lines above are "the requirement details". 2) Just want to touch base with you to be clear what you want to avoid supprises later.

When do you need this fix, Steve?

Joe, Since it means that there is no way a plugin can request a service of the browser (without implementing some kind of hack), this should be considered a very high priority. Note also that it is not just the Jave Plugin that will have this problem, but all plugins.

I have tested this on Win32 and it works. r=edburns

Chris, Would you take a look to see if you can sr=? Thanks.

The patch looks ok. Couple questions before giving sr, though: 1. I assume you are going to `cvs remove' nsIJVMPluginInstance.h? 2. Is this file required on Mac? If so, do you have somebody to buddy you on that platform to make changes to the project file? beard: do these changes look okay to you?

Joe, I tried the changes (see previous attachment) but was not able to compile MRJPlugin.cpp. I get "Illegal use of abstract class" at: line 220 : MRJPluginInstance* instance = new MRJPluginInstance(this); Someone with more Mozilla years might want to take this further.

To be more precise: Illegal use of abstract class ('snIJVMPluginInstance::GetText(char **)') MRJPlugin.cpp line 220 MRJPluginInstance* instance = new MRJPluginInstance(this);

The original interface was written before XPCOM was fully finalized. We can't very well go changing this method signture right now, because we aren't following XPCOM conventions for out parameters. The current method signature for GetText is: NS_IMETHOD GetText(const char* *result) = 0; Switching to using an XPIDL generated interface changes this to: NS_IMETHOD GetText(char** result) = 0; Which isn't compatible. What the old interface was trying to do was to NOT copy the string (although I'm not sure what this method was for anyway). I think you'll find that we are kind of stuck with the non-IDL generated headers for this version of OJI. I'd suggest we spend time on designing new and better interfaces, and leave these alone.

I should clarify, the old interface was not allocating a new copy of the string, but rather returning a pointer to a constant string that the caller would have to copy. Of course, the MRJ plugin isn't doing anything in this call, so if it is really necessary to change this interface, we can check in a fix.

Thanks for catching a bug, Terry; and thanks for pin-pointing the bug and your input, Patrick. I modified nsIJVMPluginInstance.idl so that it generates the same prototype of GetText as the original header now: NS_IMETHOD GetText(const char ** result) = 0; Terry, can you try to compiler again with the modified nsIJVMPluginInstance.idl (in the 2nd iteration of attachment), and see how it goes? Thanks. Patrick, the Java plugin group requested this change (see Steve katz's comments) so that the plugin can utilize the browser's services directly. Actually the request is to convert most (if not all) headers in ...modules/oji/public and ...modules/plugin/public to xpidl. If I only convert the ones that maintain the identical interface, would that be OK? Please advice. Thanks.

Joe, your 2nd .idl seems to work. MRJPlugin compiled ok, Mozilla ran okay on java sites. Can't claim exhaustive testing. Someone should check to see if I added the files correctly. MacOS 9.1, MRJ 2.2.3, Mozilla pulled apprx 9am today (frozen for 0.8), MRJPlugin 1.0b2 + 6872 changes.

Joe, yes, if the .idl file generates the identical header, then we can go forward with this. Reopening bug.

Chris, Mac seems compiled OK now. About removing the original header files, Ed suggested wait till after Java plugin give it a try with the nightly build containing the new idls. Would that be OK? Would you sr=? Thanks.

Why do you want to leave the original header files in there?

I think we want to wait till Java plugin get a chance to give it (the new idl) a try, and then remove it. Anything to add, Ed?

The purpose of leaving the other file in there was to minimize chances of breaking the tree until we see that it worked. Once we have a green build, we'll remove it.

Tinderbox is not the place to experiment on the build. Please verify that things work *before* removing the files.

Of course we have already verified that this works. We want to defer the removal of the .h files which are being supplanted by the .idl files as an additional check.

Yes, we've tested on Solaris, Windows and Mac.

ok. sr=waterson

Joe, your suspicion was correct, nsIPluginManager.idl was not actually being used in the Mac build. However, I made all the changes necessary to copy of the tree I pulled today, and Mozilla runs Java applets fine (at least, at a 1.0b2 level) using the .h's that were generated by your .idl's. For my own reference, here's the changes I did for nsIPluginManager.idl (analogous to the changes for nsIJVMPluginInstance.idl): 1. replace :mozilla:modules:plugin:public:nsIPluginManager.idl 2. remove from :mozilla:modules:plugin:public:MANIFEST the reference to nsIPluginManager.h 3. remove :mozilla:modules:plugin::public:nsIPluginManager.h and its alias :mozilla:dist:plugin:nsIPluginManager.h 4. create :mozilla:modules:plugin:public:MANIFEST_IDL with a reference to nsIPluginManager.idl 5. add nsIPluginManager.idl to both targets of CodeWarrior project :mozilla:modules:plugin:macbuild:pluginIDL.mcp (<- this is not a text file) 6. using line 574 of mozilla:build:mac:build_scripts:MozillaBuildList.pm as a pattern, add a line for the new MANIFEST_IDL with a dest of $distdirectory:idl: 7. Build without pulling.

Is any of this checked in? What are we waiting for? Patrick: use of the (nasty, but hey) [shared] attribute on GetText might help, or just changing nChar to be |char **|. But do we really want this to be completely non-scriptable (in a real sense -- simply specifying [scriptable] and having no scriptable methods isn't much of a move forward)? Has the nsIJVMPluginInstance interface passed API review and been frozen? If not, let's make it right and then freeze it.

Comments on the patch: - you almost certainly don't want to use the NPL for new files, and you almost certainly don't have a copyright dating back to 1998. - It's the "MOZILLA JAVA VM PLUGIN EXTENSIONS" if it's going in this tree. - Why does the browser reference a specific implementation class? Shouldn't it talk in terms of interfaces as well, so that we have a hope of being able to upgrade the plugin at some point?

We've trying to check in the patch, but first we needed to get Mac ready, then we waited for the tree to be open, .... Should be checked in this week.

The fix was checked into trunk on March 29. Mark fixed.

The original header, modules/oji/public/nsIJVMPluginInstance.h, has been removed, since it is no longer need.

Verified fixed.