include/vcl/uitest/uiobject.hxx | 6 +-
sw/qa/uitest/navigator/movechapterupdown.py | 10 ----
sw/qa/uitest/navigator/tdf134960.py | 9 ---
sw/qa/uitest/navigator/tdf137274.py | 9 ---
sw/qa/uitest/navigator/tdf144672.py | 10 ----
sw/qa/uitest/navigator/tdf151051.py | 10 ----
sw/qa/uitest/navigator/tdf154212.py | 10 ----
sw/qa/uitest/writer_tests7/tdf135938.py | 19 --------
vcl/source/treelist/uiobject.cxx | 66 +++++++++++++++++-----------
9 files changed, 45 insertions(+), 104 deletions(-)
New commits:
commit 4edff633dd36ea47d17a993e0afb30fcfc4f9a61
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Tue Oct 24 15:22:49 2023 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Tue Oct 24 18:22:58 2023 +0200
tdf#153519 make TreeListEntryUIObject safer
Do not store a raw pointer to an object that go away.
Consequently we can remove the various sleep() hacks
Change-Id: I3200c26b3a2a4eb7592cb2e5c6af64d6b739d1f6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158390
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/include/vcl/uitest/uiobject.hxx b/include/vcl/uitest/uiobject.hxx
index 293471add9b5..d27140b2c21b 100644
--- a/include/vcl/uitest/uiobject.hxx
+++ b/include/vcl/uitest/uiobject.hxx
@@ -492,7 +492,7 @@ class TreeListEntryUIObject final : public UIObject
{
public:
- TreeListEntryUIObject(const VclPtr<SvTreeListBox>& xTreeList,
SvTreeListEntry* pEntry);
+ TreeListEntryUIObject(const VclPtr<SvTreeListBox>& xTreeList,
std::vector<sal_Int32> nTreePath);
virtual StringMap get_state() override;
@@ -507,9 +507,11 @@ public:
private:
+ SvTreeListEntry* getEntry() const;
+
VclPtr<SvTreeListBox> mxTreeList;
- SvTreeListEntry* const mpEntry;
+ std::vector<sal_Int32> maTreePath;
};
class IconViewUIObject final : public TreeListUIObject
diff --git a/sw/qa/uitest/navigator/movechapterupdown.py
b/sw/qa/uitest/navigator/movechapterupdown.py
index c9e41a0971cd..6bcda9b3d4bb 100644
--- a/sw/qa/uitest/navigator/movechapterupdown.py
+++ b/sw/qa/uitest/navigator/movechapterupdown.py
@@ -10,7 +10,6 @@
from uitest.framework import UITestCase
from libreoffice.uno.propertyvalue import mkPropertyValues
from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file
-import time
class movechapterupdown(UITestCase):
@@ -26,15 +25,6 @@ class movechapterupdown(UITestCase):
# wait until the navigator panel is available
xNavigatorPanel =
self.ui_test.wait_until_child_is_available('NavigatorPanel')
- # HACK, see the `m_aUpdTimer.SetTimeout(1000)` in the
SwContentTree ctor in
- # sw/source/uibase/utlui/content.cxx, where that m_aUpdTimer is
started by
- # SwContentTree::ShowTree triggered from the SIDEBAR action above,
and which can
- # invalidate the TreeListEntryUIObjects used by the below code (see
- # 2798430c8a711861fdcdfbf9ac00a0527abd3bfc "Mark the uses of
TreeListEntryUIObject as
- # dubious"); lets double that 1000 ms timeout value here to
hopefully be on the safe
- # side:
- time.sleep(2)
-
# Given the document chapter structure:
# 1. One H1
# 1.1. one_A (H2)
diff --git a/sw/qa/uitest/navigator/tdf134960.py
b/sw/qa/uitest/navigator/tdf134960.py
index 270e9d347abb..8388b63e40f9 100644
--- a/sw/qa/uitest/navigator/tdf134960.py
+++ b/sw/qa/uitest/navigator/tdf134960.py
@@ -9,7 +9,6 @@
from uitest.framework import UITestCase
from libreoffice.uno.propertyvalue import mkPropertyValues
from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file
-import time
class tdf134960_hyperlinks(UITestCase):
@@ -28,14 +27,6 @@ class tdf134960_hyperlinks(UITestCase):
# wait until the navigator panel is available
xNavigatorPanel =
self.ui_test.wait_until_child_is_available('NavigatorPanel')
- # HACK, see the `m_aUpdTimer.SetTimeout(1000)` in the SwContentTree
ctor in
- # sw/source/uibase/utlui/content.cxx, where that m_aUpdTimer is
started by
- # SwContentTree::ShowTree triggered from the SIDEBAR action above, and
which can invalidate
- # the TreeListEntryUIObjects used by the below code (see
- # 2798430c8a711861fdcdfbf9ac00a0527abd3bfc "Mark the uses of
TreeListEntryUIObject as
- # dubious"); lets double that 1000 ms timeout value here to hopefully
be on the safe side:
- time.sleep(2)
-
xContentTree = xNavigatorPanel.getChild("contenttree")
xHyperlinks = self.get_item(xContentTree, 'Hyperlinks')
self.assertEqual('Hyperlinks', get_state_as_dict(xHyperlinks)['Text'])
diff --git a/sw/qa/uitest/navigator/tdf137274.py
b/sw/qa/uitest/navigator/tdf137274.py
index 9bd780a7e5c2..5273ddcb2f91 100644
--- a/sw/qa/uitest/navigator/tdf137274.py
+++ b/sw/qa/uitest/navigator/tdf137274.py
@@ -42,15 +42,6 @@ class tdf137274(UITestCase):
# wait until the navigator panel is available
xNavigatorPanel =
self.ui_test.wait_until_child_is_available('NavigatorPanel')
- # HACK, see the `m_aUpdTimer.SetTimeout(1000)` in the
SwContentTree ctor in
- # sw/source/uibase/utlui/content.cxx, where that m_aUpdTimer is
started by
- # SwContentTree::ShowTree triggered from the SIDEBAR action above,
and which can
- # invalidate the TreeListEntryUIObjects used by the below code (see
- # 2798430c8a711861fdcdfbf9ac00a0527abd3bfc "Mark the uses of
TreeListEntryUIObject as
- # dubious"); lets double that 1000 ms timeout value here to
hopefully be on the safe
- # side:
- time.sleep(2)
-
xContentTree = xNavigatorPanel.getChild("contenttree")
xComments = self.get_item(xContentTree, 'Comments')
self.assertEqual('Comments', get_state_as_dict(xComments)['Text'])
diff --git a/sw/qa/uitest/navigator/tdf144672.py
b/sw/qa/uitest/navigator/tdf144672.py
index 2dc11a8d3cf9..4bded66dcb08 100644
--- a/sw/qa/uitest/navigator/tdf144672.py
+++ b/sw/qa/uitest/navigator/tdf144672.py
@@ -10,7 +10,6 @@
from uitest.framework import UITestCase
from libreoffice.uno.propertyvalue import mkPropertyValues
from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file
-import time
class tdf144672(UITestCase):
@@ -33,15 +32,6 @@ class tdf144672(UITestCase):
# wait until the navigator panel is available
xNavigatorPanel =
self.ui_test.wait_until_child_is_available('NavigatorPanel')
- # HACK, see the `m_aUpdTimer.SetTimeout(1000)` in the
SwContentTree ctor in
- # sw/source/uibase/utlui/content.cxx, where that m_aUpdTimer is
started by
- # SwContentTree::ShowTree triggered from the SIDEBAR action above,
and which can
- # invalidate the TreeListEntryUIObjects used by the below code (see
- # 2798430c8a711861fdcdfbf9ac00a0527abd3bfc "Mark the uses of
TreeListEntryUIObject as
- # dubious"); lets double that 1000 ms timeout value here to
hopefully be on the safe
- # side:
- time.sleep(2)
-
xContentTree = xNavigatorPanel.getChild("contenttree")
xReferences = self.get_item(xContentTree, 'References')
diff --git a/sw/qa/uitest/navigator/tdf151051.py
b/sw/qa/uitest/navigator/tdf151051.py
index d0fb4ef0e708..1778cc94fe68 100644
--- a/sw/qa/uitest/navigator/tdf151051.py
+++ b/sw/qa/uitest/navigator/tdf151051.py
@@ -10,7 +10,6 @@
from uitest.framework import UITestCase
from libreoffice.uno.propertyvalue import mkPropertyValues
from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file
-import time
class tdf151051(UITestCase):
@@ -26,15 +25,6 @@ class tdf151051(UITestCase):
# wait until the navigator panel is available
xNavigatorPanel =
self.ui_test.wait_until_child_is_available('NavigatorPanel')
- # HACK, see the `m_aUpdTimer.SetTimeout(1000)` in the
SwContentTree ctor in
- # sw/source/uibase/utlui/content.cxx, where that m_aUpdTimer is
started by
- # SwContentTree::ShowTree triggered from the SIDEBAR action above,
and which can
- # invalidate the TreeListEntryUIObjects used by the below code (see
- # 2798430c8a711861fdcdfbf9ac00a0527abd3bfc "Mark the uses of
TreeListEntryUIObject as
- # dubious"); lets double that 1000 ms timeout value here to
hopefully be on the safe
- # side:
- time.sleep(2)
-
xContentTree = xNavigatorPanel.getChild("contenttree")
xHeadings = xContentTree.getChild('0')
diff --git a/sw/qa/uitest/navigator/tdf154212.py
b/sw/qa/uitest/navigator/tdf154212.py
index e7222b3d9097..2637780e2f4e 100644
--- a/sw/qa/uitest/navigator/tdf154212.py
+++ b/sw/qa/uitest/navigator/tdf154212.py
@@ -10,7 +10,6 @@
from uitest.framework import UITestCase
from libreoffice.uno.propertyvalue import mkPropertyValues
from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file
-import time
class tdf154212(UITestCase):
@@ -26,15 +25,6 @@ class tdf154212(UITestCase):
# wait until the navigator panel is available
xNavigatorPanel =
self.ui_test.wait_until_child_is_available('NavigatorPanel')
- # HACK, see the `m_aUpdTimer.SetTimeout(1000)` in the
SwContentTree ctor in
- # sw/source/uibase/utlui/content.cxx, where that m_aUpdTimer is
started by
- # SwContentTree::ShowTree triggered from the SIDEBAR action above,
and which can
- # invalidate the TreeListEntryUIObjects used by the below code (see
- # 2798430c8a711861fdcdfbf9ac00a0527abd3bfc "Mark the uses of
TreeListEntryUIObject as
- # dubious"); lets double that 1000 ms timeout value here to
hopefully be on the safe
- # side:
- time.sleep(2)
-
xNavigatorPanelContentTree =
xNavigatorPanel.getChild("contenttree")
xNavigatorPanelContentTreeHeadings =
xNavigatorPanelContentTree.getChild('0')
diff --git a/sw/qa/uitest/writer_tests7/tdf135938.py
b/sw/qa/uitest/writer_tests7/tdf135938.py
index 8faab5c47938..07298ab8d8e1 100755
--- a/sw/qa/uitest/writer_tests7/tdf135938.py
+++ b/sw/qa/uitest/writer_tests7/tdf135938.py
@@ -10,7 +10,6 @@
from uitest.framework import UITestCase
from uitest.uihelper.common import get_state_as_dict
from libreoffice.uno.propertyvalue import mkPropertyValues
-import time
class tdf135938(UITestCase):
@@ -29,15 +28,6 @@ class tdf135938(UITestCase):
xInsert = xDialog.getChild("ok")
xInsert.executeAction("CLICK", tuple())
- # HACK, see the `m_aUpdateTimer.SetTimeout(200)` (to "avoid
flickering of buttons")
- # in the SwChildWinWrapper ctor in
sw/source/uibase/fldui/fldwrap.cxx, where that
- # m_aUpdateTimer is started by SwChildWinWrapper::ReInitDlg
triggered from the
- # xInsert click above, and which can invalidate the
TreeListEntryUIObjects used by
- # the below get_state_as_dict calls (see
2798430c8a711861fdcdfbf9ac00a0527abd3bfc
- # "Mark the uses of TreeListEntryUIObject as dubious"); lets
double that 200 ms
- # timeout value here to hopefully be on the safe side:
- time.sleep(.4);
-
xSelect = xDialog.getChild("select-ref")
self.assertEqual("1", get_state_as_dict(xSelect)["Children"])
self.assertEqual("ABC",
get_state_as_dict(xSelect.getChild(0))["Text"])
@@ -46,15 +36,6 @@ class tdf135938(UITestCase):
xName.executeAction("TYPE", mkPropertyValues({"TEXT": "DEF"}))
xInsert.executeAction("CLICK", tuple())
- # HACK, see the `m_aUpdateTimer.SetTimeout(200)` (to "avoid
flickering of buttons")
- # in the SwChildWinWrapper ctor in
sw/source/uibase/fldui/fldwrap.cxx, where that
- # m_aUpdateTimer is started by SwChildWinWrapper::ReInitDlg
triggered from the
- # xInsert click above, and which can invalidate the
TreeListEntryUIObjects used by
- # the below get_state_as_dict calls (see
2798430c8a711861fdcdfbf9ac00a0527abd3bfc
- # "Mark the uses of TreeListEntryUIObject as dubious"); lets
double that 200 ms
- # timeout value here to hopefully be on the safe side:
- time.sleep(.4);
-
self.assertEqual("2", get_state_as_dict(xSelect)["Children"])
self.assertEqual("ABC",
get_state_as_dict(xSelect.getChild(0))["Text"])
self.assertEqual("DEF",
get_state_as_dict(xSelect.getChild(1))["Text"])
diff --git a/vcl/source/treelist/uiobject.cxx b/vcl/source/treelist/uiobject.cxx
index 30ad0eaebc53..ca45d76fa9e6 100644
--- a/vcl/source/treelist/uiobject.cxx
+++ b/vcl/source/treelist/uiobject.cxx
@@ -68,10 +68,7 @@ std::unique_ptr<UIObject> TreeListUIObject::get_child(const
OUString& rID)
if (!pEntry)
return nullptr;
- return std::unique_ptr<UIObject>(new TreeListEntryUIObject(mxTreeList,
pEntry));
- //TODO: this looks dangerous, as there appears to be no protocol
to guarantee that
- // *pEntry will live as long as the new TreeListEntryUIObject
referencing it by raw
- // pointer
+ return std::unique_ptr<UIObject>(new TreeListEntryUIObject(mxTreeList,
{nID}));
}
else if (nID == -1) // FIXME hack?
{
@@ -109,24 +106,38 @@ std::unique_ptr<UIObject>
TreeListUIObject::create(vcl::Window* pWindow)
return std::unique_ptr<UIObject>(new TreeListUIObject(pTreeList));
}
-TreeListEntryUIObject::TreeListEntryUIObject(const VclPtr<SvTreeListBox>&
xTreeList, SvTreeListEntry* pEntry):
+TreeListEntryUIObject::TreeListEntryUIObject(const VclPtr<SvTreeListBox>&
xTreeList, std::vector<sal_Int32> nTreePath):
mxTreeList(xTreeList),
- mpEntry(pEntry)
+ maTreePath(std::move(nTreePath))
{
}
+SvTreeListEntry* TreeListEntryUIObject::getEntry() const
+{
+ SvTreeListEntry* pEntry = nullptr;
+ for (sal_Int32 nID : maTreePath)
+ {
+ pEntry = mxTreeList->GetEntry(pEntry, nID);
+ if (!pEntry)
+ throw css::uno::RuntimeException("Could not find child with id: "
+ OUString::number(nID));
+ }
+ return pEntry;
+}
+
StringMap TreeListEntryUIObject::get_state()
{
+ SvTreeListEntry* pEntry = getEntry();
+
StringMap aMap;
- aMap["Text"] = mxTreeList->GetEntryText(mpEntry);
- aMap["Children"] =
OUString::number(mxTreeList->GetLevelChildCount(mpEntry));
- aMap["VisibleChildCount"] =
OUString::number(mxTreeList->GetVisibleChildCount(mpEntry));
- aMap["IsSelected"] = OUString::boolean(mxTreeList->IsSelected(mpEntry));
+ aMap["Text"] = mxTreeList->GetEntryText(pEntry);
+ aMap["Children"] =
OUString::number(mxTreeList->GetLevelChildCount(pEntry));
+ aMap["VisibleChildCount"] =
OUString::number(mxTreeList->GetVisibleChildCount(pEntry));
+ aMap["IsSelected"] = OUString::boolean(mxTreeList->IsSelected(pEntry));
- aMap["IsSemiTransparent"] = OUString::boolean(bool(mpEntry->GetFlags() &
SvTLEntryFlags::SEMITRANSPARENT));
+ aMap["IsSemiTransparent"] = OUString::boolean(bool(pEntry->GetFlags() &
SvTLEntryFlags::SEMITRANSPARENT));
- SvLBoxButton* pItem =
static_cast<SvLBoxButton*>(mpEntry->GetFirstItem(SvLBoxItemType::Button));
+ SvLBoxButton* pItem =
static_cast<SvLBoxButton*>(pEntry->GetFirstItem(SvLBoxItemType::Button));
if (pItem)
aMap["IsChecked"] = OUString::boolean(pItem->IsStateChecked());
@@ -135,49 +146,52 @@ StringMap TreeListEntryUIObject::get_state()
void TreeListEntryUIObject::execute(const OUString& rAction, const StringMap&
/*rParameters*/)
{
+ SvTreeListEntry* pEntry = getEntry();
+
if (rAction == "COLLAPSE")
{
- mxTreeList->Collapse(mpEntry);
+ mxTreeList->Collapse(pEntry);
}
else if (rAction == "EXPAND")
{
- mxTreeList->Expand(mpEntry);
+ mxTreeList->Expand(pEntry);
}
else if (rAction == "SELECT")
{
- mxTreeList->Select(mpEntry);
+ mxTreeList->Select(pEntry);
}
else if (rAction == "DESELECT")
{
- mxTreeList->Select(mpEntry, false);
+ mxTreeList->Select(pEntry, false);
}
else if (rAction == "CLICK")
{
- SvLBoxButton* pItem =
static_cast<SvLBoxButton*>(mpEntry->GetFirstItem(SvLBoxItemType::Button));
+ SvLBoxButton* pItem =
static_cast<SvLBoxButton*>(pEntry->GetFirstItem(SvLBoxItemType::Button));
if (!pItem)
return;
- pItem->ClickHdl(mpEntry);
+ pItem->ClickHdl(pEntry);
}
else if (rAction == "DOUBLECLICK")
{
- mxTreeList->SetCurEntry(mpEntry);
+ mxTreeList->SetCurEntry(pEntry);
mxTreeList->DoubleClickHdl();
}
}
std::unique_ptr<UIObject> TreeListEntryUIObject::get_child(const OUString& rID)
{
+ SvTreeListEntry* pParentEntry = getEntry();
+
sal_Int32 nID = rID.toInt32();
if (nID >= 0)
{
- SvTreeListEntry* pEntry = mxTreeList->GetEntry(mpEntry, nID);
+ SvTreeListEntry* pEntry = mxTreeList->GetEntry(pParentEntry, nID);
if (!pEntry)
return nullptr;
- return std::unique_ptr<UIObject>(new TreeListEntryUIObject(mxTreeList,
pEntry));
- //TODO: this looks dangerous, as there appears to be no protocol
to guarantee that
- // *pEntry will live as long as the new TreeListEntryUIObject
referencing it by raw
- // pointer
+ std::vector<sal_Int32> aChildTreePath(maTreePath);
+ aChildTreePath.push_back(nID);
+ return std::unique_ptr<UIObject>(new TreeListEntryUIObject(mxTreeList,
std::move(aChildTreePath)));
}
return nullptr;
@@ -185,9 +199,11 @@ std::unique_ptr<UIObject>
TreeListEntryUIObject::get_child(const OUString& rID)
std::set<OUString> TreeListEntryUIObject::get_children() const
{
+ SvTreeListEntry* pEntry = getEntry();
+
std::set<OUString> aChildren;
- size_t nChildren = mxTreeList->GetLevelChildCount(mpEntry);
+ size_t nChildren = mxTreeList->GetLevelChildCount(pEntry);
for (size_t i = 0; i < nChildren; ++i)
{
aChildren.insert(OUString::number(i));
