User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.1b) Gecko/20020912 Build Identifier: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.1b) Gecko/20020912 window.close() by FSCommand crashes Mozilla (Flash 6) Reproducible: Always Steps to Reproduce: 1. Install Flash 6.0 plugin and enable FSCommand by copying flashplayer.xpt in Mozilla plugin directory (see bug 159393). 2. Go to http://www.nhk.or.jp/henkaku/interview_special/sakamoto.html 3. Click "->" button in the right bottom, and more 2 times. 4. Click the "close" button. Actual Results: Mozilla crashes. Expected Results: The window should close, or Mozilla should not crash at least. Confirming with 2002091808-trunk/MacOS 9.2 (Flash 6.0 r40) and 2002091808-trunk/Win98SE (Flash 6.0 r47). Talkback ID on Win98SE is TB11268888H, and Macsbug's log is http://bugzilla.mozilla.gr.jp/showattachment.cgi?attach_id=1158 This "close" button calles FSCommand (see the HTML source). If the button calles javascript:window.close() by "Get URL", Mozilla doesn't crash.

normal -> critical

confirmed trace: http://climate.netscape.com/reports/SingleIncidentInfo.cfm?dynamicBBID=11275644

assigning to Anthony for debugging

Accepting. Will look into this.

delays the whole destruction of the frames of the nsObectFrame and is nsxpcwrapper through use of a new PLCloseEvent. Johnny needs to take a look, as does possibly jband.

also marking this bug dependent on Peter L's bug 90268 which should handle any other synchronous destruction of frames via plugin invoked JS.

I had the same problem with Mozilla 1.1 (and NS7) on Mac OSX with plugin version 6.0 r47

looking for some r= and sr= to get this crasher fix checked in. cc'ng jband as he may be interested in this too.

Comment on attachment 100502 [details] [diff] [review] patch beta r=peterl

still need a sr, jst or jband?

is this a dup of bug 136927?

It's similar to that bug and also to bug 90268. In this bug, Flash causes NPP_Destroy while in it's own scripting stack and it can't take that.

yeah, I see, but w/ this patch (attachment 100502 [details] [diff] [review])& patch (attachment 100631 [details] [diff] [review]) for bug bug 132759, I'm getting "Illegal Operation in Plugin" dialog w/ stack trace: USER32! NtUserWaitMessage@0 + 11 bytes nsAppShell::GetNativeEvent(nsAppShell * const 0x03db3690, int & 0x00000001, void * & 0x02271e80 msg) line 249 nsXULWindow::ShowModal(nsXULWindow * const 0x03e07d80) line 295 + 31 bytes nsWebShellWindow::ShowModal(nsWebShellWindow * const 0x03e07d80) line 1109 nsContentTreeOwner::ShowAsModal(nsContentTreeOwner * const 0x040704dc) line 449 nsWindowWatcher::OpenWindowJS(nsWindowWatcher * const 0x01a47444, nsIDOMWindow * 0x0364f3dc, const char * 0x0181aa00, const char * 0x0181b100, const char * 0x0181b0dc, int 0x00000001, unsigned int 0x00000001, long * 0x040b60e8, nsIDOMWindow * * 0x0012ea60) line 782 nsWindowWatcher::OpenWindow(nsWindowWatcher * const 0x01a47440, nsIDOMWindow * 0x0364f3dc, const char * 0x0181aa00, const char * 0x0181b100, const char * 0x0181b0dc, nsISupports * 0x040709d8, nsIDOMWindow * * 0x0012ea60) line 459 + 48 bytes nsPromptService::DoDialog(nsPromptService * const 0x01bd0f1c, nsIDOMWindow * 0x0364f3dc, nsIDialogParamBlock * 0x040709d8, const char * 0x0181aa00) line 629 + 77 bytes nsPromptService::ConfirmEx(nsPromptService * const 0x01bd0f18, nsIDOMWindow * 0x00000000, const unsigned short * 0x04093090, const unsigned short * 0x040db3b0, unsigned int 0x00000000, const unsigned short * 0x00000000, const unsigned short * 0x00000000, const unsigned short * 0x00000000, const unsigned short * 0x0404c960, int * 0x0012edc8, int * 0x0012edcc) line 347 + 36 bytes nsPrompt::ConfirmEx(nsPrompt * const 0x040c3d50, const unsigned short * 0x04093090, const unsigned short * 0x040db3b0, unsigned int 0x00000001, const unsigned short * 0x00000000, const unsigned short * 0x00000000, const unsigned short * 0x00000000, const unsigned short * 0x0404c960, int * 0x0012edc8, int * 0x0012edcc) line 168 nsPluginHostImpl::HandleBadPlugin(nsPluginHostImpl * const 0x015911ac, PRLibrary * 0x00000000, nsIPluginInstance * 0x03e05138) line 6108 + 84 bytes PluginWndProc(HWND__ * 0x00161464, unsigned int 0x00000202, unsigned int 0x00000000, long 0x0160025e) line 207 + 220 bytes USER32! UserCallWinProc@20 + 24 bytes USER32! DispatchMessageWorker@8 + 244 bytes USER32! DispatchMessageW@4 + 11 bytes nsAppShell::Run(nsAppShell * const 0x01a637d8) line 163 nsAppShellService::Run(nsAppShellService * const 0x01555d20) line 472 main1(int 0x00000001, char * * 0x00277720, nsISupports * 0x00000000) line 1508 + 32 bytes main(int 0x00000001, char * * 0x00277720) line 1869 + 37 bytes mainCRTStartup() line 338 + 17 bytes ------- and than crash w/ stack trace: XPCNativeSet::MarkSelfOnly() line 1289 + 20 bytes XPCNativeSet::Mark() line 545 XPCJSRuntime::GCCallback(JSContext * 0x03ed7138, JSGCStatus JSGC_FINALIZE_END) line 357 DOMGCCallback(JSContext * 0x03ed7138, JSGCStatus JSGC_FINALIZE_END) line 1641 + 23 bytes js_GC(JSContext * 0x03ed7138, unsigned int 0x00000000) line 1374 + 12 bytes js_ForceGC(JSContext * 0x03ed7138, unsigned int 0x00000000) line 993 + 13 bytes JS_GC(JSContext * 0x03ed7138) line 1659 + 11 bytes nsJSContext::Notify(nsJSContext * const 0x03ed70c8, nsITimer * 0x03ed38d8) line 1594 + 13 bytes nsTimerImpl::Fire() line 371 nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x01abd610) line 591 nsAppShell::Run(nsAppShell * const 0x01a637d8) line 173 nsAppShellService::Run(nsAppShellService * const 0x01555d20) line 472 main1(int 0x00000001, char * * 0x00277720, nsISupports * 0x00000000) line 1508 + 32 bytes main(int 0x00000001, char * * 0x00277720) line 1869 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! BaseProcessStart@4 + 115547 bytes

hmm, w/o the patch (attachment 100502 [details] [diff] [review]) I'm getting the same crash scenario:(

ccing av, this bug mutated after window subclassing

Can you explain this, serge? Also, you can try to comment out sublass/unsubclass lines in |nsPluginNativeWindowWin::CallSetWindow| and see if 'mutation' goes away.

the stack traces I posted are from my 20020926 w2k debug build, it's a little bit old, today release build crash on me w/ the original stack trace

This doesnt crash for me with today's build on win2k with my patch. It seems to work just fine.

I don't crash without your patch, on XP. It does not close the window though.

Are you sure you have FS Commands turned on?

I'm getting the same crash I mentioned in comment 13 w/ tony's patch on w2k 20021001 debug build. NOTE: the crash happens only if more than 1 tabs are present.

av, make sure you are using flash 6 plugin

I have just updated the tree and yes, I do have Flash 6. Anything else I can try to reproduce it?

Looks like the latest findings from Serge and me show that 100%CPU patch is likely not relevant. I confirm that the patch in this bug fixes the crash in case of separate windows but it still crashes with tabs.

yes i had two tabs..and I reproduced this crash on 1021 trunk today.

well, I could repro with only one tab present as well.

I havent checked the patch in yet. ;-)

;) i was just passing by this bug in my triage train...that's why I commented. Thx! :)

still waiting for sr= on this crasher. anyone?

I crash Mozilla using onClick="window.close();" from full frame I have no plugins

p.s. to above. These produced TB13984106Y & TB13984019K p.p.s Four bugs are listed after a search for : window.close bug . Could these be focussed.

Alan, thanks for your input, TB reports you submitted is generated by MozillaTrunk Build ID 2002101612 which is kind of old. I mean, I cannot reproduce the crash w/ latest build on url you reported www.alanfirminger.freeserve.co.uk/test/scrollbars2.html You are right there are several bugs w/ window.close in summary http://bugzilla.mozilla.org/buglist.cgi?query_format=&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&changedin=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=short_desc&type0-0-0=substring&value0-0-0=window.close maybe your case belongs to some of those bugs, but this particular bug is about window.close() by FSCommand from flash plugin, which I cannot reproduce w/ latest build anymore. Tony, could you try to repro this, please.

Yeah, there are several bugs of this nature.

Should I respond to #32 by submitting my comments elsewhere or opening another window.close bug ? See comments to 177752 #7 Alan

bug 177752 #7

Anthony, you will need to relook at the patch (bit rot), get it reviewed and checked in to resolve this crash

peterl

Changing to All/All. The URL is dead. I made a testcase.

Okay, here's the long and twisty stack causing this crash with a breakpoint set on ::DestroyWindow(mWnd): nsWindow::Destroy(nsWindow * const 0x03f0ad0c) line 1710 nsView::~nsView() line 166 nsView::`scalar deleting destructor'(unsigned int 0x000000 nsView::Destroy(nsView * const 0x03f0ac60) line 258 + 34 b nsFrame::Destroy(nsFrame * const 0x0371f028, nsIPresContex nsSplittableFrame::Destroy(nsSplittableFrame * const 0x037 nsContainerFrame::Destroy(nsContainerFrame * const 0x0371f nsBoxFrame::Destroy(nsBoxFrame * const 0x0371f028, nsIPres nsMenuPopupFrame::Destroy(nsMenuPopupFrame * const 0x0371f nsPopupSetFrame::Destroy(nsPopupSetFrame * const 0x035289c nsFrameList::DestroyFrames(nsIPresContext * 0x02f63418) li nsContainerFrame::Destroy(nsContainerFrame * const 0x03528 nsBoxFrame::Destroy(nsBoxFrame * const 0x035288b8, nsIPres nsFrameList::DestroyFrames(nsIPresContext * 0x02f63418) li nsContainerFrame::Destroy(nsContainerFrame * const 0x03528 nsBoxFrame::Destroy(nsBoxFrame * const 0x035286dc, nsIPres nsFrameList::DestroyFrames(nsIPresContext * 0x02f63418) li nsContainerFrame::Destroy(nsContainerFrame * const 0x03528 ViewportFrame::Destroy(ViewportFrame * const 0x035285d4, n FrameManager::Destroy(FrameManager * const 0x03480c50) lin PresShell::Destroy(PresShell * const 0x0345e750) line 1813 DocumentViewerImpl::Destroy(DocumentViewerImpl * const 0x0 nsDocShell::Destroy(nsDocShell * const 0x035e5134) line 29 nsWebShell::Destroy(nsWebShell * const 0x035e5134) line 12 nsXULWindow::Destroy(nsXULWindow * const 0x0335f0c0) line nsWebShellWindow::Destroy(nsWebShellWindow * const 0x0335f nsContentTreeOwner::Destroy(nsContentTreeOwner * const 0x0 GlobalWindowImpl::ReallyCloseWindow(GlobalWindowImpl * con GlobalWindowImpl::Close(GlobalWindowImpl * const 0x03f152a XPTC_InvokeByIndex(nsISupports * 0x03f152a4, unsigned int XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWr XPC_WN_CallMethod(JSContext * 0x029b2c28, JSObject * 0x036 js_Invoke(JSContext * 0x029b2c28, unsigned int 0x00000000, js_Interpret(JSContext * 0x029b2c28, long * 0x0012e92c) li js_Execute(JSContext * 0x029b2c28, JSObject * 0x035d0ac0, obj_eval(JSContext * 0x029b2c28, JSObject * 0x036c8948, un js_Invoke(JSContext * 0x029b2c28, unsigned int 0x00000001, js_Interpret(JSContext * 0x029b2c28, long * 0x0012f248) li js_Invoke(JSContext * 0x029b2c28, unsigned int 0x00000001, nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJSClass * cons nsXPCWrappedJS::CallMethod(nsXPCWrappedJS * const 0x04003b PrepareAndDispatch(nsXPTCStubBase * 0x04003be8, unsigned i SharedStub() line 139 NPSWF32! 0503b728() NPSWF32! 050388a0() NPSWF32! 0502ec85() NPSWF32! 05029e3e() NPSWF32! 05022d5a() NPSWF32! 050234e7() NPSWF32! 05039c3c() NPSWF32! 05037f5c() USER32! 77d67b17() USER32! 77d6cdce() USER32! 77d45cc9() USER32! 77d45ce8() PluginWndProc(HWND__ * 0x000705b0, unsigned int 0x00000202 USER32! 77d67b17() USER32! 77d6cdce() USER32! 77d44435() USER32! 77d49611() nsAppShellService::Run(nsAppShellService * const 0x0110b24 main1(int 0x00000001, char * * 0x002a78c0, nsISupports * 0 main(int 0x00000001, char * * 0x002a78c0) line 1904 + 37 b mainCRTStartup() line 338 + 17 bytes Note 0x00000202 in PluginWndProc is the WM_LBUTTONUP. We are destroying the plugin window while inside their winproc. attachment 100502 [details] [diff] [review] does fix this crash but I'd like jst to comment on the approach

Topcrash on Linux for M140A. Adding topcrash+ to keywords. The only comment to add from recent crashes: (19010236) Comments: invoking javascript "window.close()" via fscommand from Flash6 object embedded in a popup window

Comment on attachment 100502 [details] [diff] [review] patch beta +struct nsCloseEvent: public PLEvent { Nit of the day: Space before ':' :-) + nsCloseEvent (GlobalWindowImpl *aImpl); Given that this method is a one-liner, you might as well inline it as: + nsCloseEvent(GlobalWindowImpl *aWindow) + : mGWImpl(aWindow) + { + } + GlobalWindowImpl* mGWImpl; Make this a strong reference (nsRefPtr<GlobalWindowImpl>) and name it mWindow. +nsresult PostCloseEvent (GlobalWindowImpl* aImpl) Wouldn't it make more sense to make this a member of GlobalWindowImpl? And put the declaration of the return type on its own line. +{ + nsCOMPtr<nsIEventQueueService> eventService(do_GetService(kEventQueueServiceCID)); + if (eventService) { + nsCOMPtr<nsIEventQueue> eventQueue; + eventService->GetThreadEventQueue(PR_GetCurrentThread(), getter_AddRefs(eventQueue)); + if (eventQueue) { + nsCloseEvent * ev = new nsCloseEvent(aImpl); if (!ev) return NS_ERROR_OUT_OF_MEMORY, or set an |rv| variable and return it at the end. + if (ev) { + + PL_InitEvent(ev, nsnull, (PLHandleEventProc) ::HandleCloseEvent, (PLDestroyEventProc) ::DestroyCloseEvent); ... // If we get past the above we're closing the window right now. - return ReallyCloseWindow(); + PostCloseEvent(this); } + NS_IMETHODIMP GlobalWindowImpl::ReallyCloseWindow() No need to add one more empty line there... Same goes after PostCloseEvent(). And don't you still need that "return" there? And only call PostCloseEvent() if !IsCallerChrome(), when called from chrome, we want to close the window asap. Fix that, and I'll have one more look.

here's the updated patch

Comment on attachment 121351 [details] [diff] [review] patch v.2 +struct nsCloseEvent : public PLEvent { + nsCloseEvent (GlobalWindowImpl *aWindow) { mWindow = aWindow; } Use 2-space indentation, and change the initialzation of mWindow to: + nsCloseEvent(GlobalWindowImpl *aWindow) + : mWindow(aWindow) + { + } To avoid execute unnecessary code to first initialze mWindow to 0, then to set it to aWindow. sr=jst with those changes.

Comment on attachment 121351 [details] [diff] [review] patch v.2 Why use FAILED(rv) in one place and NS_FAILED(rv) in another? Seems like the second use should be NS_FAILED(rv) too. Besides that and that 2-space thing, r=me.

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

oops...good catch...here's the updated patch

Comment on attachment 121689 [details] [diff] [review] patch v.2.1 requesting approval for 1.4b: This patch will cause frame destruction from the event queue when window.close() is called from Javascript.

Comment on attachment 121689 [details] [diff] [review] patch v.2.1 a=asa (on behalf of drivers) for checkin to 1.4b

patch in trunk, marking FIXED. I've opened bug 203401 to possibly find a better solution to this problem after bug 90268 is fixed.