155256 - [MachO] Merge Chimera plugin code with trunk to make plugins work in Mozilla

Status: RESOLVED FIXED

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

Description

[Peter Lubczynski]

This bug spawned from bug 152978 in order to fixup plugin code on the trunk so
it works with a Mach-O built Mozilla. Even though plugins work in Chimera, they
do not work at all or even show up in about:plugins in Mozilla daily trunk
builds from:
http://ftp.mozilla.org/pub/mozilla/nightly/latest/

1) NSPR changes are needed for loading plugins
2) Mac-only sections in nsObjectFrame.cpp and other plugin files need changes
everywhere from #ifdef XP_MAC to #if defined(XP_MAC) || defined(XP_MACOSX)
3) Additional changes????  (like maybe to widget toolkit?)

Comment 1

[Peter Lubczynski]

This needs to be done to get plugins working in Mach-O Mozilla builds.

Comment 2

[hacker formerly known as [e-mail]]

Are the patches from bug 131300 & bug 131306 still valid?

Comment 3

[Brian Nesse (gone)]

The patches in bug 131300 are certainly out of date... I have made many changes
to the plugin code. The patch in bug 131306 is probably still current, but I
don't know for certain.

Comment 4

[hacker formerly known as [e-mail]]

Attachment: v1.0

Here's the current CHIMERA_M1_0_1_BRANCH changes as applied to the trunk.  I
tested the quicktime & flash plugins.  I'm also seeing a fairly frequent crash
in QDGetNativeWindowFromPort().  YMMV.

Comment 5

[Brian Nesse (gone)]

The patch in bug 131306 has a serious bug in it which causes libraries to be
unloaded at inappropriate times. There is a patch in bug 154023, but it is
currently exposing another problem (which may be chimera specific).

Comment 6

[Conrad Carlen (not reading bugmail)]

cc'ing myself

Comment 7

[hacker formerly known as [e-mail]]

Attachment: v1.1

This updated patch fixes a couple of cfm build issues and fixes a few bits of
code I missed before.  I'm still seeing the crash when clicking on the
quicktime plugin window though.  The shockwave/flash plugin appears to work
fine.

Comment 8

[hacker formerly known as [e-mail]]

Attachment: trace of qt crash

Comment 9

[Brian Nesse (gone)]

Comment on attachment 102336 [details] [diff] [review]
v1.1

Wow! This looks great Chris. A couple of things I see here:

>Index: modules/plugin/base/src/ns4xPlugin.cpp
>===================================================================
>@@ -300,6 +338,49 @@
>   if(error != NPERR_NO_ERROR || ((fCallbacks.version >> 8) < NP_VERSION_MAJOR))
>     return;
> 
>+#elif defined(XP_MACOSX)
>+  // call into the entry point
>+  NP_MAIN pfnMain = (NP_MAIN) PR_FindSymbol(aLibrary, "main");
>+  
>+  if(pfnMain == NULL)
>+    return;
>+

Just above this code is an #ifdef XP_MAC code hunk that has changed.
#elif defined(XP_MAC)
// get the main entry point
#if !TARGET_CARBON
  NP_MAIN pfnMain = (NP_MAIN) PR_FindSymbol(aLibrary, "mainRD");
#else // carbon
  NP_MAIN pfnMain = (NP_MAIN) PR_FindSymbol(aLibrary, "main");
#endif

This is important because of the #if TARGET_CARBON code below which is
removed...

>@@ -559,37 +655,7 @@
>   if (pluginRefNum == -1)
>     return NS_ERROR_FAILURE;
>   
>-#if TARGET_CARBON
>-  // call into the entry point
>-  NP_MAIN pfnMain = (NP_MAIN) PR_FindSymbol(aLibrary, "main");
>-
>-  if(pfnMain == NULL)
>-    return NS_ERROR_FAILURE;
>-
>-  NPP_ShutdownUPP pfnShutdown;
>-  NPPluginFuncs callbacks;
>-  memset((void*) &callbacks, 0, sizeof(callbacks));
>-  callbacks.size = sizeof(callbacks);
>-  NPError error;
>-
>-  NS_TRY_SAFE_CALL_RETURN(error, CallNPP_MainEntryProc(pfnMain, 
>-                                                       &(ns4xPlugin::CALLBACKS), 
>-                                                       &callbacks, 
>-                                                       &pfnShutdown), fLibrary, nsnull);
>-
>-  NPP_PLUGIN_LOG(PLUGIN_LOG_BASIC, ("NPP MainEntryProc called: return=%d\n",error));
>-
>-  if(error != NPERR_NO_ERROR)
>-    return NS_ERROR_FAILURE;
>-
>-  if ((callbacks.version >> 8) < NP_VERSION_MAJOR)
>-    return NS_ERROR_FAILURE;
>-
>-  // create the new plugin handler
>-  ns4xPlugin* plugin = new ns4xPlugin(&callbacks, aLibrary, (NP_PLUGINSHUTDOWN)pfnShutdown, aServiceMgr);
>-#else // not carbon
>   ns4xPlugin* plugin = new ns4xPlugin(nsnull, aLibrary, nsnull, aServiceMgr);
>-#endif
>   if(plugin == NULL)
>     return NS_ERROR_OUT_OF_MEMORY;
> 

Additionally, the references to mWindow in ns4xPluginInstance.cpp & .h only
support the commented out code in the event handler. I would kill both the
references to mWindow and the #if 0 code.

>Index: modules/plugin/base/src/ns4xPluginInstance.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/src/ns4xPluginInstance.cpp,v
>retrieving revision 1.100
>diff -u -r1.100 ns4xPluginInstance.cpp
>--- modules/plugin/base/src/ns4xPluginInstance.cpp	20 Sep 2002 00:17:28 -0000	1.100
>+++ modules/plugin/base/src/ns4xPluginInstance.cpp	9 Oct 2002 17:07:00 -0000
>@@ -571,6 +571,7 @@
> 
>   fNPP.pdata = NULL;
>   fNPP.ndata = this;
>+  mWindow = NULL;
> 
>   fLibrary = aLibrary;
>   mWindowless = PR_FALSE;
>@@ -1024,6 +1025,8 @@
>     // XXX In the old code, we'd just ignore any errors coming
>     // back from the plugin's SetWindow(). Is this the correct
>     // behavior?!?
>+
>+    mWindow = window;
>   }
>   return NS_OK;
> }
>@@ -1123,12 +1126,30 @@
> 
>   PRInt16 result = 0;
>   
>-  if (fCallbacks->event)
>-    {
>-#ifdef XP_MAC
>-      result = CallNPP_HandleEventProc(fCallbacks->event,
>-                                    &fNPP,
>-                                    (void*) event->event);
>+  if (fCallbacks->event) {
>+#if defined(XP_MAC) || defined(XP_MACOSX)
>+#if 0
>+    switch(event->event->what) {
>+    case updateEvt:
>+        if (mWindow) {
>+            GrafPtr port;
>+            GetPort(&port);
>+            if (port != mWindow->window->port) printf("[!!! port isn't set correctly !!!!]\n");
>+            RGBColor oldColor;
>+            RGBColor green = { 0, 0xFFFF, 0 };
>+            GetForeColor(&oldColor);
>+            RGBForeColor(&green);
>+            PaintRect((Rect*)&mWindow->clipRect);
>+            // ::QDFlushPortBuffer(port, NULL);
>+            RGBForeColor(&oldColor);
>+            // printf("[portx = %d, porty = %d]\n", mWindow->window->portx, mWindow->window->porty);
>+        }
>+        break;
>+    }
>+#endif
>+    result = CallNPP_HandleEventProc(fCallbacks->event,
>+                                     &fNPP,
>+                                     (void*) event->event);
> #endif
> 
> #if defined(XP_WIN) || defined(XP_OS2)
>Index: modules/plugin/base/src/ns4xPluginInstance.h
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/src/ns4xPluginInstance.h,v
>retrieving revision 1.27
>diff -u -r1.27 ns4xPluginInstance.h
>--- modules/plugin/base/src/ns4xPluginInstance.h	3 Oct 2002 21:26:20 -0000	1.27
>+++ modules/plugin/base/src/ns4xPluginInstance.h	9 Oct 2002 17:07:00 -0000
>@@ -189,6 +189,7 @@
>      * instance and the browser.
>      */
>     NPP_t fNPP;
>+    nsPluginWindow* mWindow;
> 
>     //these are used to store the windowless properties
>     //which the browser will later query

Comment 10

[hacker formerly known as [e-mail]]

Attachment: v1.2

Add workaround for scriptable plugins crash by returning
NS_ERROR_NOT_IMPLEMENTED for ns4xPluginInstance :: GetScriptableInterface .

Comment 11

[hacker formerly known as [e-mail]]

*** Bug 115639 has been marked as a duplicate of this bug. ***

Comment 12

[Peter Lubczynski]

Comment on attachment 103908 [details] [diff] [review]
v1.2

r=peterl

Comment 13

[hacker formerly known as [e-mail]]

*** Bug 131300 has been marked as a duplicate of this bug. ***

Comment 14

[Peter Lubczynski]

--->seawood's got this

Comment 15

[Simon Fraser [no longer active]]

Comment on attachment 103908 [details] [diff] [review]
v1.2

This is a good first pass. Some of the Chimera plugin code will have been
hacked to work with cocoa widgets, rather than the carbon widgets that Macho is
using, so we might need to do a bit more fixing. In addition, I'm about to land
a bunch of plugin fixes for chimera, some of which the trunk may also need.

But this gets us closer, so sr=sfraser, as long as you #ifdef all the non-debug
printfs (e.g. in nsPluginsDirDarwin.cpp)

Comment 16

[hacker formerly known as [e-mail]]

Attachment: v1.3

I found a problem with the previous patch when attempting to compile a cfm
carbon build.  The TARGET_CARBON problem that bnesse pointed out before wasn't
completely resolved.  The cfm carbon build was complaining that

  NS_TRY_SAFE_CALL_RETURN(error, CallNPP_MainEntryProc(pfnMain, 
						      
&(ns4xPlugin::CALLBACKS), 
						       &fCallbacks, 
						       &fShutdownEntry),
aLibrary, nsnull);

was attempting to implicitly cast a short ** to a void**.  Looking at the old
code, it looks like the carbon build was creating a local NPPluginFuncs struct
and making the above call using it instead of the class variable...which not
coincidentally is exactly what the mach-o build does. So the cfm carbon build
now takes the mach-o path.  This seems to work though it takes ~12 secs for
about:plugins to come up in my opt cfm carbon build.  I don't think it's
directly related to this latest change (since it didn't compile before).

Also, gcc was warning that this test would always be false because of the
limited range of the data type:

  if ((np_callbacks.version >> 8) < NP_VERSION_MAJOR)

version is a uint16. This works around the problem:

  int cb_version = np_callbacks.version;
  if ((cb_version >> 8) < NP_VERSION_MAJOR)

And I put DEBUG ifdefs around the printfs.

Comment 17

[Peter Lubczynski]

Comment on attachment 104830 [details] [diff] [review]
v1.3

r=peterl

Comment 18

[Simon Fraser [no longer active]]

Comment on attachment 104830 [details] [diff] [review]
v1.3

sr=sfraser

Comment 19

[hacker formerly known as [e-mail]]

The patch has been checked into the trunk.  Still pending drivers approval for
the 1.2 branch.

Comment 20

[Chris Blackstone]

QuickTime, Flash, and Shockwave Plug-ins all work

tested with:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.2b) Gecko/20021107

great job people!

Comment 21

[hacker formerly known as [e-mail]]

Not going to make 1.2.

Comment 22

[Christopher Blizzard (:blizzard)]

This can be checked in on the 1.2 branch after 1.2 has been released if you guys
want to do chimera work but it's too painful for the actual 1.2 release.