Closed Bug 374229 Opened 17 years ago Closed 17 years ago

Crash [@ nsCOMPtr_base::assign_with_AddRef] when flash plugin gets removed during focus

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

x86
Windows XP
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(3 files)

See upcoming testcase, I crash with current trunk build when right-clicking on the flash embed. If it doesn't crash directly, try a reload and right-click again.

This seems to have regressed between 2006-10-04 and 2006-10-08:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-10-04+04&maxdate=2006-10-08+09&cvsroot=%2Fcvsroot
Regression from bug 344986?
Attached file testcase
Talkback ID: TB30300188X
0x00000001
nsCOMPtr_base::assign_with_AddRef  [mozilla/xpcom/build/nscomptr.cpp, line 89]
USER32.dll + 0x8709 (0x77d18709)
USER32.dll + 0x87eb (0x77d187eb)
USER32.dll + 0xb368 (0x77d1b368)
USER32.dll + 0xb3b4 (0x77d1b3b4)
ntdll.dll + 0xeae3 (0x7c90eae3)
USER32.dll + 0x93df (0x77d193df)
nsAppShell::ProcessNextNativeEvent  [mozilla/widget/src/windows/nsappshell.cpp, line 142]
nsBaseAppShell::DoProcessNextNativeEvent  [mozilla/widget/src/xpwidgets/nsbaseappshell.cpp, line 137]
0xc0850845
Assignee: events → mats.palmgren
Attached patch Patch rev. 1Splinter Review
The DispatchEvent call causes a reentrant focus event which invokes
the script which destroys the plugin instance which destroys 'this'.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/modules/plugin/base/src/nsPluginNativeWindowWin.cpp&rev=1.20&root=/cvsroot&mark=304-306#288
The bug is Windows only and trunk only AFAICT.

Holding a strong ref on the the plugin instance over the call fixes it -
we have one later in the function so I'm just moving that up a bit.

The ESM patch fixes a related assertion - NS_PLUGIN_ACTIVATE does not
require a frame and we should not assert on it (or return failure) in the
post event handler.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/events/src/nsEventStateManager.cpp&rev=1.683&root=/cvsroot&mark=2035-2036#2020
Attachment #258937 - Flags: review?(masayuki)
Blocks: 323142
Mats:

I'm not a good reviewer for the patch. I think that you should request the review to same people as bug 323142.
Attachment #258937 - Flags: review?(masayuki) → review?(emaijala)
Comment on attachment 258937 [details] [diff] [review]
Patch rev. 1

Just one nit: change "requires" to "require" in the comment in nsEventStateManager::PostHandleEvent.
r=emaijala
Attachment #258937 - Flags: review?(emaijala) → review+
Attachment #258937 - Flags: superreview?(roc)
Attachment #258937 - Flags: superreview?(roc) → superreview+
Comment nit fixed.
Checked in to trunk at 2007-03-24 02:48 PST.

-> FIXED
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I recognize there's no test architecture to do tests with plugins, but when there is, this should have one.
Flags: in-testsuite?
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070324 Minefield/3.0a3pre
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsCOMPtr_base::assign_with_AddRef]
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: