Fixes crash in QWidget::effectiveWinId.

Widgets are left in a transitional (and incosistent) state while being
re-parented, which caused QWidget::effectiveWinId() to crash in certain
circumstances. See patch for more details.

Auto test included.

Reviewed-by: ogoffart

3 files changed, 46 insertions, 0 deletions

diff --git a/src/gui/kernel/qwidget.cpp b/src/gui/kernel/qwidget.cpp
index ac35d42..fcb098f 100644
--- a/src/gui/kernel/qwidget.cpp
+++ b/src/gui/kernel/qwidget.cpp
@@ -292,6 +292,7 @@ QWidgetPrivate::QWidgetPrivate(int version)
 #ifndef QT_NO_IM
       , inheritsInputMethodHints(0)
 #endif
+      , inSetParent(0)
 #if defined(Q_WS_X11)
       , picture(0)
 #elif defined(Q_WS_WIN)
@@ -2563,6 +2564,22 @@ WId QWidget::effectiveWinId() const
     if (id || !testAttribute(Qt::WA_WState_Created))
         return id;
     QWidget *realParent = nativeParentWidget();
+    if (!realParent && d_func()->inSetParent) {
+        // In transitional state. This is really just a workaround. The real problem
+        // is that QWidgetPrivate::setParent_sys (platform specific code) first sets
+        // the window id to 0 (setWinId(0)) before it sets the Qt::WA_WState_Created
+        // attribute to false. The correct way is to do it the other way around, and
+        // in that case the Qt::WA_WState_Created logic above will kick in and
+        // return 0 whenever the widget is in a transitional state. However, changing
+        // the original logic for all platforms is far more intrusive and might
+        // break existing applications.
+        // Note: The widget can only be in a transitional state when changing its
+        // parent -- everything else is an internal error -- hence explicitly checking
+        // against 'inSetParent' rather than doing an unconditional return whenever
+        // 'realParent' is 0 (which may cause strange artifacts and headache later).
+        return 0;
+    }
+    // This widget *must* have a native parent widget.
     Q_ASSERT(realParent);
     Q_ASSERT(realParent->internalWinId());
     return realParent->internalWinId();
@@ -10041,6 +10058,7 @@ void QWidget::setParent(QWidget *parent)
 void QWidget::setParent(QWidget *parent, Qt::WindowFlags f)
 {
     Q_D(QWidget);
+    d->inSetParent = true;
     bool resized = testAttribute(Qt::WA_Resized);
     bool wasCreated = testAttribute(Qt::WA_WState_Created);
     QWidget *oldtlw = window();
@@ -10195,6 +10213,8 @@ void QWidget::setParent(QWidget *parent, Qt::WindowFlags f)
             ancestorProxy->d_func()->embedSubWindow(this);
     }
 #endif
+
+    d->inSetParent = false;
 }
 
 /*!
diff --git a/src/gui/kernel/qwidget_p.h b/src/gui/kernel/qwidget_p.h
index 377e3a7..c9dba29 100644
--- a/src/gui/kernel/qwidget_p.h
+++ b/src/gui/kernel/qwidget_p.h
@@ -759,6 +759,7 @@ public:
 #ifndef QT_NO_IM
     uint inheritsInputMethodHints : 1;
 #endif
+    uint inSetParent : 1;
 
     // *************************** Platform specific ************************************
 #if defined(Q_WS_X11) // <----------------------------------------------------------- X11
diff --git a/tests/auto/qwidget/tst_qwidget.cpp b/tests/auto/qwidget/tst_qwidget.cpp
index bb8b8b4..a851d03 100644
--- a/tests/auto/qwidget/tst_qwidget.cpp
+++ b/tests/auto/qwidget/tst_qwidget.cpp
@@ -345,6 +345,7 @@ private slots:
     void immediateRepaintAfterInvalidateBuffer();
 #endif
     void effectiveWinId();
+    void effectiveWinId2();
     void customDpi();
     void customDpiProperty();
 
@@ -8522,6 +8523,30 @@ void tst_QWidget::effectiveWinId()
     QVERIFY(child.effectiveWinId());
 }
 
+void tst_QWidget::effectiveWinId2()
+{
+    QWidget parent;
+
+    class MyWidget : public QWidget {
+        bool event(QEvent *e)
+        {
+            if (e->type() == QEvent::WinIdChange) {
+                // Shouldn't crash.
+                effectiveWinId();
+            }
+
+            return QWidget::event(e);
+        }
+    };
+
+    MyWidget child;
+    child.setParent(&parent);
+    parent.show();
+
+    child.setParent(0);
+    child.setParent(&parent);
+}
+
 class CustomWidget : public QWidget
 {
 public: