64297 - nsIPluginManager not defined in xpidl

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Doug Turner (:dougt)]

http://lxr.mozilla.org/seamonkey/source/modules/plugin/public/nsIPluginManager.h
#69

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.  [e-mail] can fill in 
the requirement details.

Comment 1

[Steven Katz]

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.  

Comment 2

[edburns]

reassign.

Comment 3

[Joe Chou]

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

Comment 4

[Steven Katz]

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.

Comment 5

[Joe Chou]

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,
"[e-mail] 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.

Comment 6

[Joe Chou]

When do you need this fix, Steve?

Comment 7

[Steven Katz]

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.

Comment 8

[Joe Chou]

nsIPluginManager.idl already exists. I'll just add it to the make files (Unix
and Windows).

Comment 9

[Joe Chou]

Attachment: cvs diff -u, 1st iteration.

Comment 10

[edburns]

Comments on this diff

* RCS file: /cvsroot/mozilla/modules/plugin/public/makefile.win,v

Why is there an h file in the XPIDLSRCS macro?  It should be
nsIPluginManager.idl.

 XPIDLSRCS = 				      \
 	.\nsIScriptablePlugin.idl             \
-	.\nsIHTTPHeaderListener.idl     \
+	.\nsIHTTPHeaderListener.idl           \
+	.\nsIPluginManager.h                  \
 	$(NULL)
	

* RCS file: /cvsroot/mozilla/modules/plugin/public/nsIPluginManager.idl,v

gpatch -i \Projects\chaff\64297\nsIPluginManager.idl.patch nsIPluginManager.idl
patching file `nsIPluginManager.idl'
Hunk #1 FAILED at 35.
Hunk #2 FAILED at 97.
Hunk #3 FAILED at 122.
Hunk #4 FAILED at 158.
4 out of 4 hunks FAILED -- saving rejects to nsIPluginManager.idl.rej

Please post the modified IDL file as an attachment so I can test it out.
For some reason, the patch wouldn't apply.

Follow the above suggestions, and I'll give review appoval.

Comment 11

[Joe Chou]

Attachment: cvs_diff_u, 2nd iteration

Comment 12

[Joe Chou]

Attachment: patch_64297.tar.gz

Comment 13

[edburns]

Comments on second iteration:

* RCS file: /cvsroot/mozilla/modules/plugin/public/makefile.win,v

You need to remove nsIPluginManager.h from this file.

Other than that looks good.  Make that change and you have r=edburns.

Comment 14

[Joe Chou]

Attachment: cvs_diff_u, 3rd iteration

Comment 15

[Joe Chou]

Attachment: patch_64297.tar.gz, 3rd iteration

Comment 16

[edburns]

looks good.  r=edburns.

Get sr=waterson and I'll check it in (if you don't have your ID yet, but you
should by now, right?)

Comment 17

[Joe Chou]

Chris, would you check to see if you can sr= this patch? Thanks.

Comment 18

[Chris Waterson]

Please submit patches as `cvs diff -u'. If you are adding new files, `cvs diff 
-uN'.

Comment 19

[Joe Chou]

Attachment: Plain text of modified nsIPluginManager.idl

Comment 20

[Chris Waterson]

Same questions as for bug 64296:

1. Will you be `cvs remove'-ing nsIPluginManager.h?

2. Do you have a Mac buddy?

Comment 21

[Joe Chou]

Same questions as for bug 64296:

1. Will you be `cvs remove'-ing nsIPluginManager.h?

Yes, I will remove the old header files.

2. Do you have a Mac buddy?

Yes, Terry (a new member in our OJI team who supports Mac) will update the Mac
files.

Comment 22

[Terry Noyes]

See my attachment and notes in bug #64296.  Couldn't get Mac's MRJPlugin.cpp to
compile in MRJPlugin::CreateInstance.

Comment 23

[Patrick C. Beard]

Are these files generating the *EXACT* same interface? Are there Mac project 
changes that will create the header file? I haven't seen anything about that.

Comment 24

[Joe Chou]

I believe that the bug Terry found in 64296 stopped the compilation before it
came to 64297. With the fix attached to 64296, Terry is going to compile again.
About the interface, as I checked, they looked identical to me. But as you know
I've missed one in 64296 already, there goes my credit. 

Comment 25

[Joe Chou]

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.

Comment 26

[Chris Waterson]

Why do you want to leave the original headers in?

Comment 27

[Joe Chou]

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?

Comment 28

[karnaze (gone)]

Moving to m1.0.

Comment 29

[Joe Chou]

The fix was checked in on March 29th, and the original header file,
modules/plugin/public/nsIPluginManager.h, has been removed. Mark fixed.

Comment 30

[edburns]

This appears fixed.

Comment 31

[shrirang khanzode]

MArking Verified...(stamp)