RESOLVED FIXED 4319
NetscapeMoviePlugIn example code scripting doesn't work in Firefox
https://bugs.webkit.org/show_bug.cgi?id=4319
Summary NetscapeMoviePlugIn example code scripting doesn't work in Firefox
Arnaud
Reported 2005-08-07 08:18:44 PDT
The plugin located at /Developer/Examples/WebKit/NetscapeMoviePlugIn in Tiger works with safari, but in Firefox it crashes when javascript code tries to use scriptable methods or properties. I use the the sample html page on the latest Firefox release (1.0.6). I found in the plugin source that browser->retainobject() is not called when a scriptable method returns a NPObject, which is not conform to the mozilla implementation of the NSAPI. (Calling this function seems to fix the problem, but probably causes memory leaks in Safari.) I suppose the object lifecycle managment implemented in WebKit follows the CoreFoundation conventions, ie a getter doesn't increment the reference counter of a returned object. It would be better to follow mozilla conventions (which is the same as XPCOM and COM) : objects have their ref counts incremented before being returned by a scriptable function. In this case the caller has the responsibility to release the ref count of a returned object when it doesn't use it anymore.
Attachments
here's a patch that should fix the problem (22.27 KB, patch)
2006-03-14 05:59 PST, Darin Adler
timo: review-
Darin Adler
Comment 1 2005-11-03 07:57:03 PST
RealPlayer has
Darin Adler
Comment 2 2005-11-03 07:57:40 PST
Since we know that the RealPlayer plug-in works with both Safari and Firefox, and uses the scripting API, we know it's possible. Lets figure out what's up.
Darin Adler
Comment 3 2005-11-04 08:53:46 PST
Same as <rdar://problem/4211707> npapi ref count behavior differs with Mozilla, it seems.
Darin Adler
Comment 4 2006-01-22 15:25:46 PST
Tim Omernick says that this is *not* the same as <rdar://problem/4211707>.
Darin Adler
Comment 5 2006-03-14 05:57:48 PST
The WebKit examples are not part of the open-source project, so this bug belongs in Radar rather than here in the open source bugzilla. (However we'd like to see that change in the future.)
Darin Adler
Comment 6 2006-03-14 05:59:01 PST
Created attachment 7059 [details] here's a patch that should fix the problem
Darin Adler
Comment 7 2006-03-14 05:59:56 PST
My patch fixes the problem complained about here, as well as fixing many other small style issues and mistakes in the example. The biggest mistake was that there was a lot of function pointer casting.
Arnaud
Comment 8 2006-03-14 10:11:27 PST
I think it doesn't completely fix the problem even if NetscapeMoviePlugIn is now OK. Now NetscapeMoviePlugIn has probably memory leaks in Safari. Something must be changed in WebKit itself. But in this case, I don't how compatibility with existing WebKit plugins could be maintained...
Darin Adler
Comment 9 2006-03-14 10:58:31 PST
(In reply to comment #8) > I think it doesn't completely fix the problem even if NetscapeMoviePlugIn is > now OK. Now NetscapeMoviePlugIn has probably memory leaks in Safari. > > Something must be changed in WebKit itself. > But in this case, I don't how compatibility with existing WebKit plugins could > be maintained... Yes, that's right. I believe Tim has worked the details out. Tim, can you comment here or in another bug or something?
Alice Liu
Comment 10 2006-03-16 17:43:06 PST
Darin Adler
Comment 11 2006-07-30 09:14:59 PDT
Tim says there are still problems with this.
Darin Adler
Comment 12 2006-07-30 09:25:04 PDT
Comment on attachment 7059 [details] here's a patch that should fix the problem Clearing review flag on this old patch, since it didn't solve the whole problem.
Tim Omernick
Comment 13 2006-08-16 18:38:01 PDT
The plug-in needs to be changed to return a retained NPObject from NPP_GetValue().  We also need to change WebKit to expect a retained object back from NPP_GetValue() so that our retain/release rules match other browsers. I have a patch to fix this, and will be landing it shortly.
Tim Omernick
Comment 14 2006-08-28 16:13:24 PDT
Fixed in OpenSource revision 16086, Internal revision 10181.
Tim Omernick
Comment 15 2006-08-28 16:13:56 PDT
Comment on attachment 7059 [details] here's a patch that should fix the problem This is not the patch we landed.  See diffs in OpenSource revision 16086, Internal revision 10181.
Note You need to log in before you can comment on or make changes to this bug.