Patch: Crash: blocking JS alert and deleting a window

Sat Oct 19 13:19:49 BST 2002

On Sat, 19 Oct 2002, David Faure wrote:

> On Friday 18 October 2002 23:18, Koos Vriezen wrote:
> > Unfortunately removing topLevelWidget() in ecma/kjs_window makes
> > 'setTimeout('self.close()',1000); alert("Hi");' crash again, better leave
> > it then
>
> Hmm, why? Message boxes created by Window::Alert use the
> KHTMLView as parent widget, and there's only one khtmlview in this case......
>
> (or are you talking about the testcase that used window.open?)

Turns out removing topLevelWidget() does work, but changing hide() for
close() does reintroduce the crash. Not sure why,
QWidget::close(bool alsoDelete) calls QApplication::sendEvent(this, &e), e
being QCloseEvent, QDialog::closeEvent(QCloseEvent *e) calls reject() that
calls done(int r) which calls hide(). QApplication::sendEvent does call
QDialog::closeEvent without reschedule, no?
Hmmm, (qt-copy/src/kernel/qwidget.cpp:3944)
        QApplication::sendEvent( this, &e );
        deleted = !QWidget::find(id);
        if ( !deleted && !e.isAccepted() ) {
            is_closing = 0;
            return FALSE;
        }
        if ( !deleted )
            hide();
does QWidget::find return false here.

> > (can JS close a frame?).
>
> Not a frame, but a Konqueror view in a splitted window.
> Let me detail this:
> those two setups are very different:
> * One KHTMLPart in the Konqueror view, with frames inside
> * A splitted Konqueror window (using Konqueror's Split options),
> with two KHTMLParts.
> In the latter case, it's really like having two windows side by side.
> window.close() should only destroy one of the views (I believe this
> works).

Yes, indeed.

> > +void KHTMLView::closeChildDialogs()
> > +{
> > +    QObjectList *dlgs = queryList("QDialog");
> > +    for (QObject *dlg = dlgs->first(); dlg; dlg = dlgs->next())
> > +        static_cast<QWidget*>(dlg)->close();
> > +    delete dlgs;
> >  }
>
> I prefer that one, who knows if we won't have to call it from
> another place.

Yes, later I thought having a central place for this, can be usefull for
not deleting dialogs not created by JS (eg. the find dialog).

> Dirk wrote:
> > Am I the only one who is scared like hell when I see such code ?
>
> Yes, well, obviously Qt wasn't designed with this (very unusual) case in mind.
> Do you know any other app where a window or widget pops up a modal dialog,
> and then, before the user selects something in it, the window or widget is being
> destroyed? The "enter_loop" done by the modal dialog has to be exited at
> the right time - which isn't in the QWidget destructor of the parent (it's far too late,
> the control will go back into the code of the deleted widget).

Yes, additional look at the stack:
  <html><head><script>
    setTimeout('self.close()',1000);alert(somevar);
  </script></head><body></body></html>
0 main_message_loop
1 ..
2 tokenizer execute JS
3 KMessageBox::exec
4 dialog_message_loop
5 timer event
6 KHTMLPart::~KHTMLPart -> delete m_view -> closes child dialogs
7 KHTMLPart::clear -> KJSProxy::clear() -> DocumentImpl::detach

If the stack pops back, a crash occurs in the tokenizer because everyting
is deleted. Doing a hide() makes QDialog::in_loop become FALSE, so it
return from 4 and 3, which makes 2 being able to finish before
KHTMLPart::~KHTMLPart is called.

> Dirk also wrote:
> > you actually have to revert a workaround I committed yesterday to avoid the
> > crash..
> Koos: I believe this is about the !mimeName.isEmpty() in render_frames.cpp:811.

Ok reverted that. Indeed I get an do-you-want-to-download-xxx message
box, but only one and clicking no doesn't make it crash.

Koos