shouldRetainBrowserObject()


#1

This code seems to cause an intentional memory leak in Safari. I’m just wondering what the reason is? Is it possible Safari accesses the plugin after NPP_SetWindow() or something?

Its indirectly causing my plugin to leak memory over the course of many page reloads so I’m trying to find a way to avoid it.

[code] bool shouldRetainBrowserObject() const
{
const String version (browser.uagent (npp));

    if (! version.containsIgnoreCase (" AppleWebKit/"))
        return true;

    int versionNum = version.fromFirstOccurrenceOf (" AppleWebKit/", false, true).getIntValue();

    return versionNum == 0 || versionNum >= 420;
}[/code]

#2

Can’t remember the precise details, but it was something to do with safari and firefox behaving differently in terms of the reference counting of the object. I wrote it a while ago, so it could be the case that safari has changed its policy since I wrote that code…


#3

To confirm it wasn’t actually needed, I’ve modified the function to return false and I’m running it in a loop on Safari (version 531) and so far no problems. Did they fix this webkit problem in a recent build perhaps?


#4

Hmm. According to Apple it should be retained. See the note on page 19 of this:
http://www.google.co.uk/url?sa=t&source=web&ct=res&cd=5&ved=0CCQQFjAE&url=http%3A%2F%2Fdeveloper.apple.com%2Fmac%2Flibrary%2Fdocumentation%2FInternetWeb%2FConceptual%2FWebKit_PluginProgTopic%2FWebKit_PluginProgTopic.pdf&rct=j&q=appkit+420+retain+plugin+version&ei=st3rS4yDO5v40wTI3dHmBw&usg=AFQjCNGUiie-l4snQ5YHAY_hL8x8HSQHdg


#5

Wow.

I wish they provided a bit more detail on why that might be the case. Wreaks of a bug workaround to me but I don’t know enough to say for sure.

I set my page to run a JS test harness and reload if it passed all tests (searching for timing bugs). The plugin was never properly freed until I removed that line. I’ll do some more digging.


#6

I’m at a loss about this. There seems to be no info about it. Were you just following that document when you implemented your Mac version?

I’m not sure why it works without retaining in every scenario I throw at it (without leaks!). The webkit SVN WebNetscapePluginView.mm file contains:

[code]- (NPObject *)createPluginScriptableObject
{
if (![_pluginPackage.get() pluginFuncs]->getvalue || !_isStarted)
return NULL;

NPObject *value = NULL;
NPError error;
[self willCallPlugInFunction];
{
    JSC::JSLock::DropAllLocks dropAllLocks(JSC::SilenceAssertionsOnly);
    error = [_pluginPackage.get() pluginFuncs]->getvalue(plugin, NPPVpluginScriptableNPObject, &value);
}
[self didCallPlugInFunction];
if (error != NPERR_NO_ERROR)
    return NULL;

return value;

}
[/code]

…which is not too different to the Safari 420 build version of WebBaseNetscamePluginView.mm:

[code]- (NPObject *)createPluginScriptableObject
{
if (!NPP_GetValue || ![self isStarted])
return NULL;

NPObject *value = NULL;
NPError error;
[self willCallPlugInFunction];
{
    KJS::JSLock::DropAllLocks dropAllLocks;
    error = NPP_GetValue(plugin, NPPVpluginScriptableNPObject, &value);
}
[self didCallPlugInFunction];
if (error != NPERR_NO_ERROR)
    return NULL;

return value;

}
[/code]

Time to ask on a webkit board I guess… :?


#7

Yes, it’s a strange one!

I wrote it a while ago, and can’t remember exactly, but I seem to remember hitting problems with it, and I obviously came across that apple doc in searching for a fix…