ITEM: solve ScCondFormatDlgItem situation

It is used in SC, DirectPut* in Pool, fetched using
Surrogates, all bad. Only to transport data over the
Pool, may not even need to be an Item.
Trying to solve/losen that gordian knot, looks good.
Is now a normal data holder class, could find a good
parent for it that the Dialog and the instances
setting up/using that Dialog can use.
Forgot to reset that data instance in one place, but
also checked in-between a version that still used the
Item to excluse that the Poolis the same, but the
ScTabViewShell does change. FOund an error with
SfxPoolItemHolder when reseting, also changed.

Change-Id: I1c99d675d1cc3d21205c3e2df78d4b52a696e7ee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162313
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>

10 files changed, 54 insertions, 88 deletions

diff --git a/sc/source/ui/app/msgpool.cxx b/sc/source/ui/app/msgpool.cxx
index 83a47c141e48..859a8a3537c6 100644
--- a/sc/source/ui/app/msgpool.cxx
+++ b/sc/source/ui/app/msgpool.cxx
@@ -35,7 +35,6 @@ SfxItemInfo const aMsgItemInfos[] =
     { SID_PIVOT_TABLE,           SFX_ITEMINFOFLAG_NONE },   // SCITEM_PIVOTDATA
     { SID_SOLVE,                 SFX_ITEMINFOFLAG_NONE },   // SCITEM_SOLVEDATA
     { SID_SCUSERLISTS,           SFX_ITEMINFOFLAG_NONE },   // SCITEM_USERLIST
-    { 0,                         SFX_ITEMINFOFLAG_SUPPORT_SURROGATE }  // SCITEM_CONDFORMATDLGDATA
 };
 
 ScMessagePool::ScMessagePool()
@@ -52,7 +51,6 @@ ScMessagePool::ScMessagePool()
     aGlobalPivotItem        ( ScPivotItem           ( SCITEM_PIVOTDATA, nullptr, nullptr, false ) ),
     aGlobalSolveItem        ( ScSolveItem           ( SCITEM_SOLVEDATA, nullptr ) ),
     aGlobalUserListItem     ( ScUserListItem        ( SCITEM_USERLIST ) ),
-    aCondFormatDlgItem      ( ScCondFormatDlgItem   ( nullptr, -1, false ) ),
 
     mvPoolDefaults(MSGPOOL_END - MSGPOOL_START + 1),
     pDocPool(new ScDocumentPool)
@@ -66,7 +64,6 @@ ScMessagePool::ScMessagePool()
     mvPoolDefaults[SCITEM_PIVOTDATA         - MSGPOOL_START] = &aGlobalPivotItem;
     mvPoolDefaults[SCITEM_SOLVEDATA         - MSGPOOL_START] = &aGlobalSolveItem;
     mvPoolDefaults[SCITEM_USERLIST          - MSGPOOL_START] = &aGlobalUserListItem;
-    mvPoolDefaults[SCITEM_CONDFORMATDLGDATA - MSGPOOL_START] = &aCondFormatDlgItem;
 
     SetDefaults( &mvPoolDefaults );
 
diff --git a/sc/source/ui/condformat/condformatdlg.cxx b/sc/source/ui/condformat/condformatdlg.cxx
index 2a1b72459665..008113d0571e 100644
--- a/sc/source/ui/condformat/condformatdlg.cxx
+++ b/sc/source/ui/condformat/condformatdlg.cxx
@@ -21,7 +21,7 @@
 #include <docsh.hxx>
 #include <docfunc.hxx>
 #include <condformatdlgentry.hxx>
-#include <condformatdlgitem.hxx>
+#include <condformatdlgdata.hxx>
 
 ScCondFormatList::ScCondFormatList(ScCondFormatDlg* pDialogParent,
                                    std::unique_ptr<weld::ScrolledWindow> xWindow,
@@ -416,12 +416,14 @@ IMPL_LINK( ScCondFormatList, EntrySelectHdl, ScCondFrmtEntry&, rEntry, void )
 
 ScCondFormatDlg::ScCondFormatDlg(SfxBindings* pB, SfxChildWindow* pCW,
     weld::Window* pParent, ScViewData* pViewData,
-    const ScCondFormatDlgItem* pItem)
+    const std::shared_ptr<ScCondFormatDlgData>& rItem)
         : ScAnyRefDlgController(pB, pCW, pParent,
                         (SfxViewShell::Current() && SfxViewShell::Current()->isLOKMobilePhone())?OUString("modules/scalc/ui/conditionalformatdialogmobile.ui"):OUString("modules/scalc/ui/conditionalformatdialog.ui"),
                         "ConditionalFormatDialog")
     , mpViewData(pViewData)
-    , mpDlgItem(pItem->Clone())
+    // previous version based on SfxPoolItem used SfxPoolItem::Clone here, so make a copy
+    // using copy constructor
+    , mpDlgItem(std::make_shared<ScCondFormatDlgData>(*rItem))
     , mpLastEdit(nullptr)
     , mxBtnOk(m_xBuilder->weld_button("ok"))
     , mxBtnAdd(m_xBuilder->weld_button("add"))
@@ -634,8 +636,8 @@ void ScCondFormatDlg::OkPressed()
             pFormat->SetKey(nKey);
             pList->InsertNew(std::move(pFormat));
         }
-        mpViewData->GetViewShell()->GetPool().DirectPutItemInPool(*mpDlgItem);
 
+        mpViewData->GetViewShell()->setScCondFormatDlgItem(mpDlgItem);
         SetDispatcherLock( false );
         // Queue message to open Conditional Format Manager Dialog
         GetBindings().GetDispatcher()->Execute( SID_OPENDLG_CONDFRMT_MANAGER,
@@ -650,7 +652,7 @@ void ScCondFormatDlg::CancelPressed()
 {
     if ( mpDlgItem->IsManaged() )
     {
-        mpViewData->GetViewShell()->GetPool().DirectPutItemInPool(*mpDlgItem);
+        mpViewData->GetViewShell()->setScCondFormatDlgItem(mpDlgItem);
         SetDispatcherLock( false );
         // Queue message to open Conditional Format Manager Dialog
         GetBindings().GetDispatcher()->Execute( SID_OPENDLG_CONDFRMT_MANAGER,
diff --git a/sc/source/ui/condformat/condformatdlgitem.cxx b/sc/source/ui/condformat/condformatdlgdata.cxx
index e62e720049e9..89cd51b41abb 100644
--- a/sc/source/ui/condformat/condformatdlgitem.cxx
+++ b/sc/source/ui/condformat/condformatdlgdata.cxx
@@ -11,55 +11,38 @@
 #include <utility>
 
 #include <scitems.hxx>
-#include <condformatdlgitem.hxx>
+#include <condformatdlgdata.hxx>
 
-ScCondFormatDlgItem::ScCondFormatDlgItem(std::shared_ptr<ScConditionalFormatList> pCondFormats,
+ScCondFormatDlgData::ScCondFormatDlgData(std::shared_ptr<ScConditionalFormatList> pCondFormats,
         sal_Int32 nItem, bool bManaged):
-    SfxPoolItem(SCITEM_CONDFORMATDLGDATA),
     mpCondFormats(std::move(pCondFormats)),
     mnItem(nItem),
     meDialogType(condformat::dialog::CONDITION),
     mbManaged(bManaged)
 {
-    setNonShareable();
 }
 
-ScCondFormatDlgItem::~ScCondFormatDlgItem()
-{
-}
-
-bool ScCondFormatDlgItem::operator==(const SfxPoolItem& rItem) const
-{
-    assert(SfxPoolItem::operator==(rItem)); (void)rItem;
-    return false;
-}
-
-ScCondFormatDlgItem* ScCondFormatDlgItem::Clone(SfxItemPool* /*pPool*/) const
-{
-    return new ScCondFormatDlgItem(*this);
-}
-
-bool ScCondFormatDlgItem::IsManaged() const
+bool ScCondFormatDlgData::IsManaged() const
 {
     return mbManaged;
 }
 
-condformat::dialog::ScCondFormatDialogType ScCondFormatDlgItem::GetDialogType() const
+condformat::dialog::ScCondFormatDialogType ScCondFormatDlgData::GetDialogType() const
 {
     return meDialogType;
 }
 
-sal_Int32 ScCondFormatDlgItem::GetIndex() const
+sal_Int32 ScCondFormatDlgData::GetIndex() const
 {
     return mnItem;
 }
 
-ScConditionalFormatList* ScCondFormatDlgItem::GetConditionalFormatList()
+ScConditionalFormatList* ScCondFormatDlgData::GetConditionalFormatList()
 {
     return mpCondFormats.get();
 }
 
-void ScCondFormatDlgItem::SetDialogType(condformat::dialog::ScCondFormatDialogType eType)
+void ScCondFormatDlgData::SetDialogType(condformat::dialog::ScCondFormatDialogType eType)
 {
     meDialogType = eType;
 }
diff --git a/sc/source/ui/inc/condformatdlg.hxx b/sc/source/ui/inc/condformatdlg.hxx
index 51f92750d430..4b2b4cfb4c11 100644
--- a/sc/source/ui/inc/condformatdlg.hxx
+++ b/sc/source/ui/inc/condformatdlg.hxx
@@ -10,7 +10,7 @@
 #pragma once
 
 #include <rangelst.hxx>
-#include "condformatdlgitem.hxx"
+#include "condformatdlgdata.hxx"
 #include "condformatdlgentry.hxx"
 
 #include "anyrefdg.hxx"
@@ -82,7 +82,7 @@ private:
     ScAddress maPos;
     ScViewData* mpViewData;
 
-    std::shared_ptr<ScCondFormatDlgItem> mpDlgItem;
+    std::shared_ptr<ScCondFormatDlgData> mpDlgItem;
 
     OUString msBaseTitle;
 
@@ -108,7 +108,7 @@ protected:
 
 public:
     ScCondFormatDlg(SfxBindings* pB, SfxChildWindow* pCW, weld::Window* pWindow,
-                                 ScViewData* pViewData, const ScCondFormatDlgItem* pDlgItem);
+                                 ScViewData* pViewData, const std::shared_ptr<ScCondFormatDlgData>& rItem);
     virtual ~ScCondFormatDlg() override;
 
     std::unique_ptr<ScConditionalFormat> GetConditionalFormat() const;
diff --git a/sc/source/ui/inc/condformatdlgitem.hxx b/sc/source/ui/inc/condformatdlgdata.hxx
index b50b1d417c50..bbee525b3bdd 100644
--- a/sc/source/ui/inc/condformatdlgitem.hxx
+++ b/sc/source/ui/inc/condformatdlgdata.hxx
@@ -9,8 +9,7 @@
 
 #pragma once
 
-#include <svl/poolitem.hxx>
-
+#include <sal/config.h>
 #include <memory>
 
 namespace condformat::dialog
@@ -28,21 +27,14 @@ enum ScCondFormatDialogType
 
 class ScConditionalFormatList;
 
-class ScCondFormatDlgItem : public SfxPoolItem
+class ScCondFormatDlgData
 {
 public:
-    ScCondFormatDlgItem(std::shared_ptr<ScConditionalFormatList> pCondFormats, sal_Int32 nItem,
+    ScCondFormatDlgData(std::shared_ptr<ScConditionalFormatList> pCondFormats, sal_Int32 nItem,
                         bool bManaged);
 
-    virtual ~ScCondFormatDlgItem() override;
-
-    ScCondFormatDlgItem(ScCondFormatDlgItem const&) = default;
-    ScCondFormatDlgItem(ScCondFormatDlgItem&&) = default;
-    ScCondFormatDlgItem& operator=(ScCondFormatDlgItem const&) = delete; // due to SfxPoolItem
-    ScCondFormatDlgItem& operator=(ScCondFormatDlgItem&&) = delete; // due to SfxPoolItem
-
-    virtual bool operator==(const SfxPoolItem&) const override;
-    virtual ScCondFormatDlgItem* Clone(SfxItemPool* pPool = nullptr) const override;
+    ScCondFormatDlgData(ScCondFormatDlgData const&) = default;
+    ScCondFormatDlgData(ScCondFormatDlgData&&) = default;
 
     bool IsManaged() const;
     condformat::dialog::ScCondFormatDialogType GetDialogType() const;
diff --git a/sc/source/ui/inc/msgpool.hxx b/sc/source/ui/inc/msgpool.hxx
index 926fb38bf158..db8811c9c333 100644
--- a/sc/source/ui/inc/msgpool.hxx
+++ b/sc/source/ui/inc/msgpool.hxx
@@ -25,7 +25,6 @@
 #include <svl/stritem.hxx>
 
 #include "uiitems.hxx"
-#include "condformatdlgitem.hxx"
 
 class ScDocumentPool;
 
@@ -40,7 +39,6 @@ class ScMessagePool final : public SfxItemPool
     ScPivotItem         aGlobalPivotItem;
     ScSolveItem         aGlobalSolveItem;
     ScUserListItem      aGlobalUserListItem;
-    ScCondFormatDlgItem aCondFormatDlgItem;
 
     std::vector<SfxPoolItem*>    mvPoolDefaults;
     rtl::Reference<ScDocumentPool> pDocPool;
diff --git a/sc/source/ui/inc/tabvwsh.hxx b/sc/source/ui/inc/tabvwsh.hxx
index 797ca0f621d8..d14985f83284 100644
--- a/sc/source/ui/inc/tabvwsh.hxx
+++ b/sc/source/ui/inc/tabvwsh.hxx
@@ -60,6 +60,8 @@ class ScDPObject;
 class ScNavigatorSettings;
 class ScRangeName;
 class ScDrawTransferObj;
+class ScCondFormatDlgData;
+
 namespace sc { class SparklineShell; }
 
 struct ScHeaderFieldData;
@@ -189,6 +191,8 @@ private:
     OUString   maScope;
 
     std::unique_ptr<ScDragData> m_pDragData;
+    std::shared_ptr<ScCondFormatDlgData> m_pScCondFormatDlgItem;
+
 private:
     void    Construct( TriState nForceDesignMode );
 
@@ -444,6 +448,9 @@ public:
 
     void SetMoveKeepEdit(bool value) { bMoveKeepEdit = value; };
     bool GetMoveKeepEdit() { return bMoveKeepEdit; };
+
+    void setScCondFormatDlgItem(const std::shared_ptr<ScCondFormatDlgData>& rItem) { m_pScCondFormatDlgItem = rItem; }
+    const std::shared_ptr<ScCondFormatDlgData>& getScCondFormatDlgItem() const { return m_pScCondFormatDlgItem; }
 };
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/ui/view/cellsh1.cxx b/sc/source/ui/view/cellsh1.cxx
index 03d93088a54d..e13229b6ed7b 100644
--- a/sc/source/ui/view/cellsh1.cxx
+++ b/sc/source/ui/view/cellsh1.cxx
@@ -74,7 +74,7 @@
 #include <colorscale.hxx>
 #include <condformatdlg.hxx>
 #include <attrib.hxx>
-#include <condformatdlgitem.hxx>
+#include <condformatdlgdata.hxx>
 #include <impex.hxx>
 
 #include <globstr.hrc>
@@ -171,7 +171,7 @@ void SetTabNoAndCursor( const ScViewData& rViewData, std::u16string_view rCellId
 }
 
 void HandleConditionalFormat(sal_uInt32 nIndex, bool bCondFormatDlg, bool bContainsCondFormat,
-                             const sal_uInt16 nSlot, SfxViewShell* pTabViewShell)
+                             const sal_uInt16 nSlot, ScTabViewShell* pTabViewShell)
 {
     condformat::dialog::ScCondFormatDialogType eType = condformat::dialog::NONE;
     switch (nSlot)
@@ -201,9 +201,9 @@ void HandleConditionalFormat(sal_uInt32 nIndex, bool bCondFormatDlg, bool bConta
     {
         // Put the xml string parameter to initialize the
         // Conditional Format Dialog.
-        ScCondFormatDlgItem aDlgItem(nullptr, nIndex, false); // Change here
-        aDlgItem.SetDialogType(eType);
-        pTabViewShell->GetPool().DirectPutItemInPool(aDlgItem);
+        std::shared_ptr<ScCondFormatDlgData> pDlgItem(std::make_shared<ScCondFormatDlgData>(nullptr, nIndex, false));
+        pDlgItem->SetDialogType(eType);
+        pTabViewShell->setScCondFormatDlgItem(pDlgItem);
 
         sal_uInt16 nId = ScCondFormatDlgWrapper::GetChildWindowId();
         SfxViewFrame& rViewFrm = pTabViewShell->GetViewFrame();
@@ -2160,13 +2160,10 @@ void ScCellShell::ExecuteEdit( SfxRequest& rReq )
                 sal_uInt32  nIndex      = sal_uInt32(-1);
                 bool        bManaged    = false;
 
-                // Get the pool item stored by Conditional Format Manager Dialog.
-                ItemSurrogates aSurrogates;
-                pTabViewShell->GetPool().GetItemSurrogates(aSurrogates, SCITEM_CONDFORMATDLGDATA);
-                if (aSurrogates.begin() != aSurrogates.end())
+                const std::shared_ptr<ScCondFormatDlgData>& rDlgItem(pTabViewShell->getScCondFormatDlgItem());
+                if (rDlgItem)
                 {
-                    const ScCondFormatDlgItem* pDlgItem = static_cast<const ScCondFormatDlgItem*>(*aSurrogates.begin());
-                    nIndex = pDlgItem->GetIndex();
+                    nIndex = rDlgItem->GetIndex();
                     bManaged = true;
                 }
 
@@ -2872,14 +2869,10 @@ void ScCellShell::ExecuteEdit( SfxRequest& rReq )
                 ScAddress aPos(rData.GetCurX(), rData.GetCurY(), rData.GetTabNo());
 
                 ScConditionalFormatList* pList = nullptr;
-
-                const ScCondFormatDlgItem* pDlgItem = nullptr;
-                ItemSurrogates aSurrogates;
-                pTabViewShell->GetPool().GetItemSurrogates(aSurrogates, SCITEM_CONDFORMATDLGDATA);
-                if (aSurrogates.begin() != aSurrogates.end())
+                const std::shared_ptr<ScCondFormatDlgData>& rDlgItem(pTabViewShell->getScCondFormatDlgItem());
+                if (rDlgItem)
                 {
-                    pDlgItem= static_cast<const ScCondFormatDlgItem*>(*aSurrogates.begin());
-                    pList = const_cast<ScCondFormatDlgItem*>(pDlgItem)->GetConditionalFormatList();
+                    pList = rDlgItem->GetConditionalFormatList();
                 }
 
                 if (!pList)
@@ -2888,10 +2881,10 @@ void ScCellShell::ExecuteEdit( SfxRequest& rReq )
                 VclPtr<AbstractScCondFormatManagerDlg> pDlg(pFact->CreateScCondFormatMgrDlg(
                     pTabViewShell->GetFrameWeld(), rDoc, pList));
 
-                if (pDlgItem)
+                if (rDlgItem)
                     pDlg->SetModified();
 
-                pDlg->StartExecuteAsync([this, pDlg, &rData, pTabViewShell, pDlgItem, aPos](sal_Int32 nRet){
+                pDlg->StartExecuteAsync([this, pDlg, &rData, pTabViewShell, rDlgItem, aPos](sal_Int32 nRet){
                     std::unique_ptr<ScConditionalFormatList> pCondFormatList = pDlg->GetConditionalFormatList();
                     if(nRet == RET_OK && pDlg->CondFormatsChanged())
                     {
@@ -2901,8 +2894,8 @@ void ScCellShell::ExecuteEdit( SfxRequest& rReq )
                     {
                         // Put the xml string parameter to initialize the
                         // Conditional Format Dialog. ( add new )
-                        pTabViewShell->GetPool().DirectPutItemInPool(ScCondFormatDlgItem(
-                                    std::shared_ptr<ScConditionalFormatList>(pCondFormatList.release()), -1, true));
+                        pTabViewShell->setScCondFormatDlgItem(std::make_shared<ScCondFormatDlgData>(
+                            std::shared_ptr<ScConditionalFormatList>(pCondFormatList.release()), -1, true));
                         // Queue message to open Conditional Format Dialog
                         GetViewData().GetDispatcher().Execute( SID_OPENDLG_CONDFRMT, SfxCallMode::ASYNCHRON );
                     }
@@ -2912,7 +2905,7 @@ void ScCellShell::ExecuteEdit( SfxRequest& rReq )
                         sal_Int32 nIndex = pFormat ? pFormat->GetKey() : -1;
                         // Put the xml string parameter to initialize the
                         // Conditional Format Dialog. ( edit selected conditional format )
-                        pTabViewShell->GetPool().DirectPutItemInPool(ScCondFormatDlgItem(
+                        pTabViewShell->setScCondFormatDlgItem(std::make_shared<ScCondFormatDlgData>(
                                     std::shared_ptr<ScConditionalFormatList>(pCondFormatList.release()), nIndex, true));
 
                         // Queue message to open Conditional Format Dialog
@@ -2921,8 +2914,8 @@ void ScCellShell::ExecuteEdit( SfxRequest& rReq )
                     else
                         pCondFormatList.reset();
 
-                    if (pDlgItem)
-                        pTabViewShell->GetPool().DirectRemoveItemFromPool(*pDlgItem);
+                    if (rDlgItem)
+                        pTabViewShell->setScCondFormatDlgItem(nullptr);
 
                     pDlg->disposeOnce();
                 });
diff --git a/sc/source/ui/view/tabvwsh4.cxx b/sc/source/ui/view/tabvwsh4.cxx
index 1082e0613526..02ddae696640 100644
--- a/sc/source/ui/view/tabvwsh4.cxx
+++ b/sc/source/ui/view/tabvwsh4.cxx
@@ -1773,7 +1773,8 @@ ScTabViewShell::ScTabViewShell( SfxViewFrame& rViewFrame,
     bMoveKeepEdit(false),
     nCurRefDlgId(0),
     mbInSwitch(false),
-    m_pDragData(new ScDragData)
+    m_pDragData(new ScDragData),
+    m_pScCondFormatDlgItem()
 {
     const ScAppOptions& rAppOpt = SC_MOD()->GetAppOptions();
 
diff --git a/sc/source/ui/view/tabvwshc.cxx b/sc/source/ui/view/tabvwshc.cxx
index b0c3aa54343c..caa21c084c3f 100644
--- a/sc/source/ui/view/tabvwshc.cxx
+++ b/sc/source/ui/view/tabvwshc.cxx
@@ -51,7 +51,7 @@
 #include <condformatdlg.hxx>
 #include <condformateasydlg.hxx>
 #include <xmlsourcedlg.hxx>
-#include <condformatdlgitem.hxx>
+#include <condformatdlgdata.hxx>
 #include <formdata.hxx>
 #include <inputwin.hxx>
 
@@ -425,25 +425,18 @@ std::shared_ptr<SfxModelessDialogController> ScTabViewShell::CreateRefDialogCont
         }
         case WID_CONDFRMT_REF:
         {
-            const ScCondFormatDlgItem* pDlgItem = nullptr;
             // Get the pool item stored by Conditional Format Manager Dialog.
-            ItemSurrogates aSurrogates;
-            GetPool().GetItemSurrogates(aSurrogates, SCITEM_CONDFORMATDLGDATA);
-            if (aSurrogates.begin() != aSurrogates.end())
-            {
-                const SfxPoolItem* pItem = *aSurrogates.begin();
-                pDlgItem = static_cast<const ScCondFormatDlgItem*>(pItem);
-            }
+            const std::shared_ptr<ScCondFormatDlgData>& rDlgItem(getScCondFormatDlgItem());
 
-            if (pDlgItem)
+            if (rDlgItem)
             {
                 ScViewData& rViewData = GetViewData();
                 rViewData.SetRefTabNo( rViewData.GetTabNo() );
 
-                xResult = std::make_shared<ScCondFormatDlg>(pB, pCW, pParent, &rViewData, pDlgItem);
+                xResult = std::make_shared<ScCondFormatDlg>(pB, pCW, pParent, &rViewData, rDlgItem);
 
                 // Remove the pool item stored by Conditional Format Manager Dialog.
-                GetPool().DirectRemoveItemFromPool(*pDlgItem);
+                setScCondFormatDlgItem(nullptr);
             }
 
             break;