vcl/inc/salvtables.hxx        |    6 ++++--
 vcl/source/app/salvtables.cxx |   42 ++++++++++++++----------------------------
 2 files changed, 18 insertions(+), 30 deletions(-)

New commits:
commit 3ee7ac3501c9874992471ed8fd5457165dbe6ec7
Author:     Noel Grandin <[email protected]>
AuthorDate: Fri Dec 6 20:47:30 2024 +0200
Commit:     Xisco Fauli <[email protected]>
CommitDate: Mon Dec 16 15:44:17 2024 +0100

    improve SalInstanceNotebook m_aPages handling
    
    I added some asserts here, and discovered that sometimes
    m_aPages and the pages in the underlying TabControl get out
    of sync. So rather than relying on indexing into a vector,
    just store a map indexed on the page identifier, which
    means everything uses the same scheme now.
    
    Change-Id: I1b8228e9b124521bda0e79c98e4961bedc71d641
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/178003
    Reviewed-by: Noel Grandin <[email protected]>
    Tested-by: Jenkins
    (cherry picked from commit 0aa4bfe37a80f3b17430cb91b4fd5e9e528efcbd)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/178561
    Reviewed-by: Xisco Fauli <[email protected]>

diff --git a/vcl/inc/salvtables.hxx b/vcl/inc/salvtables.hxx
index a511f021940d..2ae6f5040ea6 100644
--- a/vcl/inc/salvtables.hxx
+++ b/vcl/inc/salvtables.hxx
@@ -1160,7 +1160,8 @@ class SalInstanceNotebook : public SalInstanceWidget, 
public virtual weld::Noteb
 {
 private:
     VclPtr<TabControl> m_xNotebook;
-    mutable std::vector<std::shared_ptr<SalInstanceContainer>> m_aPages;
+    /// Constructed on-demand.
+    mutable std::map<OUString, std::shared_ptr<SalInstanceContainer>> m_aPages;
     std::map<OUString, std::pair<VclPtr<TabPage>, VclPtr<VclGrid>>> 
m_aAddedPages;
 
     DECL_LINK(DeactivatePageHdl, TabControl*, bool);
@@ -2297,7 +2298,8 @@ class SalInstanceVerticalNotebook : public 
SalInstanceWidget, public virtual wel
 {
 private:
     VclPtr<VerticalTabControl> m_xNotebook;
-    mutable std::vector<std::unique_ptr<SalInstanceContainer>> m_aPages;
+    /// Constructed on-demand.
+    mutable std::map<OUString, std::shared_ptr<SalInstanceContainer>> m_aPages;
 
     DECL_LINK(DeactivatePageHdl, VerticalTabControl*, bool);
     DECL_LINK(ActivatePageHdl, VerticalTabControl*, void);
diff --git a/vcl/source/app/salvtables.cxx b/vcl/source/app/salvtables.cxx
index 3b06cc0e05e6..51658e77f900 100644
--- a/vcl/source/app/salvtables.cxx
+++ b/vcl/source/app/salvtables.cxx
@@ -2655,11 +2655,12 @@ weld::Container* SalInstanceNotebook::get_page(const 
OUString& rIdent) const
     sal_uInt16 nPageId = m_xNotebook->GetPageId(rIdent);
     TabPage* pPage = m_xNotebook->GetTabPage(nPageId);
     vcl::Window* pChild = pPage->GetChild(0);
-    if (m_aPages.size() < nPageIndex + 1U)
-        m_aPages.resize(nPageIndex + 1U);
-    if (!m_aPages[nPageIndex])
-        m_aPages[nPageIndex] = std::make_shared<SalInstanceContainer>(pChild, 
m_pBuilder, false);
-    return m_aPages[nPageIndex].get();
+    auto it = m_aPages.find(rIdent);
+    if (it != m_aPages.end())
+        return it->second.get();
+    auto pNew = std::make_shared<SalInstanceContainer>(pChild, m_pBuilder, 
false);
+    m_aPages[rIdent] = pNew;
+    return pNew.get();
 }
 
 void SalInstanceNotebook::set_current_page(int nPage)
@@ -2680,8 +2681,7 @@ void SalInstanceNotebook::remove_page(const OUString& 
rIdent)
         return;
 
     m_xNotebook->RemovePage(nPageId);
-    if (nPageIndex < m_aPages.size())
-        m_aPages.erase(m_aPages.begin() + nPageIndex);
+    m_aPages.erase(rIdent);
 
     auto iter = m_aAddedPages.find(rIdent);
     if (iter != m_aAddedPages.end())
@@ -2709,13 +2709,6 @@ void SalInstanceNotebook::insert_page(const OUString& 
rIdent, const OUString& rL
     m_xNotebook->SetTabPage(nNewPageId, xPage);
     m_xNotebook->SetPageName(nNewPageId, rIdent);
     m_aAddedPages.try_emplace(rIdent, xPage, xGrid);
-
-    if (nPos != -1)
-    {
-        unsigned int nPageIndex = static_cast<unsigned int>(nPos);
-        if (nPageIndex < m_aPages.size())
-            m_aPages.insert(m_aPages.begin() + nPageIndex, nullptr);
-    }
 }
 
 int SalInstanceNotebook::get_n_pages() const { return 
m_xNotebook->GetPageCount(); }
@@ -2795,11 +2788,12 @@ weld::Container* 
SalInstanceVerticalNotebook::get_page(const OUString& rIdent) c
     if (nPageIndex == -1)
         return nullptr;
     auto pChild = m_xNotebook->GetPage(rIdent);
-    if (m_aPages.size() < nPageIndex + 1U)
-        m_aPages.resize(nPageIndex + 1U);
-    if (!m_aPages[nPageIndex])
-        m_aPages[nPageIndex].reset(new SalInstanceContainer(pChild, 
m_pBuilder, false));
-    return m_aPages[nPageIndex].get();
+    auto it = m_aPages.find(rIdent);
+    if (it != m_aPages.end())
+        return it->second.get();
+    auto pNew = std::make_shared<SalInstanceContainer>(pChild, m_pBuilder, 
false);
+    m_aPages[rIdent] = pNew;
+    return pNew.get();
 }
 
 void SalInstanceVerticalNotebook::set_current_page(int nPage)
@@ -2818,8 +2812,7 @@ void SalInstanceVerticalNotebook::remove_page(const 
OUString& rIdent)
     if (nPageIndex == TAB_PAGE_NOTFOUND)
         return;
     m_xNotebook->RemovePage(rIdent);
-    if (nPageIndex < m_aPages.size())
-        m_aPages.erase(m_aPages.begin() + nPageIndex);
+    m_aPages.erase(rIdent);
 }
 
 void SalInstanceVerticalNotebook::insert_page(const OUString& rIdent, const 
OUString& rLabel,
@@ -2829,13 +2822,6 @@ void SalInstanceVerticalNotebook::insert_page(const 
OUString& rIdent, const OUSt
     xGrid->set_hexpand(true);
     xGrid->set_vexpand(true);
     m_xNotebook->InsertPage(rIdent, rLabel, Image(), u""_ustr, xGrid, nPos);
-
-    if (nPos != -1)
-    {
-        unsigned int nPageIndex = static_cast<unsigned int>(nPos);
-        if (nPageIndex < m_aPages.size())
-            m_aPages.insert(m_aPages.begin() + nPageIndex, nullptr);
-    }
 }
 
 int SalInstanceVerticalNotebook::get_n_pages() const { return 
m_xNotebook->GetPageCount(); }

Reply via email to