Use __weak for implementation of QObjCWeakPointer

By moving the implementation to qcore_mac.mm, and using a union
for the object pointer, we can build qcore_mac.mm with -fobjc-weak
to take advantage of the automatic weak-tracking. This allows us
to drop the manual handling via ObjC associated objects, while
also allowing non-Objective-C code to use QObjCWeakPointer.

In particular we want to use it for QMacKeyValueObserver, which
today runs the risk of removing the observation on an object that
is long gone.

Pick-to: 6.9
Change-Id: I5d605e5ac82b39223b246d6758d0da88a1702357
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/CMakeLists.txt

@@ -664,14 +664,22 @@ qt_internal_extend_target(Core CONDITION APPLE
                             #   - Provide DT_* macros to qfilesystemengine_unix.cpp
                             #   - Enables SOCK_MAXADDRLEN in case its missing during the unity build
     NO_UNITY_BUILD_SOURCES
+        kernel/qcore_mac.mm # See below
         kernel/qsystemerror.cpp
         # This makes sure that the tst_qmakelib passes. For some reason,
         # QtCore ends up returning a corrupted error message in
         # write_file(): fail
+    NO_PCH_SOURCES
+        kernel/qcore_mac.mm # See below
     ATTRIBUTION_FILE_DIR_PATHS
         kernel
 )
 
+if(APPLE)
+    # For QObjCWeakPointer
+    set_source_files_properties(kernel/qcore_mac.mm PROPERTIES COMPILE_FLAGS -fobjc-weak)
+endif()
+
 qt_internal_extend_target(Core CONDITION MACOS
     LIBRARIES
         ${FWAppKit}

src/corelib/kernel/qcore_mac.mm

@@ -775,17 +775,39 @@ QMacVersion::VersionTuple QMacVersion::libraryVersion()
 
 // -------------------------------------------------------------------------
 
-#if !(__has_feature(objc_arc_weak) && __has_feature(objc_arc_fields))
+QObjCWeakPointerBase::QObjCWeakPointerBase(NSObject *object)
-QT_END_NAMESPACE
+    : m_weakReference(object)
-@implementation QT_MANGLE_NAMESPACE(WeakPointerLifetimeTracker)
-- (void)dealloc
 {
-    *self.pointer = {};
-    [super dealloc];
 }
-@end
+
-QT_BEGIN_NAMESPACE
+QObjCWeakPointerBase::QObjCWeakPointerBase(const QObjCWeakPointerBase &other)
-#endif
+{
+    QMacAutoReleasePool pool;
+    m_weakReference = other.m_weakReference;
+}
+
+QObjCWeakPointerBase &QObjCWeakPointerBase::operator=(const QObjCWeakPointerBase &other)
+{
+    QMacAutoReleasePool pool;
+    m_weakReference = other.m_weakReference;
+    return *this;
+}
+
+QObjCWeakPointerBase::~QObjCWeakPointerBase()
+{
+    QMacAutoReleasePool pool;
+    m_weakReference = nil;
+}
+
+NSObject *QObjCWeakPointerBase::get() const
+{
+    // Loading from a __weak variable will retain and auto-release (in non-ARC).
+    // Unlike cases above, we want the object to stay alive until the outer
+    // auto-release pool is drained, so that consumers of QObjCWeakPointer
+    // can trust that the variable they get back will be alive, similar to
+    // the semantics of loading from __weak.
+    return m_weakReference;
+}
 
 // -------------------------------------------------------------------------
 

src/corelib/kernel/qcore_mac_p.h

@@ -184,6 +184,35 @@ private:
     QString string;
 };
 
+class Q_CORE_EXPORT QObjCWeakPointerBase
+{
+public:
+    QObjCWeakPointerBase(NSObject *object = nil);
+    QObjCWeakPointerBase(const QObjCWeakPointerBase &other);
+    QObjCWeakPointerBase &operator=(const QObjCWeakPointerBase &other);
+
+protected:
+    ~QObjCWeakPointerBase();
+    NSObject *get() const;
+    union {
+        NSObject *m_object = nil;
+#if __has_feature(objc_arc_weak) && __has_feature(objc_arc_fields)
+        // Used by qcore_mac.mm, built with -fobjc-weak, to track lifetime
+        __weak id m_weakReference;
+#endif
+    };
+};
+
+template <typename T>
+class QObjCWeakPointer : public QObjCWeakPointerBase
+{
+public:
+    using QObjCWeakPointerBase::QObjCWeakPointerBase;
+    operator T*() const { return static_cast<T*>(get()); }
+};
+
+// -------------------------------------------------------------------------
+
 #ifdef Q_OS_MACOS
 Q_CORE_EXPORT bool qt_mac_runningUnderRosetta();
 Q_CORE_EXPORT std::optional<uint32_t> qt_mac_sipConfiguration();
@@ -461,92 +490,6 @@ qt_objc_cast(id object)
 
 // -------------------------------------------------------------------------
 
-#if defined( __OBJC__)
-
-template <typename T = NSObject>
-class QObjCWeakPointer;
-
-#if __has_feature(objc_arc_weak) && __has_feature(objc_arc_fields)
-#  define USE_OBJC_WEAK 1
-#endif
-
-#if !USE_OBJC_WEAK
-QT_END_NAMESPACE
-#include <objc/runtime.h>
-Q_CORE_EXPORT
-QT_DECLARE_NAMESPACED_OBJC_INTERFACE(WeakPointerLifetimeTracker, NSObject
-@property (atomic, assign) QT_PREPEND_NAMESPACE(QObjCWeakPointer)<NSObject> *pointer;
-)
-QT_BEGIN_NAMESPACE
-#endif
-
-template <typename T>
-class QObjCWeakPointer
-{
-public:
-    QObjCWeakPointer(T *object = nil) : m_object(object)
-    {
-#if !USE_OBJC_WEAK
-        trackObjectLifetime();
-#endif
-    }
-
-    QObjCWeakPointer(const QObjCWeakPointer &other)
-    {
-        QMacAutoReleasePool pool;
-        m_object = other.m_object;
-#if !USE_OBJC_WEAK
-        trackObjectLifetime();
-#endif
-    }
-
-    QObjCWeakPointer &operator=(const QObjCWeakPointer &other)
-    {
-        QMacAutoReleasePool pool;
-#if !USE_OBJC_WEAK
-        objc_setAssociatedObject(m_object, this, nil, OBJC_ASSOCIATION_RETAIN);
-#endif
-        m_object = other.m_object;
-#if !USE_OBJC_WEAK
-        trackObjectLifetime();
-#endif
-        return *this;
-    }
-
-    ~QObjCWeakPointer()
-    {
-#if !USE_OBJC_WEAK
-        if (m_object)
-            objc_setAssociatedObject(m_object, this, nil, OBJC_ASSOCIATION_RETAIN);
-#endif
-    }
-
-    operator T*() const { return static_cast<T*>([[m_object retain] autorelease]); }
-
-private:
-#if USE_OBJC_WEAK
-    __weak
-#else
-    void trackObjectLifetime()
-    {
-        if (!m_object)
-            return;
-
-        auto *lifetimeTracker = [WeakPointerLifetimeTracker new];
-        lifetimeTracker.pointer = reinterpret_cast<QObjCWeakPointer<NSObject>*>(this);
-        objc_setAssociatedObject(m_object, this, lifetimeTracker, OBJC_ASSOCIATION_RETAIN);
-        [lifetimeTracker release];
-    }
-#endif
-    NSObject *m_object = nil;
-};
-
-#undef USE_OBJC_WEAK
-
-#endif //  __OBJC__
-
-// -------------------------------------------------------------------------
-
 QT_END_NAMESPACE
 
 #endif // QCORE_MAC_P_H

src/tools/bootstrap/CMakeLists.txt

@@ -130,10 +130,19 @@ qt_internal_extend_target(Bootstrap CONDITION APPLE
     SOURCES
         ../../corelib/kernel/qcore_foundation.mm
         ../../corelib/kernel/qcore_mac.mm
+    NO_UNITY_BUILD_SOURCES
+        kernel/qcore_mac.mm # See below
+    NO_PCH_SOURCES
+        kernel/qcore_mac.mm # See below
     PUBLIC_LIBRARIES
         ${FWFoundation}
 )
 
+if(APPLE)
+    # For QObjCWeakPointer
+    set_source_files_properties(../../corelib/kernel/qcore_mac.mm PROPERTIES COMPILE_FLAGS -fobjc-weak)
+endif()
+
 qt_internal_extend_target(Bootstrap CONDITION MACOS
     SOURCES
     LIBRARIES