Make sure the plugin scriptable object is released before NPP_Destroy.

Make sure the plugin scriptable object is released before NPP_Destroy.

When the we tear down a plugin instance the plugin process first invokes NPP_Destroy, and then tears down the IPC channel to the renderer, to give NPP_Destroy a chance to do last-minute scripting.  When the IPC channel for the last instance is torn down we also clean up the IPC channels and stubs for any plugin-side NPObjects that remain.

We suspect that some plugins implement the scriptable object as part of the plugin instance, rather than independently ref-counted, so that our releasing the object after NPP_Destroy actually triggers the plugin process to crash.

This CL tears down the stub for the plugin's scriptable object before we call NPP_Destroy.

As per crbug.com/119414, we will remove this code if it doesn't significantly impact crashes.

BUG=101968
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128179

[Wez, 2012-03-21 23:31:32]

PTAL

If this proves to address the crashes we're seeing then I'll follow-up with the
a unit-test; if not then I'll probably back the change out.

[cpu_(ooo_6.6-7.5), 2012-03-22 00:09:28]

lgtm

adding john. If you get his lgtm you don't need to wait for Darin's.

http://codereview.chromium.org/9817023/diff/1/content/plugin/webplugin_delega...
File content/plugin/webplugin_delegate_stub.cc (right):

http://codereview.chromium.org/9817023/diff/1/content/plugin/webplugin_delega...
content/plugin/webplugin_delegate_stub.cc:291: // The stub will delete itself
when the proxy tells it that it's released, or
stale comment line 291

[Wez, 2012-03-22 00:42:20]

http://codereview.chromium.org/9817023/diff/1/content/plugin/webplugin_delega...
File content/plugin/webplugin_delegate_stub.cc (right):

http://codereview.chromium.org/9817023/diff/1/content/plugin/webplugin_delega...
content/plugin/webplugin_delegate_stub.cc:291: // The stub will delete itself
when the proxy tells it that it's released, or
On 2012/03/22 00:09:28, cpu wrote:
> stale comment line 291

Done.

[jam, 2012-03-22 00:55:07]

how do we know that it's the plugin scriptable object, and not any other objects
that we got from it?

[Wez, 2012-03-22 01:03:03]

On 2012/03/22 00:55:07, John Abd-El-Malek wrote:
> how do we know that it's the plugin scriptable object, and not any other
objects
> that we got from it?

We don't.  Whether this significantly reduces the crashes we see on Canary then
will help answer that.

[jam, 2012-03-22 01:21:00]

talked to wez, lgtm but please file a bug to track removing this code by m19/20
stable if things don't improve

[commit-bot: I haz the power, 2012-03-22 01:41:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/9817023/4001

[commit-bot: I haz the power, 2012-03-22 02:08:58]

Try job failure for 9817023-4001 (retry) on android for steps "Compile, build".
It's a second try, previously, steps "Compile, build" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

[commit-bot: I haz the power, 2012-03-22 04:44:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/9817023/18001

[commit-bot: I haz the power, 2012-03-22 06:39:48]

Change committed as 128179