98285 - Add a possibility to access the service manager via NP (legacy plugin) API

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[av (gone)]

We can use the NPN_GetValue back door to achieve this goal just by adding one
more variable to NPNVariable enumeration type.

Comment 1

[av (gone)]

Targeting for 0.9.4.

Comment 2

[av (gone)]

Attachment: patch -- first try

Comment 3

[Patrick C. Beard]

One comment, even though 
nsServiceManager::GetGlobalServiceManager() doesn't AddRef the 
nsIServiceManager, I don't think we should adhere to that here. I think we 
should instead AddRef() the nsIServiceManager instance returned to the 
caller, so the caller can use Release() on the interface.

Comment 4

[Patrick C. Beard]

Here's a revised patch, that actually returns the reference to the service 
manager, and adds a reference to it.

Comment 5

[Patrick C. Beard]

Attachment: Add a reference on the way out, do proper casting of result variable.

Comment 6

[Jaime Rodriguez, Jr.]

Adding PDT to status whiteboard. + pending review of risk

Comment 7

[av (gone)]

Attachment: third try -- |break;| added to the last case statement so it doesn't fall through the next case when it is added

Comment 8

[av (gone)]

Can anybody comment, review, superreview etc. please?

Comment 9

[av (gone)]

Attachment: fourth try -- typo and compiler warning fixed

Comment 10

[Patrick C. Beard]

r=beard

Comment 11

[Doug Turner (:dougt)]

patrick, av, 

what is the |gServiceMgr| for?  Is _getvalue called prior to
ns4xPlugin::ns4xPlugin?  I guess I just don't understand why in some of the _XXX
functions, we are using |gServiceMgr| and in this one, we are calling the static
nsServiceManager function...

Comment 12

[Patrick C. Beard]

Doug, the idea is to provide a single hook to allow an NPAPI plugin to access
the service manager, and thus any services or components that we may choose to
freeze and encourage external developers to use. Without access to the
service/component manager, this is impossible. I cc'd you and dp to get your
input on this. We want to make non-XPCOM based plugins first class citizens in
future versions of the browser.

Comment 13

[Patrick C. Beard]

Doug, is there a better way to obtain a reference to the global service manager,
rather than through nsServiceManager::GetGlobalServiceManager()?

Comment 14

[Doug Turner (:dougt)]

since you are already linking to xpcom for various reasons, and modules/plugin
(dispite is location under 'modules') is a core part of the browser, no.

Comment 15

[vidur (gone)]

I presume the idea is to not require plugins to link with the XPCOM DLL. If so,
sr=vidur.

Comment 16

[Patrick C. Beard]

Vidur, yes, that is the case. A classic NPAPI plugin must still be able to run
in other browsers, and hard linking against XPCOM would prevent that on many
platforms.

Comment 17

[Patrick C. Beard]

Let's please get this approved for checkin on the 0.9.4 branch.

Comment 18

[Jaime Rodriguez, Jr.]

AV - This looks a good nomination. Pls work with you manager to get this one
nsbranch+ ASAP.

Comment 19

[av (gone)]

Missing the train :( Removing milestone and marking fixed.

Comment 20

[av (gone)]

Reopening as we still want it for the branch.

Comment 21

[Patrick C. Beard]

Marking nsbranch+, I believe that this is an important feature of our plugin 
architecture to get into 0.9.4, so that our plugin developers can get earlier 
access to XPCOM services. The scriptable plugin support was added in 
0.9.2, and I would hate to see this complementary feature lag by yet 
another milestone.

Comment 22

[Asa Dotzler [:asa]]

0.9.4 is out the door.

Comment 23

[Jaime Rodriguez, Jr.]

AV - Are you ready to check-in? - PDT

Comment 24

[av (gone)]

Yes, waiting for PDT+ in the status, as to my understanding of the process this 
is what the formal permission to check in is.

Comment 25

[Jaime Rodriguez, Jr.]

U GOT IT - PDT+ it is.

Comment 26

[av (gone)]

Checked in to the branch. Marking fixed.

Comment 27

[shrirang khanzode]

marking verif(stamp)