344425 - support NP_GetEntryPoints as a way to get plugin function pointers

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, enhancement)

Description

[Josh Aas]

When we load a plugin, we get function pointers from it by calling its "main()" function. This is a bit antiquated, WebKit has support for getting function pointers via a function called NP_GetEntryPoints. We should try to get function pointers from there instead of main() if possible, that way it'll be easier for plugin authors to support both browsers.

Comment 1

[Josh Aas]

Attachment: fix v1.0

This patch adds support for NP_Initialize/NP_GetEntryPoints for sending/getting function pointers. However, support for this is #ifdef'd so that it is Intel-only. The reason for this is that on PPC we run into the possibility of the plugin expecting or sending tvector-wrapped function pointers. Since there was no protocol for this, some plugins expect tvectors via NP_Initialize, others expect function pointers, and we have the same problem with not knowing what we're getting from the plugin.

There are two possible solutions to this problem if we want to move towards API compatibility with WebKit. First we could take a patch like this and as we move towards an Intel-Mac-Only world we gain API compatibility. The other possible solution is to add a plugin-side call to npapi - if that function exists then we can call it and find out whether the plugin wants tvectors or function pointers. Actually, if the function exists then the author will probably be aware of our situation and we can just have that author follow a certain protocol and we don't even really need to ask.

Comment 2

[Mark Mentovai]

Comment on attachment 229678 [details] [diff] [review]
fix v1.0

+  NP_PLUGINSHUTDOWN pfnShutdown = (NP_PLUGINSHUTDOWN)PR_FindSymbol(aLibrary, "NP_Shutdown");

I think this should be inside the #ifdef, but this patch does other confusing things with pfnShutdown and pfnMainShutdown, so I don't really know what you intended.

>+#ifdef MACOSX_GETENTRYPOINT_SUPPORT
>+  NP_GETENTRYPOINTS pfnGetEntryPoints = (NP_GETENTRYPOINTS)PR_FindSymbol(aLibrary, "NP_GetEntryPoints");
>+  NP_PLUGININIT pfnInitialize = (NP_PLUGININIT)PR_FindSymbol(aLibrary, "NP_Initialize");
>+  usesGetEntryPoints = (pfnGetEntryPoints && pfnInitialize && pfnShutdown);
>+
>+  if (usesGetEntryPoints) {
>+    // we call NP_Initialize before getting function pointers to match
>+    // WebKit's behavior. They implemented this first on Mac OS X.
>+    if (pfnInitialize(&(ns4xPlugin::CALLBACKS)) != NS_OK)
>+      return;
>+    if (pfnGetEntryPoints(&np_callbacks) != NS_OK)
>+      return;
>+  }
>+  else {
>+#endif

End the block like this:
  }
  else
#endif
  {

Then you can get rid of the maintenance-nightmare #ifdef at the end of the next block, and you ensure that the block always has the same scoping, regardless of whether the #ifdef is true or false.

>+    NPError error;
>+    NPP_ShutdownUPP pfnMainShutdown;
>+    
>+    // call into the entry point
>+    NP_MAIN pfnMain = (NP_MAIN)PR_FindSymbol(aLibrary, "main");
>+    
>+    if (pfnMain == NULL)
>+      return;
>+    
>+    NS_TRY_SAFE_CALL_RETURN(error,
>+                            CallNPP_MainEntryProc(pfnMain,
>+                                                  &(ns4xPlugin::CALLBACKS),
>+                                                  &np_callbacks,
>+                                                  &pfnMainShutdown),

What's pfnMainShutdown?  You never use it, is an API change.  I assume you meant to leave pfnShutdown?

>+                            aLibrary,
>+                            nsnull);
>+    
>+    NPP_PLUGIN_LOG(PLUGIN_LOG_BASIC,
>+                   ("NPP MainEntryProc called: return=%d\n",error));
>+    
>+    if (error != NPERR_NO_ERROR)
>+      return;
>+    
>+    fShutdownEntry = (NP_PLUGINSHUTDOWN) TV2FP(pfnShutdown);

pfnShutdown was already a function pointer.

>-  fShutdownEntry = (NP_PLUGINSHUTDOWN) TV2FP(pfnShutdown);
>+  fShutdownEntry = pfnShutdown;

I think fShutdownEntry should be what we got back from CallNPP_MainEntryProc if we're using main.

Comment 3

[Josh Aas]

Attachment: fix v1.1

Addresses comments

Comment 4

[Robert Sayre]

Comment on attachment 236983 [details] [diff] [review]
fix v1.1

>
>+    NPP_ShutdownUPP pfnMainShutdown;

pfnMainShutdown lives on. doesn't bother me though. r=sayrer

Comment 5

[Robert Sayre]

(In reply to comment #4)
> (From update of attachment 236983 [details] [diff] [review] [edit])
> >
> >+    NPP_ShutdownUPP pfnMainShutdown;

on IRC, Josh says pfnMainShutdown was the name of a different variable in the old patch. No issue here.

Comment 6

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 236983 [details] [diff] [review]
fix v1.1

sr=pink

Comment 7

[Simon Fraser [no longer active]]

Comment on attachment 236983 [details] [diff] [review]
fix v1.1

>Index: ns4xPlugin.cpp
>===================================================================

>+#ifdef MACOSX_GETENTRYPOINT_SUPPORT
>+  fShutdownEntry = (NP_PLUGINSHUTDOWN)PR_FindSymbol(aLibrary, "NP_Shutdown");
>+  NP_GETENTRYPOINTS pfnGetEntryPoints = (NP_GETENTRYPOINTS)PR_FindSymbol(aLibrary, "NP_GetEntryPoints");
>+  NP_PLUGININIT pfnInitialize = (NP_PLUGININIT)PR_FindSymbol(aLibrary, "NP_Initialize");
>+  usesGetEntryPoints = (pfnGetEntryPoints && pfnInitialize && fShutdownEntry);
>+
>+  if (usesGetEntryPoints) {
>+    // we call NP_Initialize before getting function pointers to match
>+    // WebKit's behavior. They implemented this first on Mac OS X.
>+    if (pfnInitialize(&(ns4xPlugin::CALLBACKS)) != NS_OK)

Should this be testing against NS_OK, or NPERR_NO_ERROR?

>+      return;
>+    if (pfnGetEntryPoints(&np_callbacks) != NS_OK)
>+      return;

Ditto.

Comment 8

[Josh Aas]

Simon's comment #7 fixed on trunk. Nice catch!