Commit cf7d6d65 authored by tomernic's avatar tomernic

WebCore:

        Reviewed by John Sullivan.

        Part of <rdar://problem/4481553> NetscapeMoviePlugIn example code scripting doesn't work in Firefox (4319)
        <http://bugzilla.opendarwin.org/show_bug.cgi?id=4319>: NetscapeMoviePlugIn example code scripting doesn't work
        in Firefox

        No test cases added, since this is essentially a leak fix.

        A brief history of NPP_GetValue(), NPObjects, and reference counting.

        Earlier versions of WebKit incorrectly interpreted the NPRuntime reference counting rules.  We failed to take
        into account the fact that plug-ins are required to retain NPObjects before returning them.  This creates several
        classes of interesting plug-ins:

        1) Plug-ins tested in WebKit and other browsers.  These plug-ins may have WebKit-specific workarounds to not retain
           the returned NPObject, thus avoiding the memory leak in WebKit.

        2) Plug-ins tested only in other browsers.  These plug-ins must already retain their NPObjects, since other browsers
           implemented the NPRuntime retain/release rules correctly.  These plug-ins likely work in WebKit, but probably leak
           NPObjects since WebKit adds its own retain in addition to the plug-in's retain.

        3) Plug-ins tested only in WebKit, that fail to retain their NPObjects before returning them.
           Such plug-ins are guaranteed to crash in other browsers due to the missing expected retain.  These plug-ins
           work in older WebKits because WebKit did not expect the plug-in to retain the NPObject.  Now that our retain
           rules match other browsers, these plug-ins may crash due to the difference in retain/release behavior.  We could
           potentially detect that situation and correct it here, but I consider it a bug that the plug-in did not follow the
           documented NPRuntime reference counting rules.  Furthermore, it is extremely unlikely that someone would develop
           a Netscape plug-in and test it *only* in WebKit.  The entire purpose of creating a Netscape plugin is so that it
           works in all browsers!

        4) Plug-ins tested only in WebKit, that properly retain their NPObjects before returning them.
           These plug-ins probably work in other browsers, and leak their NPObjects in older WebKits because of WebKit's
           extra retain.  A developer of this type of plug-in is probably unaware of the NPObject leak.  A more savvy developer
           would create a plug-in that fits into category #1.
        
        I am changing our NPP_GetValue() behavior to match Firefox and other browsers -- the plug-in is now expected to retain the
        returned NPObject, and the browser is expected to release it when done.  This means that plug-ins in category #3 need to be
        changed so that they don't crash in Safari.  However, such plug-ins already crash in every other browser, so I do not feel that
        this needs to be handled specifically by WebKit.

        * bridge/mac/FrameMac.mm:
        Changed -pluginScriptableObject to -createPluginScriptableObject to make clearer the contract that the method must return a
        retained NPObject.  Also changed it to return an actual NPObject* instead of a void*.  There is only one caller of this method,
        and only one implementor.  Using void* here is a needless abstraction.  It's an NPObject*!  Admit it!
        (WebCore::getInstanceForView):
        Release the NPObject after creating the bindings instance.  This is the actual bug fix.

WebKit:

        Reviewed by John Sullivan.

        Part of <rdar://problem/4481553> NetscapeMoviePlugIn example code scripting doesn't work in Firefox (4319)
        <http://bugzilla.opendarwin.org/show_bug.cgi?id=4319>: NetscapeMoviePlugIn example code scripting doesn't work
        in Firefox

        * Plugins/WebBaseNetscapePluginView.h:
        * Plugins/WebBaseNetscapePluginView.m:
        (-[WebBaseNetscapePluginView createPluginScriptableObject]):
        Renamed this method (see corresponding WebCore ChangeLog entry for an explanation).
        Style changes.

WebKitTools:

        Reviewed by John Sullivan.

        Part of <rdar://problem/4481553> NetscapeMoviePlugIn example code scripting doesn't work in Firefox (4319)
        <http://bugzilla.opendarwin.org/show_bug.cgi?id=4319>: NetscapeMoviePlugIn example code scripting doesn't work
        in Firefox

        * DumpRenderTree/TestNetscapePlugIn.subproj/main.c:
        (NPP_GetValue):
        WebKit's NPP_GetValue() reference counting behavior has been changed to match Firefox.  NPObject return values
        are expected to be retained by the plug-in, and released by the caller.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@16086 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 4bd01007
2006-08-28 Tim Omernick <timo@apple.com>
Reviewed by John Sullivan.
Part of <rdar://problem/4481553> NetscapeMoviePlugIn example code scripting doesn't work in Firefox (4319)
<http://bugzilla.opendarwin.org/show_bug.cgi?id=4319>: NetscapeMoviePlugIn example code scripting doesn't work
in Firefox
No test cases added, since this is essentially a leak fix.
A brief history of NPP_GetValue(), NPObjects, and reference counting.
Earlier versions of WebKit incorrectly interpreted the NPRuntime reference counting rules. We failed to take
into account the fact that plug-ins are required to retain NPObjects before returning them. This creates several
classes of interesting plug-ins:
1) Plug-ins tested in WebKit and other browsers. These plug-ins may have WebKit-specific workarounds to not retain
the returned NPObject, thus avoiding the memory leak in WebKit.
2) Plug-ins tested only in other browsers. These plug-ins must already retain their NPObjects, since other browsers
implemented the NPRuntime retain/release rules correctly. These plug-ins likely work in WebKit, but probably leak
NPObjects since WebKit adds its own retain in addition to the plug-in's retain.
3) Plug-ins tested only in WebKit, that fail to retain their NPObjects before returning them.
Such plug-ins are guaranteed to crash in other browsers due to the missing expected retain. These plug-ins
work in older WebKits because WebKit did not expect the plug-in to retain the NPObject. Now that our retain
rules match other browsers, these plug-ins may crash due to the difference in retain/release behavior. We could
potentially detect that situation and correct it here, but I consider it a bug that the plug-in did not follow the
documented NPRuntime reference counting rules. Furthermore, it is extremely unlikely that someone would develop
a Netscape plug-in and test it *only* in WebKit. The entire purpose of creating a Netscape plugin is so that it
works in all browsers!
4) Plug-ins tested only in WebKit, that properly retain their NPObjects before returning them.
These plug-ins probably work in other browsers, and leak their NPObjects in older WebKits because of WebKit's
extra retain. A developer of this type of plug-in is probably unaware of the NPObject leak. A more savvy developer
would create a plug-in that fits into category #1.
I am changing our NPP_GetValue() behavior to match Firefox and other browsers -- the plug-in is now expected to retain the
returned NPObject, and the browser is expected to release it when done. This means that plug-ins in category #3 need to be
changed so that they don't crash in Safari. However, such plug-ins already crash in every other browser, so I do not feel that
this needs to be handled specifically by WebKit.
* bridge/mac/FrameMac.mm:
Changed -pluginScriptableObject to -createPluginScriptableObject to make clearer the contract that the method must return a
retained NPObject. Also changed it to return an actual NPObject* instead of a void*. There is only one caller of this method,
and only one implementor. Using void* here is a needless abstraction. It's an NPObject*! Admit it!
(WebCore::getInstanceForView):
Release the NPObject after creating the bindings instance. This is the actual bug fix.
2006-08-28 Alice Liu <alice.liu@apple.com>
Reviewed by Geoff.
......
......@@ -81,7 +81,7 @@
@interface NSObject (WebPlugIn)
- (id)objectForWebScript;
- (void *)pluginScriptableObject;
- (NPObject *)createPluginScriptableObject;
@end
using namespace std;
......@@ -2997,11 +2997,16 @@ static KJS::Bindings::Instance *getInstanceForView(NSView *aView)
return KJS::Bindings::Instance::createBindingForLanguageInstance (KJS::Bindings::Instance::ObjectiveCLanguage, object, executionContext);
}
}
else if ([aView respondsToSelector:@selector(pluginScriptableObject)]){
void *object = [aView pluginScriptableObject];
else if ([aView respondsToSelector:@selector(createPluginScriptableObject)]) {
NPObject *object = [aView createPluginScriptableObject];
if (object) {
KJS::Bindings::RootObject *executionContext = KJS::Bindings::RootObject::findRootObjectForNativeHandleFunction ()(aView);
return KJS::Bindings::Instance::createBindingForLanguageInstance (KJS::Bindings::Instance::CLanguage, object, executionContext);
KJS::Bindings::RootObject *executionContext = KJS::Bindings::RootObject::findRootObjectForNativeHandleFunction()(aView);
KJS::Bindings::Instance *instance = KJS::Bindings::Instance::createBindingForLanguageInstance(KJS::Bindings::Instance::CLanguage, object, executionContext);
// -createPluginScriptableObject returns a retained NPObject. The caller is expected to release it.
_NPN_ReleaseObject(object);
return instance;
}
}
return 0;
......
2006-08-28 Tim Omernick <timo@apple.com>
Reviewed by John Sullivan.
Part of <rdar://problem/4481553> NetscapeMoviePlugIn example code scripting doesn't work in Firefox (4319)
<http://bugzilla.opendarwin.org/show_bug.cgi?id=4319>: NetscapeMoviePlugIn example code scripting doesn't work
in Firefox
* Plugins/WebBaseNetscapePluginView.h:
* Plugins/WebBaseNetscapePluginView.m:
(-[WebBaseNetscapePluginView createPluginScriptableObject]):
Renamed this method (see corresponding WebCore ChangeLog entry for an explanation).
Style changes.
2006-08-28 Brady Eidson <beidson@apple.com>
Reviewed by Tim Hatcher's rubberstamp
......
......@@ -132,7 +132,8 @@ typedef union PluginPort {
- (void)viewDidMoveToHostWindow;
// Returns the NPObject that represents the plugin interface.
- (void *)pluginScriptableObject;
// The return value is expected to be retained.
- (NPObject *)createPluginScriptableObject;
// -willCallPlugInFunction must be called before calling any of the NPP_* functions for this view's NPP instance.
// This is necessary to ensure that plug-ins are not destroyed while WebKit calls into them. Some plug-ins (Flash
......
......@@ -1711,18 +1711,19 @@ - (void)preferencesHaveChanged:(NSNotification *)notification
}
}
- (void *)pluginScriptableObject
- (NPObject *)createPluginScriptableObject
{
if (NPP_GetValue) {
void *value = 0;
[self willCallPlugInFunction];
NPError npErr = NPP_GetValue (instance, NPPVpluginScriptableNPObject, (void *)&value);
[self didCallPlugInFunction];
if (npErr == NPERR_NO_ERROR) {
return value;
}
}
return (void *)0;
if (!NPP_GetValue)
return NULL;
void *value = NULL;
[self willCallPlugInFunction];
NPError error = NPP_GetValue(instance, NPPVpluginScriptableNPObject, (void *)&value);
[self didCallPlugInFunction];
if (error != NPERR_NO_ERROR)
return NULL;
return value;
}
- (void)willCallPlugInFunction
......
2006-08-16 Tim Omernick <timo@apple.com>
Reviewed by John Sullivan.
Part of <rdar://problem/4481553> NetscapeMoviePlugIn example code scripting doesn't work in Firefox (4319)
<http://bugzilla.opendarwin.org/show_bug.cgi?id=4319>: NetscapeMoviePlugIn example code scripting doesn't work
in Firefox
* DumpRenderTree/TestNetscapePlugIn.subproj/main.c:
(NPP_GetValue):
WebKit's NPP_GetValue() reference counting behavior has been changed to match Firefox. NPObject return values
are expected to be retained by the plug-in, and released by the caller.
2006-08-28 Nikolas Zimmermann <zimmermann@kde.org>
Reviewed by Tim Hatcher.
......
......@@ -133,6 +133,8 @@ NPError NPP_GetValue(NPP instance, NPPVariable variable, void *value)
if (variable == NPPVpluginScriptableNPObject) {
void **v = (void **)value;
PluginObject *obj = instance->pdata;
// Return value is expected to be retained
browser->retainobject((NPObject *)obj);
*v = obj;
return NPERR_NO_ERROR;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment