Avoid set parent for QLabel.pixmap()

Setting owner as default to not allow Python to create a copy
of the QPixmap associated with the QLabel.
The C++ object pointer is acquired through the pixmap() method.
A test case was included.

Task-number: PYSIDE-150
Change-Id: Ie6975c39cbf49a59ebd478db0e1a0c30fc14864a
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

5 files changed, 105 insertions, 3 deletions

diff --git a/sources/pyside2/PySide2/QtWidgets/typesystem_widgets_common.xml b/sources/pyside2/PySide2/QtWidgets/typesystem_widgets_common.xml
index 488f5081d..27b772064 100644
--- a/sources/pyside2/PySide2/QtWidgets/typesystem_widgets_common.xml
+++ b/sources/pyside2/PySide2/QtWidgets/typesystem_widgets_common.xml
@@ -2593,7 +2593,14 @@
       </modify-argument>
     </modify-function>
 
-    <modify-function signature="setMovie(QMovie*)">
+    <modify-function signature="pixmap()const">
+      <modify-argument index="return">
+        <!-- Defining ownership as "default" avoids the object to be automatically
+             set as parent of the returned pointer. -->
+        <define-ownership class="target" owner="default"/>
+      </modify-argument>
+    </modify-function>
+    <modify-function signature="setMovie(QMovie *)">
       <modify-argument index="1">
         <parent index="this" action="add"/>
       </modify-argument>
diff --git a/sources/pyside2/tests/CMakeLists.txt b/sources/pyside2/tests/CMakeLists.txt
index 3440c1591..199192b3c 100644
--- a/sources/pyside2/tests/CMakeLists.txt
+++ b/sources/pyside2/tests/CMakeLists.txt
@@ -6,6 +6,7 @@ else()
     endif()
 
    # tests/QtWidgets/qstandarditemmodel_test.py needs shiboken2
+   # tests/QtWidgets/qlabel_test.py needs shiboken2
    if(WIN32)
         set(TEST_PYTHONPATH "${CMAKE_BINARY_DIR};${CMAKE_SOURCE_DIR}/tests/util;${CMAKE_BINARY_DIR}/tests/pysidetest;${CMAKE_BINARY_DIR}/../shiboken2/shibokenmodule;$ENV{PYTHONPATH}")
         set(TEST_LIBRARY_PATH   "${libpyside_BINARY_DIR};${pysidetest_BINARY_DIR};${SHIBOKEN_INCLUDE_DIR}/../../bin;$ENV{PATH}")
diff --git a/sources/pyside2/tests/QtWidgets/CMakeLists.txt b/sources/pyside2/tests/QtWidgets/CMakeLists.txt
index 9caf7e365..b350133f9 100644
--- a/sources/pyside2/tests/QtWidgets/CMakeLists.txt
+++ b/sources/pyside2/tests/QtWidgets/CMakeLists.txt
@@ -99,6 +99,7 @@ PYSIDE_TEST(qgraphicsscene_test.py)
 PYSIDE_TEST(qimage_test.py)
 PYSIDE_TEST(qinputdialog_get_test.py)
 PYSIDE_TEST(qkeysequenceedit_test.py)
+PYSIDE_TEST(qlabel_test.py)
 PYSIDE_TEST(qlayout_ref_test.py)
 PYSIDE_TEST(qlayout_test.py)
 PYSIDE_TEST(qlcdnumber_test.py)
diff --git a/sources/pyside2/tests/QtWidgets/bug_714.py b/sources/pyside2/tests/QtWidgets/bug_714.py
index 93dee6630..35d4f6d62 100644
--- a/sources/pyside2/tests/QtWidgets/bug_714.py
+++ b/sources/pyside2/tests/QtWidgets/bug_714.py
@@ -38,8 +38,12 @@ class TestLabelPixmap(unittest.TestCase):
         l.setPixmap(p) # doesn't increment pixmap ref because this makes a copy
         self.assertEqual(sys.getrefcount(p), 2)
 
-        p = l.pixmap() # this increment the reference because this is an internal pointer
-        self.assertEqual(sys.getrefcount(p), 3)
+        p = l.pixmap()
+        # this used to increment the reference because this is
+        # an internal pointer, but not anymore since we don't create
+        # a copy
+        # self.assertEqual(sys.getrefcount(p), 3)
+        self.assertEqual(sys.getrefcount(p), 2)
 
         p2 = l.pixmap()
         self.assertEqual(p, p2)
diff --git a/sources/pyside2/tests/QtWidgets/qlabel_test.py b/sources/pyside2/tests/QtWidgets/qlabel_test.py
new file mode 100644
index 000000000..3f2ae78c6
--- /dev/null
+++ b/sources/pyside2/tests/QtWidgets/qlabel_test.py
@@ -0,0 +1,89 @@
+#############################################################################
+##
+## Copyright (C) 2018 The Qt Company Ltd.
+## Contact: https://www.qt.io/licensing/
+##
+## This file is part of the test suite of PySide2.
+##
+## $QT_BEGIN_LICENSE:GPL-EXCEPT$
+## Commercial License Usage
+## Licensees holding valid commercial Qt licenses may use this file in
+## accordance with the commercial license agreement provided with the
+## Software or, alternatively, in accordance with the terms contained in
+## a written agreement between you and The Qt Company. For licensing terms
+## and conditions see https://www.qt.io/terms-conditions. For further
+## information use the contact form at https://www.qt.io/contact-us.
+##
+## GNU General Public License Usage
+## Alternatively, this file may be used under the terms of the GNU
+## General Public License version 3 as published by the Free Software
+## Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+## included in the packaging of this file. Please review the following
+## information to ensure the GNU General Public License requirements will
+## be met: https://www.gnu.org/licenses/gpl-3.0.html.
+##
+## $QT_END_LICENSE$
+##
+#############################################################################
+
+'''Test cases for QLabel'''
+
+import unittest
+
+from PySide2.QtGui import QPixmap
+from PySide2.QtWidgets import QLabel
+try:
+    # The normal import statement when PySide2 is installed.
+    from PySide2 import shiboken2 as shiboken
+except ImportError:
+    # When running make test in shiboken build dir, or when running
+    # testrunner.py, shiboken2 is not part of the PySide2 module,
+    # so it needs to be imported as a standalone module.
+    import shiboken2 as shiboken
+
+from helper import UsesQApplication
+
+class QLabelTest(UsesQApplication):
+    '''Test case for calling QLabel.setPixmap'''
+
+    def setUp(self):
+        super(QLabelTest, self).setUp()
+        self.label = QLabel()
+
+    def tearDown(self):
+        del self.label
+        super(QLabelTest, self).tearDown()
+
+    def testSetPixmap(self):
+
+        p1 = QPixmap(5, 5)
+        p2 = QPixmap(10, 10)
+
+        self.label.setPixmap(p1)
+        self.assertIsNotNone(self.label.pixmap())
+
+
+        # PYSIDE-150:
+        #   When a new QPixmap is assigned to a QLabel,
+        #   the previous one needs to be cleared.
+        #   This means we should not keep a copy of the QPixmap
+        #   on Python-side.
+
+        # Getting pointer to the QPixmap
+        ret_p = self.label.pixmap()
+        self.assertIsNot(p1, ret_p)
+        # Save the address of the pointer
+        ret_p_addr = shiboken.getCppPointer(ret_p)
+        # Remove the QPixmap
+        del ret_p
+        # Set new QPixmap
+        self.label.setPixmap(p2)
+
+        # There should be no pointers remaining with the same
+        # address that our QPixmap p1 because it was deleted
+        # using `del ret_p`
+        self.assertTrue(all(shiboken.getCppPointer(o) != ret_p_addr
+                   for o in shiboken.getAllValidWrappers()))
+
+if __name__ == '__main__':
+    unittest.main()