Last Comment Bug 678474 - window.focus() doesn't work intermittently after multiple modal dialogs are opened quickly in succession
: window.focus() doesn't work intermittently after multiple modal dialogs are o...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla8
Assigned To: Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
:
Mentors:
Depends on:
Blocks: 626455
  Show dependency treegraph
 
Reported: 2011-08-12 05:06 PDT by Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
Modified: 2011-08-21 23:56 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
log output (34.95 KB, text/plain)
2011-08-14 03:07 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
no flags Details
patch v1 (890 bytes, patch)
2011-08-14 04:01 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
karlt: review+
Details | Diff | Splinter Review

Description Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-12 05:06:21 PDT
I have a test that fails >80% of the time on my local machine. It sometimes fails on try, too. The problem is that waitForFocus() times out because (nsIFocusManager.activeWindow == null) _but_ (gtk_window_is_active() == true). So window.focus() does nothing and no focus event is dispatched.

My testcase is a page with an onbeforeunload, an onunload and an onpagehide handler that shows a confirm() or alert() dialog. So when closing that tab there are three modal dialogs that I close on after the other. The test finishes but the mochitest framework hangs while waiting for focus.

I tried to understand how all this works (with tons of debug output) and guess that the following happens:

1.) a modal dialog is shown
2.) the modal dialog gets closed
3.) gdk_window_show_unraised() gets called in nsWindow::SetFocus()
4.) we wait for the "focus_in_event" signal
5.) the next modal dialog shows up, calls self.showModal() and that calls EnableParent(false)
6.) the main browser windows is now (mEnabled == false)
7.) the "focus_in_event" signal for the main window arrives _but_ the browser window is not enabled, so no "activate" event is dispatched
8.) we're now in a state where GTK says the window is raised but we think otherwise
9.) window.focus() doesn't work anymore
Comment 1 Karl Tomlinson (:karlt) 2011-08-13 01:36:58 PDT
Thanks for the analysis.  It sounds like we should not skip dispatch of the activate event when mEnabled is false.  That would be consistent with Enable(PR_FALSE) not doing anything to lose focus.

Are you able to confirm, please, that the the modal dialogs are not receiving focus before they are closed?

NSPR_LOG_MODULES=Widget:5,WidgetFocus:5 in the environment may be helpful.
Comment 2 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-14 03:07:35 PDT
Created attachment 552947 [details]
log output

(In reply to Karl Tomlinson (:karlt) from comment #1)
> Are you able to confirm, please, that the the modal dialogs are not
> receiving focus before they are closed?
> 
> NSPR_LOG_MODULES=Widget:5,WidgetFocus:5 in the environment may be helpful.

I hope the attached log file helps you. It's the complete run of the hanging test.

This looks suspicious, just before the test hangs:

###!!! ASSERTION: Uh, IsInModalState() called w/o a reachable top window?: 'Error', file /home/tim/workspace/mozilla-central/dom/base/nsGlobalWindow.cpp, line 6728

Is that possibly the result of the described erroneous state?
Comment 3 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-14 04:01:54 PDT
Created attachment 552954 [details] [diff] [review]
patch v1

(In reply to Karl Tomlinson (:karlt) from comment #1)
> Thanks for the analysis.  It sounds like we should not skip dispatch of the
> activate event when mEnabled is false.  That would be consistent with
> Enable(PR_FALSE) not doing anything to lose focus.

Is this what you had in mind?
Comment 4 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-14 10:56:23 PDT
Comment on attachment 552954 [details] [diff] [review]
patch v1

Passed try:

http://tbpl.mozilla.org/?tree=Try&pusher=tim.taubert@gmx.de&rev=30527793fbd0
Comment 5 Karl Tomlinson (:karlt) 2011-08-14 21:05:59 PDT
Comment on attachment 552954 [details] [diff] [review]
patch v1

Thanks.  The mEnabled test here replaced a similar test in bug 567704.
Bug 567704 comment 14 explains why I think it is unnecessary.
Comment 6 Karl Tomlinson (:karlt) 2011-08-14 21:08:33 PDT
(In reply to Tim Taubert [:ttaubert] from comment #3)
> This looks suspicious, just before the test hangs:
> 
> ###!!! ASSERTION: Uh, IsInModalState() called w/o a reachable top window?:
> 'Error', file
> /home/tim/workspace/mozilla-central/dom/base/nsGlobalWindow.cpp, line 6728
> 
> Is that possibly the result of the described erroneous state?

I don't know what that means, but it shows up after cause of the problem.
Comment 7 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-15 07:09:05 PDT
http://hg.mozilla.org/integration/fx-team/rev/621138727ae4
Comment 8 Rob Campbell [:rc] (:robcee) 2011-08-16 08:42:05 PDT
http://hg.mozilla.org/mozilla-central/rev/621138727ae4

Note You need to log in before you can comment on or make changes to this bug.