183586 - Crash while showing context menu in Flash plugin on loading new page [@ ns4xPluginInstance::HandleEvent]

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Chris Petersen]

Build: 2002-12-04-04
Platform: OS X 10.2.2

Expected Results: Selecting a menu item in the contextual should do nothing.
What I got: The application crashes


Steps to reproduce:

1) Go to http://www.kartoo.com
2) Click on a previous saved bookmark 
3) Before that page loads, control click on the flash object and continue to
mouse down
4) The bookmarked page should load and the flash contextual menu should still be
present.
5) Now, select a menu item from the contextual menu.
6) The application should crash.

Comment 1

[Chris Petersen]

Attachment: stack trace

Comment 2

[rubydoo123]

does this only happen in chimera? how about other mac builds? how about 10.1.5?

Comment 3

[Chris Petersen]

Yes, this is a chimera only issue. Tested under both OS X 10.2.2 and OS X 10.1.5.

Comment 4

[thebeastwitheyesthatstared]

Chris / beppe: Does this still happen? If yes, have you contacted Macromedia?

Comment 5

[Chris Petersen]

Still occurs in the 2003-08-10-02 Camino NB.

Comment 6

[Chris Casciano]

Camino 2003-08-12-02 / Flash player 7b... crashes with same stack

Comment 7

[Peter Lubczynski]

The stack shows us crashing after calling HandleEvent through a mouseDown. 

Could someone with a Mac build could set a breakpoint during this call to
mouseDown and see if we are passing any bogus data to the plugin?

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

See also bug 183313, which has a similar but not identical stack for the same URL.

My most recent crash on that site failed to generate a Mac OS X crash log, but
Talkback ID is TB5392537W.  (I can't get it to crash again because now I'm
always getting the Camino contextual menu, bug 192276, for any Flash I attempt
to ctrl-click)

Comment 9

[Mike Pinkerton (not reading bugmail)]

any recent updates here?

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: 2005-era crashlog

I wonder if we could prevent this particular crash on the Camino end by fixing
bug 168065?

Not sure the stack is really much different, but here's a more modern one from
10.3.9 and yesterday's 0.9a2+ (1.8 branch) nightly....

Comment 11

[Simon Fraser [no longer active]]

I think the key here is that we're showing the plugin's context menu but tearing
down the plugin while loading the new page at the same time.

Comment 12

[Simon Fraser [no longer active]]

This stack demonstrates the bad stuff happening:

...
ns4xPluginInstance::Stop()
StopPluginInstance(PresShell*, nsIContent*)
PresShell::EnumeratePlugins(nsIDOMDocument*, nsString const&, void
(*)(PresShell*, nsIContent*))
PresShell::Freeze()
DocumentViewerImpl::Destroy()
DocumentViewerImpl::Show()
nsPresContext::EnsureVisible(int)
PresShell::UnsuppressAndInvalidate()
PresShell::UnsuppressPainting()
DocumentViewerImpl::LoadComplete(unsigned)
nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, unsigned)
nsWebShell::EndPageLoad(nsIWebProgress*, nsIChannel*, unsigned)
nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned, unsigned)
nsDocLoader::FireOnStateChange(nsIWebProgress*, nsIRequest*, int, unsigned)
nsDocLoader::doStopDocumentLoad(nsIRequest*, unsigned)
nsDocLoader::DocLoaderIsEmpty()
nsDocLoader::DocLoaderIsEmpty()
nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned)
nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, unsigned)
PresShell::RemoveDummyLayoutRequest()
HandleDummyLayoutRequestPLEvent(PLEvent*)
PL_HandleEvent
PL_ProcessPendingEvents
__CFRunLoopDoSources0
__CFRunLoopRun
CFRunLoopRunSpecific
RunCurrentEventLoopInMode
ReceiveNextEventCommon
AcquireNextEventInMode
IsUserStillTracking(MenuSelectData*, unsigned char*)
TrackMenuCommon(MenuSelectData&, unsigned char*)
PopUpMenuSelectCore(MenuData*, Point, double, Point, GDevice**, Rect const*,
unsigned short, unsigned long, Rect const*, Rect const*, __CFString const*,
OpaqueMenuRef**, unsigned short*)
PopUpMenuSelect
0x8316f7c
0x830a27c
0x8309be0
ns4xPluginInstance::HandleEvent(nsPluginEvent*, int*)
nsPluginInstanceOwner::ProcessEvent(nsGUIEvent const&)
nsPluginInstanceOwner::MouseDown(nsIDOMEvent*)
DispatchToInterface(nsIDOMEvent*, nsIDOMEventListener*, unsigned
(nsIDOMEventListener::*)(nsIDOMEvent*), nsID const&, int*)
nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**,
nsIDOMEventTarget*, unsigned, nsEventStatus*)
nsGenericElement::HandleDOMEvent(nsPresContext*, nsEvent*, nsIDOMEvent**,
unsigned, nsEventStatus*)
PresShell::HandleEventInternal(nsEvent*, nsIView*, unsigned, nsEventStatus*)
PresShell::HandleEvent(nsIView*, nsGUIEvent*, nsEventStatus*, int, int&)
nsViewManager::HandleEvent(nsView*, nsGUIEvent*, int)
nsViewManager::DispatchEvent(nsGUIEvent*, nsEventStatus*)
HandleEvent(nsGUIEvent*)
nsChildView::DispatchEvent(nsGUIEvent*, nsEventStatus&)
nsChildView::DispatchWindowEvent(nsGUIEvent&)
nsChildView::DispatchMouseEvent(nsMouseEvent&)
-[ChildView rightMouseDown:]
-[NSWindow sendEvent:]
-[BrowserWindow sendEvent:]
-[NSApplication sendEvent:]
-[NSApplication run]
NSApplicationMain
_start
start

We're inside the plugin's call to PopUpMenuSelect() processing PLEvents (via a
CFRunLoopSource), and doing teardown of the plugin while it's click handling
code is still on the stack.

I'm not sure whether the browser or the plugin should be the one to stop this
happening -- Michelle?

Comment 13

[Michelle Sintov]

If you try to reproduce this in Mac Firefox 1.0, you can see that the browser
will not move off of the kartoo page until the mouse is released. If Camino
could do this as well that would fix this bug, I think.

Comment 14

[Simon Fraser [no longer active]]

Firefox 1.5 will behave the same way that camino does now (CFRunLoop fallout).

Comment 15

[Michelle Sintov]

I think that the browser should not unload the plug-in until the mouse is
released. This is the behavior I see in other browsers. Also, this is what
Samuel Sidler proposed in bug 168065.

Comment 16

[Adam Guthrie]

*** Bug 310308 has been marked as a duplicate of this bug. ***

Comment 17

[Samuel Sidler (old account; do not CC)]

(In reply to comment #15)
> I think that the browser should not unload the plug-in until the mouse is
> released. This is the behavior I see in other browsers.

Is this something we can have done by 1.0? If it can/will fix a crash, I think it's worth it even if we want to go another route after 1.0.

Comment 18

[Simon Fraser [no longer active]]

We should investigate not firing PLEvents for certain runloop modes.

Comment 19

[Mike Pinkerton (not reading bugmail)]

does this crash in ff as comment 14 suggests? If so, i can't see this happening on the branch soon with the lockdown. also, should this be re-directed to core?

Comment 20

[Mark Mentovai]

Yeah, this crashes Firefox too.

Comment 21

[Mike Pinkerton (not reading bugmail)]

-> core

Comment 22

[Asa Dotzler [:asa]]

Josh, can you look into this and help us understand how severe this is and how risky a fix would be?

Comment 23

[Asa Dotzler [:asa]]

Josh, if by code inspection, you can find a way that this might be more common than the steps provided in comment 0

Comment 24

[Marcia Knous [:marcia]]

I just filed a bug which seems to have a similar stack to this one - (contains ns4xPluginInstance::HandleEvent(nsPluginEvent) - Bug 314558. I didn't get talkback but I did get two apple reports which I attached to that bug. 

Comment 25

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: crashlog from trunk debug build

FWIW, I've got an easier way to repro this (without the timing issues of loading the bookmark vs the context menu in comment 0):

1. Go to http://online.tvguide.com/listings/grid.asp
2. Click on the next or previous arrows to cause the grid to reload
3. Ctrl-click on the "arrow" animation to bring up the Flash CM
4. After the new grid has loaded, click elsewhere in the page to make the CM close.
5. Camino crashes.

Comment 26

[Boris Zbarsky [:bzbarsky]]

So wait.  Is the problem that we're spinning the Gecko event loop here when Gecko doesn't expect it to spin, basically?  This has come up before, no?

Comment 27

[Michelle Sintov]

Can you reproduce this bug with Flash Player 9r16 on Mac PPC? If yes, could you clarify if you are following the steps to repro in comment #1 or other steps? The tvguide site in comment #25 no longer appears to use the Flash Player.

Comment 28

[Wayne Mery (:wsmwk)]

bug 319857, which may be a dup, suggests this was fixed by threadmanager checkin, in which case it should work with a trunk build after 2006-5-10.

Do you still see this bug with a current trunk build?  
Or http://archive.mozilla.org/pub/firefox/nightly/2006-05-12-06-trunk/

Comment 29

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This appears "fixed" on the trunk because the "fix" is actually a symptom of bug 356720, since clicks to any native stuff (menus, scrollbars, context menus) stops Gecko entirely.  We need to make sure this "fix" doesn't regress when bug 356720 is fixed.

Comment 30

[Damon Sicore (:damons)]

Josh, can you please verify this bug?  Let's get this moving if need be.

Comment 31

[Marcia Knous [:marcia]]

I don't see any crash using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007100904 Minefield/3.0a9pre and r47 flash and using r60 flash.

Comment 32

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Marking fixed, but we need to get it verified.

Comment 33

[Carsten Book [:Tomcat]]

verified fixed using the testurls from this bug and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008020601 Firefox/3.0b4pre with flash r115 - no crash --> Verified fixed