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
Change-Id: Ibdd07118e79f1e1f36e1faea0289150eca734e27
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
(cherry picked from commit dfc16e40ab43e8062b93c566e4316efe4d4f10a0)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

8 files changed, 51 insertions, 24 deletions

diff --git a/src/quickcontrols2impl/qquickiconlabel.cpp b/src/quickcontrols2impl/qquickiconlabel.cpp
index 111c68cc98..b18d483675 100644
--- a/src/quickcontrols2impl/qquickiconlabel.cpp
+++ b/src/quickcontrols2impl/qquickiconlabel.cpp
@@ -399,7 +399,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 11a6fb7152..a6a94b0ba7 100644
--- a/src/quicktemplates2/qquickabstractbutton.cpp
+++ b/src/quicktemplates2/qquickabstractbutton.cpp
@@ -790,7 +790,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 d8e49add90..193169162d 100644
--- a/src/quicktemplates2/qquickaction.cpp
+++ b/src/quicktemplates2/qquickaction.cpp
@@ -403,7 +403,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 2b58529ec1..5cc1f793c0 100644
--- a/src/quicktemplates2/qquickicon.cpp
+++ b/src/quicktemplates2/qquickicon.cpp
@@ -59,6 +59,7 @@ public:
 
     QString name;
     QUrl source;
+    QUrl resolvedSource;
     int width = 0;
     int height = 0;
     QColor color = Qt::transparent;
@@ -104,6 +105,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
@@ -154,6 +156,7 @@ void QQuickIcon::setSource(const QUrl &source)
 
     d.detach();
     d->source = source;
+    d->resolvedSource.clear();
     d->resolveMask |= QQuickIconPrivate::SourceResolved;
 }
 
@@ -161,18 +164,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
@@ -264,14 +276,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;
@@ -280,8 +284,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 aa05055e5f..0312812429 100644
--- a/src/quicktemplates2/qquickicon_p.h
+++ b/src/quicktemplates2/qquickicon_p.h
@@ -91,6 +91,7 @@ public:
     void setSource(const QUrl &source);
     void resetSource();
     QUrl resolvedSource() const;
+    void ensureRelativeSourceResolved(const QObject *owner);
 
     int width() const;
     void setWidth(int width);
@@ -108,10 +109,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 2ee0bf4c9f..13aad8b465 100644
--- a/tests/auto/quickcontrols2/qquickiconlabel/tst_qquickiconlabel.cpp
+++ b/tests/auto/quickcontrols2/qquickiconlabel/tst_qquickiconlabel.cpp
@@ -37,6 +37,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>
@@ -345,7 +346,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;