621099 - plugins.unloadASAP set to true no longer unloads plugin-container.exe after closing tab that was using plugin

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Chris]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101222 Firefox/4.0b9pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101222 Firefox/4.0b9pre

I have plugins.unloadASAP set to true to unload plugin-container.exe on closing of a tab that was running a plugin.  I testes hourly builds to find when it broke.  It worked as of 1292806595-20101219165635-150af817b65d-firefox-4.0b9pre.en-US.win32.zip 19-Dec-2010, and broke on 1292809339-20101219174219-302d1d3e2817-firefox-4.0b9pre.en-US.win32.zip 19-Dec-2010.

Reproducible: Always

Steps to Reproduce:
1. Set plugins.unloadASAP to true
2. load a page that uses a plugin (youtube, etc.)
3. close tab
4. check Windows task manager
Actual Results:  
plugin-container.exe no longer closes when the tab using it is closed.

Expected Results:  
plugin-container.exe should close with the tab that was using it.

Comment 1

[Alice0775 White]

Confirmed on
Mozilla/5.0 (X11; Linux i686; rv:2.0b9pre) Gecko/20101222 Firefox/4.0b9pre ID:20101222030351

In local build:
build from ade671d15514 : fails
build from 955ba94047fd : works

Candidate ade671d15514	Robert O'Callahan — Bug 617152. Part 5: nsPluginHost::StopPluginInstance should skip plugin instances that are already destroyed (e.g. because the plugin was disabled). r=josh

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks a ton Alice!

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, so Part 5 in bug 617152 is definitely causing this. And the core issue probably triggered the crash regressions from bug 617152 as well.

The problem is that DoStopPlugin calls inst->Stop() followed by pluginHost->StopPluginInstance(inst). The call to Stop() sets mRunning to DESTROYED, and with my patch, StopPluginInstance(inst) then returns early without doing anything.

I have to say it's confusing that DoStopPlugin calls Stop() and StopPluginInstance does too...

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Fixed, see https://bugzilla.mozilla.org/show_bug.cgi?id=620794#c4