RESOLVED FIXED 30549
[Qt] Infinite loop (leading to crash) when setting cursor in QGraphicsWebView 
https://bugs.webkit.org/show_bug.cgi?id=30549
Summary [Qt] Infinite loop (leading to crash) when setting cursor in QGraphicsWebView
Antonio Gomes
Reported 2009-10-19 19:26:10 PDT
The http://trac.webkit.org/changeset/49782 commit has reveal a bug in QGWV's setCursor method. * Problem (manual backtrace): _1) Widget[Qt]::setCursor _2) QWebPageClient::setCursor _3) QGraphicsWebViewPrivate::updateCursor _4) QGraphicsItem::setCursor. _5) QGraphicsWidget::itemChange method is called as the first action of qgraphicsitem.cpp::setCursor (see qt/src/gui/graphicsview/qgraphicsitem.cpp). _6) QGraphicsWidget::itemChange fires QApplication::sendEvent(CursorChange), that is captured by QGWV's ::event. At this point QGWV cursor has not been set yet (remember we are in the middle of _3_ yet). _7) the quote below is executed: void QGraphicsWebView::event() { (...) if (event->type() == QEvent::CursorChange) { if (cursor().shape() == Qt::ArrowCursor) d->resetCursor(); } (...) } _8) d->resetCursor goes to 3) again, in an infinite loop ....... crash ! Solution: When QGraphicsItem::setCursor calls QGraphicsWidget::itemChange as its first action, it passes 'CursorChange' as 'change'. However we can not emit 'CursoChange' event yet (see loop above). At the end of setCursor method (when cursor had already been set), QGraphicsWidget::itemChange method is called again, but now passing the 'CursorHasChanged' as 'change'. This is the time we have to act in QGraphicsWebView. patch coming ...
Attachments
(commit in r49846) patch 0.1 - late emission of CursorChange event. (3.53 KB, patch)
2009-10-19 19:30 PDT, Antonio Gomes 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Antonio Gomes
Comment 1 2009-10-19 19:30:54 PDT
Created attachment 41470 [details] (commit in r49846) patch 0.1 - late emission of CursorChange event.
Antonio Gomes
Comment 2 2009-10-19 19:32:40 PDT
QWebView works because QWidget emits the CursorChange event just like the proposed patch does (at the end of the setCursor method): void QWidget::setCursor(const QCursor &cursor) { Q_D(QWidget); (...) setAttribute(Qt::WA_SetCursor); d->setCursor_sys(cursor); QEvent event(QEvent::CursorChange); QApplication::sendEvent(this, &event); }
Kenneth Rohde Christiansen
Comment 3 2009-10-20 01:00:00 PDT
Seems like a workaround. Shouldn't this be fixed in Qt instead? Right now it seems that with QWidgets, the event is being called after the cursor has been set, and that that is not the case with QGraphicsWidgets. That is different behaviour and if that is not intentional (which I guess not), it can be considered a bug.
Antonio Gomes
Comment 4 2009-10-20 02:55:45 PDT
Comment on attachment 41470 [details] (commit in r49846) patch 0.1 - late emission of CursorChange event. clearing r+ flag since patch has landed.
Antonio Gomes
Comment 5 2009-10-20 02:58:46 PDT
as i talked to ariya on irc and pointed out in bug description and comment #2, i agree w/ kenneth that it has to be investigated in qt side about why such behaviour, however i do not need we have to be crashy until then. So i am landing the fix/workaround and filing to followup bugs: 1) one for auto test this 2) for investigating the real reason on why qt differ from qwidget to qgraphicsitem here.
Antonio Gomes
Comment 6 2009-10-20 03:09:25 PDT
(In reply to comment #5) > as i talked to ariya on irc and pointed out in bug description and comment #2, > i agree w/ kenneth that it has to be investigated in qt side about why such > behaviour, however i do not need we have to be crashy until then. So i am > landing the fix/workaround and filing to followup bugs: > > 1) one for auto test this > 2) for investigating the real reason on why qt differ from qwidget to > qgraphicsitem here. 1) 30557 2) 30558
Note You need to log in before you can comment on or make changes to this bug.