[Webkit-unassigned] [Bug 35350] DumpRenderTree should allow tests with modal dialogs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=35350


Prasad Tammana <prasadt at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |atwilson at chromium.org




--- Comment #7 from Prasad Tammana <prasadt at chromium.org>  2010-06-14 13:31:33 PST ---
(In reply to comment #6)
> The override of NSApplication should be done in DumpRenderTree instead of changing WebKit to make DumpRenderTree work. If we later decide to change NSApplication then we can remove the code from DumpRenderTree.

I tried this but I'm seeing random crashes in runModalCleanup after calling abortModal.  I need to call abortModal to get out of the runModalForWindow call.  I looked at with a "local Mac expert" but we couldn't figure why this would happen.  My broad conclusion is that NSApplication pretending to be in normal loop mode when its not, is interacting badly with the cleanup code from runModalForWindow.  Unscientific, but that's as far as I could go in terms of analyzing this random crash.

Please see below for more comments on your response.

(In reply to comment #5)
> (From update of attachment 57603 [details])
> > -    [m_webView performSelector:@selector(_closeWindow) withObject:nil afterDelay:0.0];
> > +    [m_webView performSelector:@selector(_closeWindow) withObject:nil afterDelay:0.0
> > +        inModes:[NSArray arrayWithObjects:NSDefaultRunLoopMode, NSModalPanelRunLoopMode,
> > +                 NSConnectionReplyMode, NSEventTrackingRunLoopMode, nil]];
> 
> This change is not needed to make showModalDialog work in DumpRenderTree. Making this change could affect the behavior of applications on Mac and so the decision of whether to change it should not be driven by the needs of our test tool alone.

I tried several things but couldn't find any other way to close a modal dialog from script.  As you mentioned above, this affects the behavior of apps on Mac which is why I factored this one change into a separate patch to get a focused review on just this - https://bugs.webkit.org/show_bug.cgi?id=40036.  You reviewed that and gave me feedback to which I responded.  I haven't seen your response to my last set of comments.  Not sure if you had a chance to look at that.  We can continue discussion on that in that bug, so we don't duplicate that here.  I gave detailed comments on why showModalDialog needs this change, to support being able to close from script.

Its a whole another thing if you think we shouldn't be able to close showModalDialog from script, in which case, I'll can to go with my plan B to make this work and upload a new patch.  My plan B is to add abort method to LayoutTestController and invoke that from my test script.  That was my initial approach and I implemented that but switched to what I have now following feedback from a team mate who thought we should fix this in WebKit instead of working around it in LayoutTestController.

> The rest of the patch seems OK.

Thanks for reviewing this somewhat lengthy patch.  Happy to see that I'm getting close to being able to land this.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.