Bug 40036 – Make WebChromeClient::closeWindowSoon work for all RunLoop modes

• Status: RESOLVED
• Resolution: INVALID
• Component: WebKit Misc.

[Prasad Tammana]

performSelector:withObject:afterDelay method is being used to post the _closeWindow message to the queue.  Those message will only get dispatched if the RunLoop is in NSDefaultRunLoopMode.  Since RunLoop is in NSModalPanelRunLoopMode mode when a modal window is being displayed, you can't close the window.  Replaced the call with the inModes version to allow the close window message to be dispatched in all RunLoop modes.

[Prasad Tammana]

Created attachment 57611 [details]
Patch to make closeWindowSoon work in all RunLoop modes.

[David Levin]

This is follow up from http://www.mail-archive.com/[e-mail]/msg11394.html as  understand it.

[Darin Adler]

Comment on attachment 57611 [details]
Patch to make closeWindowSoon work in all RunLoop modes.

I don’t think this change is correct or helpful. For example, I don’t think we want to close a window and run all the code that triggers inside an NSConnectionReply modal event loop.

I don’t understand the benefit side of this patch either.

[Prasad Tammana]

(In reply to comment #3)
> (From update of attachment 57611 [details])
> I don’t think this change is correct or helpful. For example, I don’t think we want to close a window and run all the code that triggers inside an NSConnectionReply modal event loop.
> 
> I don’t understand the benefit side of this patch either.

When you write a WebKit application that implements showModalDialog, you don't have a way to close the window from script.  The close call ends up in the closeWindowSoon() function and the _closeWindow() doesn't get dispatched because the run loop is in NSModalPanelRunLoopMode mode.

I didn't realize that dispatching these calls is inappropriate in certain modes.  I could make this fix more targeted and make it to be:

[m_webView performSelector:@selector(_closeWindow) withObject:nil afterDelay:0.0
        inModes:[NSArray arrayWithObjects:NSDefaultRunLoopMode, NSModalPanelRunLoopMode]];

With that change the only new mode we'll be closing windows would be in NSModalPanelRunLoopMode.  Does that seem like a correct change?

I ran into this issue when adding showModalDialog to DumpRenderTree as part of the bug fix for - https://bugs.webkit.org/show_bug.cgi?id=35350.  You'll notice I have a patch attached to that bug that includes this change.  I decided to factor out this particular change into a separate patch as this is the more sensitive part of that big patch.


Thanks,
Prasad

[Dmitry Titov]

Comment on attachment 57611 [details]
Patch to make closeWindowSoon work in all RunLoop modes.

Removing r? since the root bug 35350 has landed and it seems a different approach was selected to support showModalDialog testing. Please update if this is not so.

[Prasad Tammana]

We don't need this change anymore.