From 08863b6fdaa8383e2274826db3ec42c4d4f11576 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C4=99drzej=20Nowacki?= <jedrzej.nowacki@nokia.com>
Date: Tue, 29 Nov 2011 15:42:33 +0100
Subject: [PATCH] Refactor QVariant handlers.

QVariant implementation is based on delegation to a handler. The handler
has rather simple construction, it is a set of function that implements
a switch statement over known types and redirects calls to a right
method of an encapsulated types instance. Unfortunately after qt
modularization project, it is not easy to use types directly from
different modules, as they can be undefined or completely unaccessible.
Which means that each module has to implement own handler to cooperate
correctly with QVariant. We can suspect that list of modules known to
QVariant will grow and it is not limited to GUI, Widgets and Core,
therefore it would be nice to have an unified, from performance and
source code point of view, way of working with handlers.

This patch is an attempt to cleanup handlers. Keynotes:
- Each handler is working only on types defined in the same module
- Core handler implements handling of primitive types too
- Custom types have an own handler
- Each handler is independent which means that dispatch between handlers
  is done on QVariant level
- Handlers might be registered / unregistered using same interface

Change-Id: Ib096df65e2c4ce464bc7a684aade5af7d1264c24
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>
---
 src/corelib/kernel/qmetatype_p.h              |  34 ++-
 src/corelib/kernel/qvariant.cpp               | 261 ++++++++++++++----
 src/corelib/kernel/qvariant.h                 |  19 +-
 src/corelib/kernel/qvariant_p.h               |  54 +---
 src/gui/kernel/qguivariant.cpp                |  72 ++---
 src/widgets/kernel/qwidgetsvariant.cpp        |  27 +-
 .../corelib/kernel/qvariant/tst_qvariant.cpp  |  30 +-
 7 files changed, 321 insertions(+), 176 deletions(-)

diff --git a/src/corelib/kernel/qmetatype_p.h b/src/corelib/kernel/qmetatype_p.h
index 46c5697678..391f37c93d 100644
--- a/src/corelib/kernel/qmetatype_p.h
+++ b/src/corelib/kernel/qmetatype_p.h
@@ -57,11 +57,22 @@
 
 QT_BEGIN_NAMESPACE
 
-enum { /* TYPEMODULEINFO flags */
-    Q_CORE_TYPE = 1,
-    Q_GUI_TYPE = 2,
-    Q_WIDGET_TYPE = 3
-};
+namespace QModulesPrivate {
+enum Names { Core, Gui, Widgets, Unknown, ModulesCount /* ModulesCount has to be at the end */ };
+
+static inline int moduleForType(const int typeId)
+{
+    if (typeId <= QMetaType::LastCoreType)
+        return Core;
+    if (typeId <= QMetaType::LastGuiType)
+        return Gui;
+    if (typeId <= QMetaType::LastWidgetsType)
+        return Widgets;
+    if (typeId <= QMetaType::LastCoreExtType)
+        return Core;
+    return Unknown;
+}
+}
 
 template <typename T>
 class QTypeModuleInfo
@@ -73,7 +84,6 @@ public:
         IsGui = false,
         IsUnknown = !IsCore
     };
-    static inline int module() { return IsCore ? Q_CORE_TYPE : 0; }
 };
 
 #define QT_ASSIGN_TYPE_TO_MODULE(TYPE, MODULE) \
@@ -82,9 +92,9 @@ class QTypeModuleInfo<TYPE > \
 { \
 public: \
     enum Module { \
-        IsCore = (((MODULE) == (Q_CORE_TYPE))), \
-        IsWidget = (((MODULE) == (Q_WIDGET_TYPE))), \
-        IsGui = (((MODULE) == (Q_GUI_TYPE))), \
+        IsCore = (((MODULE) == (QModulesPrivate::Core))), \
+        IsWidget = (((MODULE) == (QModulesPrivate::Widgets))), \
+        IsGui = (((MODULE) == (QModulesPrivate::Gui))), \
         IsUnknown = !(IsCore || IsWidget || IsGui) \
     }; \
     static inline int module() { return MODULE; } \
@@ -96,11 +106,11 @@ public: \
 
 
 #define QT_DECLARE_CORE_MODULE_TYPES_ITER(TypeName, TypeId, Name) \
-    QT_ASSIGN_TYPE_TO_MODULE(Name, Q_CORE_TYPE);
+    QT_ASSIGN_TYPE_TO_MODULE(Name, QModulesPrivate::Core);
 #define QT_DECLARE_GUI_MODULE_TYPES_ITER(TypeName, TypeId, Name) \
-    QT_ASSIGN_TYPE_TO_MODULE(Name, Q_GUI_TYPE);
+    QT_ASSIGN_TYPE_TO_MODULE(Name, QModulesPrivate::Gui);
 #define QT_DECLARE_WIDGETS_MODULE_TYPES_ITER(TypeName, TypeId, Name) \
-    QT_ASSIGN_TYPE_TO_MODULE(Name, Q_WIDGET_TYPE);
+    QT_ASSIGN_TYPE_TO_MODULE(Name, QModulesPrivate::Widgets);
 
 QT_FOR_EACH_STATIC_CORE_CLASS(QT_DECLARE_CORE_MODULE_TYPES_ITER)
 QT_FOR_EACH_STATIC_CORE_TEMPLATE(QT_DECLARE_CORE_MODULE_TYPES_ITER)
diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp
index 29717398a8..f898cc4823 100644
--- a/src/corelib/kernel/qvariant.cpp
+++ b/src/corelib/kernel/qvariant.cpp
@@ -73,6 +73,25 @@ QT_BEGIN_NAMESPACE
 #  define FLT_DIG 6
 #endif
 
+namespace {
+class HandlersManager
+{
+    static const QVariant::Handler *Handlers[QModulesPrivate::ModulesCount];
+public:
+    const QVariant::Handler *operator[] (const int typeId) const
+    {
+        return Handlers[QModulesPrivate::moduleForType(typeId)];
+    }
+
+    void registerHandler(const QModulesPrivate::Names name, const QVariant::Handler *handler)
+    {
+        Handlers[name] = handler;
+    }
+
+    inline void unregisterHandler(const QModulesPrivate::Names name);
+};
+}  // namespace
+
 namespace {
 template<typename T>
 struct TypeDefiniton {
@@ -100,7 +119,9 @@ struct CoreTypesFilter {
         static const bool IsAccepted = QTypeModuleInfo<T>::IsCore && TypeDefiniton<T>::IsAvailable;
     };
 };
-} // namspace
+} // annonymous used to hide TypeDefiniton
+
+namespace { // annonymous used to hide QVariant handlers
 
 static void construct(QVariant::Private *x, const void *copy)
 {
@@ -813,13 +834,142 @@ const QVariant::Handler qt_kernel_variant_handler = {
 #endif
 };
 
+static void dummyConstruct(QVariant::Private *, const void *) { Q_ASSERT_X(false, "QVariant", "Trying to construct an unknown type"); }
+static void dummyClear(QVariant::Private *) { Q_ASSERT_X(false, "QVariant", "Trying to clear an unknown type"); }
+static bool dummyIsNull(const QVariant::Private *d) { Q_ASSERT_X(false, "QVariant::isNull", "Trying to call isNull on an unknown type"); return d->is_null; }
+static bool dummyCompare(const QVariant::Private *, const QVariant::Private *) { Q_ASSERT_X(false, "QVariant", "Trying to compare an unknown types"); return false; }
+static bool dummyConvert(const QVariant::Private *, QVariant::Type , void *, bool *) { Q_ASSERT_X(false, "QVariant", "Trying to convert an unknown type"); return false; }
+#if !defined(QT_NO_DEBUG_STREAM) && !defined(Q_BROKEN_DEBUG_STREAM)
+static void dummyStreamDebug(QDebug, const QVariant &) { Q_ASSERT_X(false, "QVariant", "Trying to convert an unknown type"); }
+#endif
+const QVariant::Handler qt_dummy_variant_handler = {
+    dummyConstruct,
+    dummyClear,
+    dummyIsNull,
+#ifndef QT_NO_DATASTREAM
+    0,
+    0,
+#endif
+    dummyCompare,
+    dummyConvert,
+    0,
+#if !defined(QT_NO_DEBUG_STREAM) && !defined(Q_BROKEN_DEBUG_STREAM)
+    dummyStreamDebug
+#else
+    0
+#endif
+};
+
+static void customConstruct(QVariant::Private *d, const void *copy)
+{
+    const uint size = QMetaType::sizeOf(d->type);
+    if (!size) {
+        d->type = QVariant::Invalid;
+        return;
+    }
+
+    // this logic should match with QVariantIntegrator::CanUseInternalSpace
+    if (size <= sizeof(QVariant::Private::Data)
+            && (QMetaType::typeFlags(d->type) & QMetaType::MovableType)) {
+        QMetaType::construct(d->type, &d->data.ptr, copy);
+        d->is_shared = false;
+    } else {
+        void *ptr = QMetaType::create(d->type, copy);
+        d->is_shared = true;
+        d->data.shared = new QVariant::PrivateShared(ptr);
+    }
+}
+
+static void customClear(QVariant::Private *d)
+{
+    if (!d->is_shared) {
+        QMetaType::destruct(d->type, &d->data.ptr);
+    } else {
+        QMetaType::destroy(d->type, d->data.shared->ptr);
+        delete d->data.shared;
+    }
+}
+
+static bool customIsNull(const QVariant::Private *d)
+{
+    return d->is_null;
+}
+
+static bool customCompare(const QVariant::Private *a, const QVariant::Private *b)
+{
+    const char *const typeName = QMetaType::typeName(a->type);
+    if (Q_UNLIKELY(!typeName) && Q_LIKELY(!QMetaType::isRegistered(a->type)))
+        qFatal("QVariant::compare: type %d unknown to QVariant.", a->type);
+
+    const void *a_ptr = a->is_shared ? a->data.shared->ptr : &(a->data.ptr);
+    const void *b_ptr = b->is_shared ? b->data.shared->ptr : &(b->data.ptr);
+
+    uint typeNameLen = qstrlen(typeName);
+    if (typeNameLen > 0 && typeName[typeNameLen - 1] == '*')
+        return *static_cast<void *const *>(a_ptr) == *static_cast<void *const *>(b_ptr);
+
+    if (a->is_null && b->is_null)
+        return true;
+
+    return !memcmp(a_ptr, b_ptr, QMetaType::sizeOf(a->type));
+}
+
+static bool customConvert(const QVariant::Private *, QVariant::Type, void *, bool *ok)
+{
+    if (ok)
+        *ok = false;
+    return false;
+}
+
+#if !defined(QT_NO_DEBUG_STREAM) && !defined(Q_BROKEN_DEBUG_STREAM)
+static void customStreamDebug(QDebug, const QVariant &) {}
+#endif
+
+const QVariant::Handler qt_custom_variant_handler = {
+    customConstruct,
+    customClear,
+    customIsNull,
+#ifndef QT_NO_DATASTREAM
+    0,
+    0,
+#endif
+    customCompare,
+    customConvert,
+    0,
+#if !defined(QT_NO_DEBUG_STREAM) && !defined(Q_BROKEN_DEBUG_STREAM)
+    customStreamDebug
+#else
+    0
+#endif
+};
+
+} // annonymous used to hide QVariant handlers
+
+static HandlersManager handlerManager;
+Q_STATIC_ASSERT_X(!QModulesPrivate::Core, "Initialization assumes that ModulesNames::Core is 0");
+const QVariant::Handler *HandlersManager::Handlers[QModulesPrivate::ModulesCount]
+                                        = { &qt_kernel_variant_handler, &qt_dummy_variant_handler,
+                                            &qt_dummy_variant_handler, &qt_custom_variant_handler };
+
 Q_CORE_EXPORT const QVariant::Handler *qcoreVariantHandler()
 {
     return &qt_kernel_variant_handler;
 }
 
+inline void HandlersManager::unregisterHandler(const QModulesPrivate::Names name)
+{
+    Handlers[name] = &qt_dummy_variant_handler;
+}
 
-const QVariant::Handler *QVariant::handler = &qt_kernel_variant_handler;
+Q_CORE_EXPORT void QVariantPrivate::registerHandler(const int /* Modules::Names */name, const QVariant::Handler *handler)
+{
+    handlerManager.registerHandler(static_cast<QModulesPrivate::Names>(name), handler);
+}
+
+Q_CORE_EXPORT void QVariantPrivate::unregisterHandler(const int /* Modules::Names */ name)
+{
+    handlerManager.unregisterHandler(static_cast<QModulesPrivate::Names>(name));
+}
 
 /*!
     \class QVariant
@@ -1018,7 +1168,7 @@ const QVariant::Handler *QVariant::handler = &qt_kernel_variant_handler;
 void QVariant::create(int type, const void *copy)
 {
     d.type = type;
-    handler->construct(&d, copy);
+    handlerManager[type]->construct(&d, copy);
 }
 
 /*!
@@ -1035,7 +1185,7 @@ void QVariant::create(int type, const void *copy)
 QVariant::~QVariant()
 {
     if ((d.is_shared && !d.data.shared->ref.deref()) || (!d.is_shared && d.type > Char))
-        handler->clear(&d);
+        handlerManager[d.type]->clear(&d);
 }
 
 /*!
@@ -1051,7 +1201,7 @@ QVariant::QVariant(const QVariant &p)
     if (d.is_shared) {
         d.data.shared->ref.ref();
     } else if (p.d.type > Char) {
-        handler->construct(&d, p.constData());
+        handlerManager[d.type]->construct(&d, p.constData());
         d.is_null = p.d.is_null;
     }
 }
@@ -1435,7 +1585,7 @@ QVariant& QVariant::operator=(const QVariant &variant)
         d = variant.d;
     } else if (variant.d.type > Char) {
         d.type = variant.d.type;
-        handler->construct(&d, variant.constData());
+        handlerManager[d.type]->construct(&d, variant.constData());
         d.is_null = variant.d.is_null;
     } else {
         d = variant.d;
@@ -1465,9 +1615,9 @@ void QVariant::detach()
 
     Private dd;
     dd.type = d.type;
-    handler->construct(&dd, constData());
+    handlerManager[d.type]->construct(&dd, constData());
     if (!d.data.shared->ref.deref())
-        handler->clear(&d);
+        handlerManager[d.type]->clear(&d);
     d.data.shared = dd.data.shared;
 }
 
@@ -1496,7 +1646,7 @@ const char *QVariant::typeName() const
 void QVariant::clear()
 {
     if ((d.is_shared && !d.data.shared->ref.deref()) || (!d.is_shared && d.type > Char))
-        handler->clear(&d);
+        handlerManager[d.type]->clear(&d);
     d.type = Invalid;
     d.is_null = true;
     d.is_shared = false;
@@ -1732,14 +1882,13 @@ QDataStream& operator<<(QDataStream &s, const QVariant::Type p)
 */
 
 template <typename T>
-inline T qVariantToHelper(const QVariant::Private &d, QVariant::Type t,
-                          const QVariant::Handler *handler, T * = 0)
+inline T qVariantToHelper(const QVariant::Private &d, QVariant::Type t, const HandlersManager &handler)
 {
     if (d.type == t)
         return *v_cast<T>(&d);
 
     T ret;
-    handler->convert(&d, t, &ret, 0);
+    handler[d.type]->convert(&d, t, &ret, 0);
     return ret;
 }
 
@@ -1754,7 +1903,7 @@ inline T qVariantToHelper(const QVariant::Private &d, QVariant::Type t,
 */
 QStringList QVariant::toStringList() const
 {
-    return qVariantToHelper<QStringList>(d, StringList, handler);
+    return qVariantToHelper<QStringList>(d, StringList, handlerManager);
 }
 
 /*!
@@ -1767,7 +1916,7 @@ QStringList QVariant::toStringList() const
 */
 QString QVariant::toString() const
 {
-    return qVariantToHelper<QString>(d, String, handler);
+    return qVariantToHelper<QString>(d, String, handlerManager);
 }
 
 /*!
@@ -1778,7 +1927,7 @@ QString QVariant::toString() const
 */
 QVariantMap QVariant::toMap() const
 {
-    return qVariantToHelper<QVariantMap>(d, Map, handler);
+    return qVariantToHelper<QVariantMap>(d, Map, handlerManager);
 }
 
 /*!
@@ -1789,7 +1938,7 @@ QVariantMap QVariant::toMap() const
 */
 QVariantHash QVariant::toHash() const
 {
-    return qVariantToHelper<QVariantHash>(d, Hash, handler);
+    return qVariantToHelper<QVariantHash>(d, Hash, handlerManager);
 }
 
 /*!
@@ -1805,7 +1954,7 @@ QVariantHash QVariant::toHash() const
 */
 QDate QVariant::toDate() const
 {
-    return qVariantToHelper<QDate>(d, Date, handler);
+    return qVariantToHelper<QDate>(d, Date, handlerManager);
 }
 
 /*!
@@ -1821,7 +1970,7 @@ QDate QVariant::toDate() const
 */
 QTime QVariant::toTime() const
 {
-    return qVariantToHelper<QTime>(d, Time, handler);
+    return qVariantToHelper<QTime>(d, Time, handlerManager);
 }
 
 /*!
@@ -1838,7 +1987,7 @@ QTime QVariant::toTime() const
 */
 QDateTime QVariant::toDateTime() const
 {
-    return qVariantToHelper<QDateTime>(d, DateTime, handler);
+    return qVariantToHelper<QDateTime>(d, DateTime, handlerManager);
 }
 
 /*!
@@ -1853,7 +2002,7 @@ QDateTime QVariant::toDateTime() const
 #ifndef QT_BOOTSTRAPPED
 QEasingCurve QVariant::toEasingCurve() const
 {
-    return qVariantToHelper<QEasingCurve>(d, EasingCurve, handler);
+    return qVariantToHelper<QEasingCurve>(d, EasingCurve, handlerManager);
 }
 #endif
 
@@ -1868,7 +2017,7 @@ QEasingCurve QVariant::toEasingCurve() const
 */
 QByteArray QVariant::toByteArray() const
 {
-    return qVariantToHelper<QByteArray>(d, ByteArray, handler);
+    return qVariantToHelper<QByteArray>(d, ByteArray, handlerManager);
 }
 
 #ifndef QT_NO_GEOM_VARIANT
@@ -1882,7 +2031,7 @@ QByteArray QVariant::toByteArray() const
 */
 QPoint QVariant::toPoint() const
 {
-    return qVariantToHelper<QPoint>(d, Point, handler);
+    return qVariantToHelper<QPoint>(d, Point, handlerManager);
 }
 
 /*!
@@ -1895,7 +2044,7 @@ QPoint QVariant::toPoint() const
 */
 QRect QVariant::toRect() const
 {
-    return qVariantToHelper<QRect>(d, Rect, handler);
+    return qVariantToHelper<QRect>(d, Rect, handlerManager);
 }
 
 /*!
@@ -1908,7 +2057,7 @@ QRect QVariant::toRect() const
 */
 QSize QVariant::toSize() const
 {
-    return qVariantToHelper<QSize>(d, Size, handler);
+    return qVariantToHelper<QSize>(d, Size, handlerManager);
 }
 
 /*!
@@ -1921,7 +2070,7 @@ QSize QVariant::toSize() const
 */
 QSizeF QVariant::toSizeF() const
 {
-    return qVariantToHelper<QSizeF>(d, SizeF, handler);
+    return qVariantToHelper<QSizeF>(d, SizeF, handlerManager);
 }
 
 /*!
@@ -1934,7 +2083,7 @@ QSizeF QVariant::toSizeF() const
 */
 QRectF QVariant::toRectF() const
 {
-    return qVariantToHelper<QRectF>(d, RectF, handler);
+    return qVariantToHelper<QRectF>(d, RectF, handlerManager);
 }
 
 /*!
@@ -1947,7 +2096,7 @@ QRectF QVariant::toRectF() const
 */
 QLineF QVariant::toLineF() const
 {
-    return qVariantToHelper<QLineF>(d, LineF, handler);
+    return qVariantToHelper<QLineF>(d, LineF, handlerManager);
 }
 
 /*!
@@ -1960,7 +2109,7 @@ QLineF QVariant::toLineF() const
 */
 QLine QVariant::toLine() const
 {
-    return qVariantToHelper<QLine>(d, Line, handler);
+    return qVariantToHelper<QLine>(d, Line, handlerManager);
 }
 
 /*!
@@ -1973,7 +2122,7 @@ QLine QVariant::toLine() const
 */
 QPointF QVariant::toPointF() const
 {
-    return qVariantToHelper<QPointF>(d, PointF, handler);
+    return qVariantToHelper<QPointF>(d, PointF, handlerManager);
 }
 
 #endif // QT_NO_GEOM_VARIANT
@@ -1988,7 +2137,7 @@ QPointF QVariant::toPointF() const
 */
 QUrl QVariant::toUrl() const
 {
-    return qVariantToHelper<QUrl>(d, Url, handler);
+    return qVariantToHelper<QUrl>(d, Url, handlerManager);
 }
 
 /*!
@@ -2001,7 +2150,7 @@ QUrl QVariant::toUrl() const
 */
 QLocale QVariant::toLocale() const
 {
-    return qVariantToHelper<QLocale>(d, Locale, handler);
+    return qVariantToHelper<QLocale>(d, Locale, handlerManager);
 }
 
 /*!
@@ -2016,7 +2165,7 @@ QLocale QVariant::toLocale() const
 #ifndef QT_NO_REGEXP
 QRegExp QVariant::toRegExp() const
 {
-    return qVariantToHelper<QRegExp>(d, RegExp, handler);
+    return qVariantToHelper<QRegExp>(d, RegExp, handlerManager);
 }
 #endif
 
@@ -2030,7 +2179,7 @@ QRegExp QVariant::toRegExp() const
 */
 QChar QVariant::toChar() const
 {
-    return qVariantToHelper<QChar>(d, Char, handler);
+    return qVariantToHelper<QChar>(d, Char, handlerManager);
 }
 
 /*!
@@ -2041,12 +2190,12 @@ QChar QVariant::toChar() const
 */
 QBitArray QVariant::toBitArray() const
 {
-    return qVariantToHelper<QBitArray>(d, BitArray, handler);
+    return qVariantToHelper<QBitArray>(d, BitArray, handlerManager);
 }
 
 template <typename T>
 inline T qNumVariantToHelper(const QVariant::Private &d,
-                             const QVariant::Handler *handler, bool *ok, const T& val)
+                             const HandlersManager &handler, bool *ok, const T& val)
 {
     uint t = qMetaTypeId<T>();
     if (ok)
@@ -2054,8 +2203,8 @@ inline T qNumVariantToHelper(const QVariant::Private &d,
     if (d.type == t)
         return val;
 
-    T ret;
-    if (!handler->convert(&d, QVariant::Type(t), &ret, ok) && ok)
+    T ret = 0;
+    if (!handler[d.type]->convert(&d, QVariant::Type(t), &ret, ok) && ok)
         *ok = false;
     return ret;
 }
@@ -2077,7 +2226,7 @@ inline T qNumVariantToHelper(const QVariant::Private &d,
 */
 int QVariant::toInt(bool *ok) const
 {
-    return qNumVariantToHelper<int>(d, handler, ok, d.data.i);
+    return qNumVariantToHelper<int>(d, handlerManager, ok, d.data.i);
 }
 
 /*!
@@ -2097,7 +2246,7 @@ int QVariant::toInt(bool *ok) const
 */
 uint QVariant::toUInt(bool *ok) const
 {
-    return qNumVariantToHelper<uint>(d, handler, ok, d.data.u);
+    return qNumVariantToHelper<uint>(d, handlerManager, ok, d.data.u);
 }
 
 /*!
@@ -2112,7 +2261,7 @@ uint QVariant::toUInt(bool *ok) const
 */
 qlonglong QVariant::toLongLong(bool *ok) const
 {
-    return qNumVariantToHelper<qlonglong>(d, handler, ok, d.data.ll);
+    return qNumVariantToHelper<qlonglong>(d, handlerManager, ok, d.data.ll);
 }
 
 /*!
@@ -2128,7 +2277,7 @@ qlonglong QVariant::toLongLong(bool *ok) const
 */
 qulonglong QVariant::toULongLong(bool *ok) const
 {
-    return qNumVariantToHelper<qulonglong>(d, handler, ok, d.data.ull);
+    return qNumVariantToHelper<qulonglong>(d, handlerManager, ok, d.data.ull);
 }
 
 /*!
@@ -2148,7 +2297,7 @@ bool QVariant::toBool() const
         return d.data.b;
 
     bool res = false;
-    handler->convert(&d, Bool, &res, 0);
+    handlerManager[d.type]->convert(&d, Bool, &res, 0);
 
     return res;
 }
@@ -2165,7 +2314,7 @@ bool QVariant::toBool() const
 */
 double QVariant::toDouble(bool *ok) const
 {
-    return qNumVariantToHelper<double>(d, handler, ok, d.data.d);
+    return qNumVariantToHelper<double>(d, handlerManager, ok, d.data.d);
 }
 
 /*!
@@ -2182,7 +2331,7 @@ double QVariant::toDouble(bool *ok) const
 */
 float QVariant::toFloat(bool *ok) const
 {
-    return qNumVariantToHelper<float>(d, handler, ok, d.data.f);
+    return qNumVariantToHelper<float>(d, handlerManager, ok, d.data.f);
 }
 
 /*!
@@ -2199,7 +2348,7 @@ float QVariant::toFloat(bool *ok) const
 */
 qreal QVariant::toReal(bool *ok) const
 {
-    return qNumVariantToHelper<qreal>(d, handler, ok, d.data.real);
+    return qNumVariantToHelper<qreal>(d, handlerManager, ok, d.data.real);
 }
 
 /*!
@@ -2210,7 +2359,7 @@ qreal QVariant::toReal(bool *ok) const
 */
 QVariantList QVariant::toList() const
 {
-    return qVariantToHelper<QVariantList>(d, List, handler);
+    return qVariantToHelper<QVariantList>(d, List, handlerManager);
 }
 
 
@@ -2418,12 +2567,24 @@ bool QVariant::convert(Type t)
         return false;
 
     bool isOk = true;
-    if (!handler->convert(&oldValue.d, t, data(), &isOk))
+    if (!handlerManager[d.type]->convert(&oldValue.d, t, data(), &isOk))
         isOk = false;
     d.is_null = !isOk;
     return isOk;
 }
 
+/*!
+  \fn convert(const int type, void *ptr) const
+  \internal
+  Created for qvariant_cast() usage
+*/
+bool QVariant::convert(const int type, void *ptr) const
+{
+    Q_ASSERT(type < int(QMetaType::User));
+    return handlerManager[type]->convert(&d, QVariant::Type(type), ptr, 0);
+}
+
+
 /*!
     \fn bool operator==(const QVariant &v1, const QVariant &v2)
 
@@ -2490,7 +2651,7 @@ bool QVariant::cmp(const QVariant &v) const
         if (!v2.canConvert(Type(d.type)) || !v2.convert(Type(d.type)))
             return false;
     }
-    return handler->compare(&d, &v2.d);
+    return handlerManager[d.type]->compare(&d, &v2.d);
 }
 
 /*! \internal
@@ -2520,7 +2681,7 @@ void* QVariant::data()
 */
 bool QVariant::isNull() const
 {
-    return handler->isNull(&d);
+    return handlerManager[d.type]->isNull(&d);
 }
 
 #ifndef QT_NO_DEBUG_STREAM
@@ -2528,7 +2689,7 @@ QDebug operator<<(QDebug dbg, const QVariant &v)
 {
 #ifndef Q_BROKEN_DEBUG_STREAM
     dbg.nospace() << "QVariant(" << v.typeName() << ", ";
-    QVariant::handler->debugStream(dbg, v);
+    handlerManager[v.d.type]->debugStream(dbg, v);
     dbg.nospace() << ')';
     return dbg.space();
 #else
diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h
index 02458a07e0..9b477e5dd6 100644
--- a/src/corelib/kernel/qvariant.h
+++ b/src/corelib/kernel/qvariant.h
@@ -370,19 +370,21 @@ class Q_CORE_EXPORT QVariant
     { return !cmp(v); }
 
 protected:
-    friend inline bool qvariant_cast_helper(const QVariant &, QVariant::Type, void *);
-    friend void qRegisterGuiVariant();
-    friend void qUnregisterGuiVariant();
     friend inline bool operator==(const QVariant &, const QVariantComparisonHelper &);
 #ifndef QT_NO_DEBUG_STREAM
     friend Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant &);
 #endif
     Private d;
-
-    static const Handler *handler;
-
+#ifndef Q_NO_TEMPLATE_FRIENDS
+    template<typename T>
+    friend inline T qvariant_cast(const QVariant &);
+private:
+#else
+public:
+#endif
     void create(int type, const void *copy);
     bool cmp(const QVariant &other) const;
+    bool convert(const int t, void *ptr) const;
 
 private:
     // force compile error, prevent QVariant(bool) to be called
@@ -396,9 +398,6 @@ public:
     inline DataPtr &data_ptr() { return d; }
 };
 
-inline bool qvariant_cast_helper(const QVariant &v, QVariant::Type tp, void *ptr)
-{ return QVariant::handler->convert(&v.d, tp, ptr, 0); }
-
 template <typename T>
 inline QVariant qVariantFromValue(const T &t)
 {
@@ -488,7 +487,7 @@ template<typename T> inline T qvariant_cast(const QVariant &v)
         return *reinterpret_cast<const T *>(v.constData());
     if (vid < int(QMetaType::User)) {
         T t;
-        if (qvariant_cast_helper(v, QVariant::Type(vid), &t))
+        if (v.convert(vid, &t))
             return t;
     }
     return T();
diff --git a/src/corelib/kernel/qvariant_p.h b/src/corelib/kernel/qvariant_p.h
index a90164fa71..0436e9fe29 100644
--- a/src/corelib/kernel/qvariant_p.h
+++ b/src/corelib/kernel/qvariant_p.h
@@ -58,8 +58,6 @@
 
 #include <QtCore/qglobal.h>
 #include <QtCore/qvariant.h>
-#include <QtCore/private/qmetatype_p.h>
-
 #include "qmetatypeswitcher_p.h"
 
 QT_BEGIN_NAMESPACE
@@ -172,24 +170,7 @@ class QVariantComparator {
     };
     template<typename T>
     struct FilteredComparator<T, /* IsAcceptedType = */ false> {
-        static bool compare(const QVariant::Private *m_a, const QVariant::Private *m_b)
-        {
-            const char *const typeName = QMetaType::typeName(m_a->type);
-            if (Q_UNLIKELY(!typeName) && Q_LIKELY(!QMetaType::isRegistered(m_a->type)))
-                qFatal("QVariant::compare: type %d unknown to QVariant.", m_a->type);
-
-            const void *a_ptr = m_a->is_shared ? m_a->data.shared->ptr : &(m_a->data.ptr);
-            const void *b_ptr = m_b->is_shared ? m_b->data.shared->ptr : &(m_b->data.ptr);
-
-            uint typeNameLen = qstrlen(typeName);
-            if (typeNameLen > 0 && typeName[typeNameLen - 1] == '*')
-                return *static_cast<void *const *>(a_ptr) == *static_cast<void *const *>(b_ptr);
-
-            if (m_a->is_null && m_b->is_null)
-                return true;
-
-            return !memcmp(a_ptr, b_ptr, QMetaType::sizeOf(m_a->type));
-        }
+        static bool compare(const QVariant::Private *, const QVariant::Private *) { return false; }
     };
 public:
     QVariantComparator(const QVariant::Private *a, const QVariant::Private *b)
@@ -372,22 +353,8 @@ public:
             m_x->is_shared = false;
             return;
         }
-        const uint size = QMetaType::sizeOf(m_x->type);
-        if (!size) {
-            m_x->type = QVariant::Invalid;
-            return;
-        }
-
-        // this logic should match with QVariantIntegrator::CanUseInternalSpace
-        if (size <= sizeof(QVariant::Private::Data)
-                && (QMetaType::typeFlags(m_x->type) & QMetaType::MovableType)) {
-            QMetaType::construct(m_x->type, &m_x->data.ptr, m_copy);
-            m_x->is_shared = false;
-        } else {
-            void *ptr = QMetaType::create(m_x->type, m_copy);
-            m_x->is_shared = true;
-            m_x->data.shared = new QVariant::PrivateShared(ptr);
-        }
+        qWarning("Trying to construct an instance of an invalid type, type id: %i", m_x->type);
+        m_x->type = QVariant::Invalid;
     }
 
     void delegate(const void*)
@@ -436,13 +403,9 @@ public:
 
     void delegate(const QMetaTypeSwitcher::UnknownType*)
     {
-        // This is not a static type, so lets delegate everyting to QMetaType
-        if (!m_d->is_shared) {
-            QMetaType::destruct(m_d->type, &m_d->data.ptr);
-        } else {
-            QMetaType::destroy(m_d->type, m_d->data.shared->ptr);
-            delete m_d->data.shared;
-        }
+        if (m_d->type == QVariant::UserType)
+            return;
+        qWarning("Trying to destruct an instance of an invalid type, type id: %i", m_d->type);
     }
     // Ignore nonconstructible type
     void delegate(const void*) {}
@@ -450,6 +413,11 @@ private:
     QVariant::Private *m_d;
 };
 
+namespace QVariantPrivate {
+Q_CORE_EXPORT void registerHandler(const int /* Modules::Names */ name, const QVariant::Handler *handler);
+Q_CORE_EXPORT void unregisterHandler(const int /* Modules::Names */ name);
+}
+
 QT_END_NAMESPACE
 
 #endif // QVARIANT_P_H
diff --git a/src/gui/kernel/qguivariant.cpp b/src/gui/kernel/qguivariant.cpp
index a7f2f83aab..45100131de 100644
--- a/src/gui/kernel/qguivariant.cpp
+++ b/src/gui/kernel/qguivariant.cpp
@@ -94,8 +94,6 @@
 
 QT_BEGIN_NAMESPACE
 
-Q_GUI_EXPORT const QVariant::Handler *qt_widgets_variant_handler = 0;
-
 Q_CORE_EXPORT const QVariant::Handler *qcoreVariantHandler();
 
 namespace {
@@ -136,53 +134,35 @@ template<> struct TypeDefiniton<QVector4D> { static const bool IsAvailable = fal
 template<> struct TypeDefiniton<QQuaternion> { static const bool IsAvailable = false; };
 #endif
 
-struct CoreAndGuiTypesFilter {
+struct GuiTypesFilter {
     template<typename T>
     struct Acceptor {
-        static const bool IsAccepted = (QTypeModuleInfo<T>::IsCore || QTypeModuleInfo<T>::IsGui) && TypeDefiniton<T>::IsAvailable;
+        static const bool IsAccepted = QTypeModuleInfo<T>::IsGui && TypeDefiniton<T>::IsAvailable;
     };
 };
-} // namespace
+} // namespace used to hide TypeDefinition
 
+namespace {
 static void construct(QVariant::Private *x, const void *copy)
 {
     const int type = x->type;
-    QVariantConstructor<CoreAndGuiTypesFilter> constructor(x, copy);
-    QMetaTypeSwitcher::switcher<void>(constructor, type, 0);
-
-    // FIXME This is an ugly hack if QVariantConstructor fails to build a value it constructs an invalid type
-    if (Q_UNLIKELY(x->type == QVariant::Invalid)) {
-        if (type == 62) {
-            // small 'trick' to let a QVariant(Qt::blue) create a variant
-            // of type QColor
-            // TODO Get rid of this hack.
-            x->type = QVariant::Color;
-            QColor color(*reinterpret_cast<const Qt::GlobalColor *>(copy));
-            v_construct<QColor>(x, &color);
-            return;
-        }
-        if (type == QVariant::Icon || type == QVariant::SizePolicy) {
-            // TODO we need to clean up variant handlers, so they are replacament, not extension
-            x->type = type;
-            if (qt_widgets_variant_handler) {
-                qt_widgets_variant_handler->construct(x, copy);
-            }
-        }
+    if (Q_UNLIKELY(type == 62)) {
+        // small 'trick' to let a QVariant(Qt::blue) create a variant
+        // of type QColor
+        // TODO Get rid of this hack.
+        x->type = QVariant::Color;
+        QColor color(*reinterpret_cast<const Qt::GlobalColor *>(copy));
+        v_construct<QColor>(x, &color);
+        return;
     }
+    QVariantConstructor<GuiTypesFilter> constructor(x, copy);
+    QMetaTypeSwitcher::switcher<void>(constructor, type, 0);
 }
 
 static void clear(QVariant::Private *d)
 {
-    const int type = d->type;
-    if (type == QVariant::Icon || type == QVariant::SizePolicy) {
-        // TODO we need to clean up variant handlers, so they are replacament, not extension
-        if (qt_widgets_variant_handler) {
-            qt_widgets_variant_handler->clear(d);
-            return;
-        }
-    }
-    QVariantDestructor<CoreAndGuiTypesFilter> destructor(d);
-    QMetaTypeSwitcher::switcher<void>(destructor, type, 0);
+    QVariantDestructor<GuiTypesFilter> destructor(d);
+    QMetaTypeSwitcher::switcher<void>(destructor, d->type, 0);
 }
 
 // This class is a hack that customizes access to QPolygon
@@ -200,7 +180,7 @@ public:
 };
 static bool isNull(const QVariant::Private *d)
 {
-    QGuiVariantIsNull<CoreAndGuiTypesFilter> isNull(d);
+    QGuiVariantIsNull<GuiTypesFilter> isNull(d);
     return QMetaTypeSwitcher::switcher<bool>(isNull, d->type, 0);
 }
 
@@ -215,11 +195,6 @@ public:
     template<typename T>
     bool delegate(const T *p)
     {
-        if (Q_UNLIKELY(Base::m_a->type == QVariant::Icon || Base::m_a->type == QVariant::SizePolicy)) {
-            // TODO we need to clean up variant handlers, so they are replacament, not extension
-            if (Q_LIKELY(qt_widgets_variant_handler))
-                return qt_widgets_variant_handler->compare(Base::m_a, Base::m_b);
-        }
         return Base::delegate(p);
     }
     bool delegate(const QPixmap*)
@@ -241,7 +216,7 @@ public:
 
 static bool compare(const QVariant::Private *a, const QVariant::Private *b)
 {
-    QGuiVariantComparator<CoreAndGuiTypesFilter> comparator(a, b);
+    QGuiVariantComparator<GuiTypesFilter> comparator(a, b);
     return QMetaTypeSwitcher::switcher<bool>(comparator, a->type, 0);
 }
 
@@ -474,8 +449,6 @@ const QVariant::Handler qt_gui_variant_handler = {
 #endif
 };
 
-extern Q_CORE_EXPORT const QMetaTypeInterface *qMetaTypeGuiHelper;
-
 #define QT_IMPL_METATYPEINTERFACE_GUI_TYPES(MetaTypeName, MetaTypeId, RealName) \
     QT_METATYPE_INTERFACE_INIT(RealName),
 
@@ -484,19 +457,20 @@ static const QMetaTypeInterface qVariantGuiHelper[] = {
 };
 
 #undef QT_IMPL_METATYPEINTERFACE_GUI_TYPES
+} // namespace used to hide QVariant handler
+
+extern Q_CORE_EXPORT const QMetaTypeInterface *qMetaTypeGuiHelper;
 
-static const QVariant::Handler *qt_guivariant_last_handler = 0;
 void qRegisterGuiVariant()
 {
-    qt_guivariant_last_handler = QVariant::handler;
-    QVariant::handler = &qt_gui_variant_handler;
+    QVariantPrivate::registerHandler(QModulesPrivate::Gui, &qt_gui_variant_handler);
     qMetaTypeGuiHelper = qVariantGuiHelper;
 }
 Q_CONSTRUCTOR_FUNCTION(qRegisterGuiVariant)
 
 void qUnregisterGuiVariant()
 {
-    QVariant::handler = qt_guivariant_last_handler;
+    QVariantPrivate::unregisterHandler(QModulesPrivate::Gui);
     qMetaTypeGuiHelper = 0;
 }
 Q_DESTRUCTOR_FUNCTION(qUnregisterGuiVariant)
diff --git a/src/widgets/kernel/qwidgetsvariant.cpp b/src/widgets/kernel/qwidgetsvariant.cpp
index 97a8238ca7..92c8d9e6c2 100644
--- a/src/widgets/kernel/qwidgetsvariant.cpp
+++ b/src/widgets/kernel/qwidgetsvariant.cpp
@@ -49,7 +49,7 @@
 
 QT_BEGIN_NAMESPACE
 
-
+namespace {
 static void construct(QVariant::Private *x, const void *copy)
 {
     switch (x->type) {
@@ -97,10 +97,8 @@ static bool isNull(const QVariant::Private *d)
     case QVariant::Icon:
         return v_cast<QIcon>(d)->isNull();
 #endif
-    default:
-        Q_ASSERT(false);
     }
-    return true;
+    return false;
 }
 
 static bool compare(const QVariant::Private *a, const QVariant::Private *b)
@@ -119,6 +117,15 @@ static bool compare(const QVariant::Private *a, const QVariant::Private *b)
     return false;
 }
 
+static bool convert(const QVariant::Private *d, QVariant::Type type, void *result, bool *ok)
+{
+    Q_UNUSED(d);
+    Q_UNUSED(type);
+    Q_UNUSED(result);
+    if (ok)
+        *ok = false;
+    return false;
+}
 
 static const QVariant::Handler widgets_handler = {
     construct,
@@ -129,7 +136,7 @@ static const QVariant::Handler widgets_handler = {
     0,
 #endif
     compare,
-    0,
+    convert,
     0,
 #if !defined(QT_NO_DEBUG_STREAM) && !defined(Q_BROKEN_DEBUG_STREAM)
     0
@@ -138,8 +145,6 @@ static const QVariant::Handler widgets_handler = {
 #endif
 };
 
-extern Q_CORE_EXPORT const QMetaTypeInterface *qMetaTypeWidgetsHelper;
-
 #define QT_IMPL_METATYPEINTERFACE_WIDGETS_TYPES(MetaTypeName, MetaTypeId, RealName) \
     QT_METATYPE_INTERFACE_INIT(RealName),
 
@@ -149,18 +154,20 @@ static const QMetaTypeInterface qVariantWidgetsHelper[] = {
 
 #undef QT_IMPL_METATYPEINTERFACE_WIDGETS_TYPES
 
-extern Q_GUI_EXPORT const QVariant::Handler *qt_widgets_variant_handler;
+}  // namespace
+
+extern Q_CORE_EXPORT const QMetaTypeInterface *qMetaTypeWidgetsHelper;
 
 void qRegisterWidgetsVariant()
 {
-    qt_widgets_variant_handler = &widgets_handler;
     qMetaTypeWidgetsHelper = qVariantWidgetsHelper;
+    QVariantPrivate::registerHandler(QModulesPrivate::Widgets, &widgets_handler);
 }
 Q_CONSTRUCTOR_FUNCTION(qRegisterWidgetsVariant)
 
 void qUnregisterWidgetsVariant()
 {
-    qt_widgets_variant_handler = 0;
+    QVariantPrivate::unregisterHandler(QModulesPrivate::Widgets);
     qMetaTypeWidgetsHelper = 0;
 }
 Q_DESTRUCTOR_FUNCTION(qUnregisterWidgetsVariant)
diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
index 9404130d35..d61536160c 100644
--- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
+++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
@@ -1376,8 +1376,29 @@ void tst_QVariant::quaternion()
     QMetaType::destroy(QVariant::Quaternion, pquaternion);
 }
 
+struct CustomStreamableClass
+{
+    int i;
+    bool operator==(const CustomStreamableClass& other) const
+    {
+        return i == other.i;
+    }
+};
+Q_DECLARE_METATYPE(CustomStreamableClass);
+
+QDataStream &operator<<(QDataStream &out, const CustomStreamableClass &myObj)
+{
+    return out << myObj.i;
+}
+
+QDataStream &operator>>(QDataStream &in, CustomStreamableClass &myObj)
+{
+    return in >> myObj.i;
+}
+
 void tst_QVariant::writeToReadFromDataStream_data()
 {
+    qRegisterMetaTypeStreamOperators<CustomStreamableClass>();
 
     QTest::addColumn<QVariant>("writeVariant");
     QTest::addColumn<bool>("isNull");
@@ -1484,6 +1505,8 @@ void tst_QVariant::writeToReadFromDataStream_data()
     QTest::newRow("QMetaType::Float invalid") << QVariant(QMetaType::Float, (void *) 0) << false;
     float f = 1.234f;
     QTest::newRow("QMetaType::Float") << QVariant(QMetaType::Float, &f) << false;
+    CustomStreamableClass custom = {123};
+    QTest::newRow("Custom type") << QVariant::fromValue(custom) << false;
 }
 
 void tst_QVariant::writeToReadFromDataStream()
@@ -1502,8 +1525,10 @@ void tst_QVariant::writeToReadFromDataStream()
     // Best way to confirm the readVariant contains the same data?
     // Since only a few won't match since the serial numbers are different
     // I won't bother adding another bool in the data test.
-    QVariant::Type writeType = writeVariant.type();
-    if ( writeType != QVariant::Invalid && writeType != QVariant::Bitmap && writeType != QVariant::Pixmap
+    const int writeType = writeVariant.userType();
+    if (writeType == qMetaTypeId<CustomStreamableClass>())
+        QCOMPARE(qvariant_cast<CustomStreamableClass>(readVariant), qvariant_cast<CustomStreamableClass>(writeVariant));
+    else if ( writeType != QVariant::Invalid && writeType != QVariant::Bitmap && writeType != QVariant::Pixmap
         && writeType != QVariant::Image) {
         switch (writeType) {
         default:
@@ -1563,6 +1588,7 @@ void tst_QVariant::writeToReadFromOldDataStream()
 
 void tst_QVariant::checkDataStream()
 {
+    QTest::ignoreMessage(QtWarningMsg, "Trying to construct an instance of an invalid type, type id: 46");
     const QByteArray settingsHex("0000002effffffffff");
     const QByteArray settings = QByteArray::fromHex(settingsHex);
     QDataStream in(settings);