QQuickIcon: Resolve source URL relative to outermost property owner

The original attempt, deb238586a "QQuickIcon: Resolve URL relative to
current element," works fine for IconLabel itself, but not for Buttons
containing IconLabel.

  // <style>/Button.qml
  T.Button {
      id: control
      icon:  // owner: control
      contentItem: IconLabel {
          icon: control.icon  // owner: this, but should be control
      }
  }

  // user code
  Button { icon.source: "a.png" }

Since IconLabel is an implementation detail of the Button, IconLabel.icon
owner needs to point to the Button so the user-specified icon.source can
be resolved relative to the user code, not to the <style>/ directory.

This patch fixes the problem by explicitly resolving the source URL on
setIcon() and propagating the resolved icon object to the inner items.
If the relative URL has already been resolved by e.g. Button, the inner
IconLabel never resolves the URL again to itself.

The problem could be addressed by initializing icon owner only once by
Action/Button constructor, but that would lead to dangling owner pointer
as icon object could be copied anywhere. So I've added resolvedSource
data member in place of the owner pointer.

  Button { id: dangling }
  Button { id: victim; icon: dangling.icon }  // owner: dangling
  Component.onCompleted: dangling.destroy()
  // ... would SEGV (or use after free) if victim.icon.source modified.

Fixes: QTBUG-95587
Pick-to: 6.2 6.3
Change-Id: Ibdd07118e79f1e1f36e1faea0289150eca734e27
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

8 files changed, 51 insertions, 24 deletions

diff --git a/src/quickcontrols2impl/qquickiconlabel.cpp b/src/quickcontrols2impl/qquickiconlabel.cpp
index a37ce02f6e..309a2c47d4 100644
--- a/src/quickcontrols2impl/qquickiconlabel.cpp
+++ b/src/quickcontrols2impl/qquickiconlabel.cpp
@@ -404,7 +404,7 @@ void QQuickIconLabel::setIcon(const QQuickIcon &icon)
         return;
 
     d->icon = icon;
-    d->icon.setOwner(this);
+    d->icon.ensureRelativeSourceResolved(this);
     d->updateOrSyncImage();
 }
 
diff --git a/src/quicktemplates2/qquickabstractbutton.cpp b/src/quicktemplates2/qquickabstractbutton.cpp
index 191dd81b0b..2e5ac9a576 100644
--- a/src/quicktemplates2/qquickabstractbutton.cpp
+++ b/src/quicktemplates2/qquickabstractbutton.cpp
@@ -820,7 +820,7 @@ void QQuickAbstractButton::setIcon(const QQuickIcon &icon)
 {
     Q_D(QQuickAbstractButton);
     d->icon = icon;
-    d->icon.setOwner(this);
+    d->icon.ensureRelativeSourceResolved(this);
     d->updateEffectiveIcon();
 }
 
diff --git a/src/quicktemplates2/qquickaction.cpp b/src/quicktemplates2/qquickaction.cpp
index ac2b449cd1..189bb860d9 100644
--- a/src/quicktemplates2/qquickaction.cpp
+++ b/src/quicktemplates2/qquickaction.cpp
@@ -406,7 +406,7 @@ void QQuickAction::setIcon(const QQuickIcon &icon)
         return;
 
     d->icon = icon;
-    d->icon.setOwner(this);
+    d->icon.ensureRelativeSourceResolved(this);
     emit iconChanged(icon);
 }
 
diff --git a/src/quicktemplates2/qquickicon.cpp b/src/quicktemplates2/qquickicon.cpp
index 8b2f53f450..439e321001 100644
--- a/src/quicktemplates2/qquickicon.cpp
+++ b/src/quicktemplates2/qquickicon.cpp
@@ -62,6 +62,7 @@ public:
 
     QString name;
     QUrl source;
+    QUrl resolvedSource;
     int width = 0;
     int height = 0;
     QColor color = Qt::transparent;
@@ -107,6 +108,7 @@ bool QQuickIcon::operator==(const QQuickIcon &other) const
 {
     return d == other.d || (d->name == other.d->name
                             && d->source == other.d->source
+                            && d->resolvedSource == other.d->resolvedSource
                             && d->width == other.d->width
                             && d->height == other.d->height
                             && d->color == other.d->color
@@ -157,6 +159,7 @@ void QQuickIcon::setSource(const QUrl &source)
 
     d.detach();
     d->source = source;
+    d->resolvedSource.clear();
     d->resolveMask |= QQuickIconPrivate::SourceResolved;
 }
 
@@ -164,18 +167,27 @@ void QQuickIcon::resetSource()
 {
     d.detach();
     d->source = QString();
+    d->resolvedSource.clear();
     d->resolveMask &= ~QQuickIconPrivate::SourceResolved;
 }
 
 QUrl QQuickIcon::resolvedSource() const
 {
-    if (QObject *owner = d->ownerAndCache.data()) {
-        QQmlData *data = QQmlData::get(owner);
-        if (data && data->outerContext)
-            return data->outerContext->resolvedUrl(d->source);
-    }
+    return d->resolvedSource.isEmpty() ? d->source : d->resolvedSource;
+}
 
-    return d->source;
+// must be called by the property owner (e.g. Button) prior to emitting changed signal.
+void QQuickIcon::ensureRelativeSourceResolved(const QObject *owner)
+{
+    if (d->source.isEmpty())
+        return;
+    if (!d->resolvedSource.isEmpty())
+        return; // already resolved relative to (possibly) different owner
+    const QQmlData *data = QQmlData::get(owner);
+    if (!data || !data->outerContext)
+        return;
+    d.detach();
+    d->resolvedSource = data->outerContext->resolvedUrl(d->source);
 }
 
 int QQuickIcon::width() const
@@ -267,14 +279,6 @@ void QQuickIcon::resetCache()
     d->resolveMask &= ~QQuickIconPrivate::CacheResolved;
 }
 
-void QQuickIcon::setOwner(QObject *owner)
-{
-    if (d->ownerAndCache.data() == owner)
-        return;
-    d.detach();
-    d->ownerAndCache = owner;
-}
-
 QQuickIcon QQuickIcon::resolve(const QQuickIcon &other) const
 {
     QQuickIcon resolved = *this;
@@ -283,8 +287,10 @@ QQuickIcon QQuickIcon::resolve(const QQuickIcon &other) const
     if (!(d->resolveMask & QQuickIconPrivate::NameResolved))
         resolved.d->name = other.d->name;
 
-    if (!(d->resolveMask & QQuickIconPrivate::SourceResolved))
+    if (!(d->resolveMask & QQuickIconPrivate::SourceResolved)) {
         resolved.d->source = other.d->source;
+        resolved.d->resolvedSource = other.d->resolvedSource;
+    }
 
     if (!(d->resolveMask & QQuickIconPrivate::WidthResolved))
         resolved.d->width = other.d->width;
diff --git a/src/quicktemplates2/qquickicon_p.h b/src/quicktemplates2/qquickicon_p.h
index 13e52b956e..60a24a97be 100644
--- a/src/quicktemplates2/qquickicon_p.h
+++ b/src/quicktemplates2/qquickicon_p.h
@@ -94,6 +94,7 @@ public:
     void setSource(const QUrl &source);
     void resetSource();
     QUrl resolvedSource() const;
+    void ensureRelativeSourceResolved(const QObject *owner);
 
     int width() const;
     void setWidth(int width);
@@ -111,10 +112,6 @@ public:
     void setCache(bool cache);
     void resetCache();
 
-    // owner is not a property - it is set internally by classes using icon
-    // so that we can resolve relative URL's correctly
-    void setOwner(QObject *owner);
-
     QQuickIcon resolve(const QQuickIcon &other) const;
 
 private:
diff --git a/tests/auto/quickcontrols2/qquickiconlabel/data/iconSourceContext.qml b/tests/auto/quickcontrols2/qquickiconlabel/data/iconSourceContext.qml
index 0d562d1500..8cf09666d0 100644
--- a/tests/auto/quickcontrols2/qquickiconlabel/data/iconSourceContext.qml
+++ b/tests/auto/quickcontrols2/qquickiconlabel/data/iconSourceContext.qml
@@ -1,7 +1,18 @@
-import QtQuick.Controls
 import QtQuick
+import QtQuick.Controls.Basic
+import QtQuick.Controls.impl
+import "sub" as Sub
 
 Item {
     Image { source: "a.png" }
     IconLabel { icon.source: "a.png" }
+    Button {
+        icon.color: "transparent"
+        icon.source: "a.png"
+    }
+    Button {
+        action: actions.action
+        icon.color: "transparent"
+        Sub.Actions { id: actions }
+    }
 }
diff --git a/tests/auto/quickcontrols2/qquickiconlabel/data/sub/Actions.qml b/tests/auto/quickcontrols2/qquickiconlabel/data/sub/Actions.qml
new file mode 100644
index 0000000000..20bd6a5e6b
--- /dev/null
+++ b/tests/auto/quickcontrols2/qquickiconlabel/data/sub/Actions.qml
@@ -0,0 +1,6 @@
+import QtQuick
+import QtQuick.Controls.Basic
+
+QtObject {
+    readonly property Action action: Action { icon.source: "../a.png" }
+}
diff --git a/tests/auto/quickcontrols2/qquickiconlabel/tst_qquickiconlabel.cpp b/tests/auto/quickcontrols2/qquickiconlabel/tst_qquickiconlabel.cpp
index 32560d01a6..507a4645ee 100644
--- a/tests/auto/quickcontrols2/qquickiconlabel/tst_qquickiconlabel.cpp
+++ b/tests/auto/quickcontrols2/qquickiconlabel/tst_qquickiconlabel.cpp
@@ -38,6 +38,7 @@
 #include <QtQuick/private/qquickimage_p_p.h>
 #include <QtQuickTestUtils/private/qmlutils_p.h>
 #include <QtQuickTestUtils/private/visualtestutils_p.h>
+#include <QtQuickTemplates2/private/qquickabstractbutton_p.h>
 #include <QtQuickTemplates2/private/qquickicon_p.h>
 #include <QtQuickControls2Impl/private/qquickiconimage_p.h>
 #include <QtQuickControls2Impl/private/qquickiconlabel_p.h>
@@ -346,7 +347,13 @@ void tst_qquickiconlabel::iconSourceContext()
     for (QQuickItem *child : root->childItems()) {
         QQuickImage *image = qobject_cast<QQuickImage *>(child);
         if (!image) {
-            if (QQuickIconLabel *label = qobject_cast<QQuickIconLabel *>(child)) {
+            QQuickIconLabel *label = nullptr;
+            if (QQuickAbstractButton *button = qobject_cast<QQuickAbstractButton *>(child)) {
+                label = qobject_cast<QQuickIconLabel *>(button->contentItem());
+            } else {
+                label = qobject_cast<QQuickIconLabel *>(child);
+            }
+            if (label) {
                 QQuickIconLabelPrivate *labelPrivate = static_cast<QQuickIconLabelPrivate *>(
                             QQuickItemPrivate::get(label));
                 image = labelPrivate->image;