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 <noel.grandin@collabora.co.uk>
Tested-by: Jenkins

2 files changed, 18 insertions, 30 deletions

diff --git a/vcl/inc/salvtables.hxx b/vcl/inc/salvtables.hxx
index c183b7fdb57c..1f8e6ea02e52 100644
--- a/vcl/inc/salvtables.hxx
+++ b/vcl/inc/salvtables.hxx
@@ -1161,7 +1161,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);
@@ -2298,7 +2299,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 af609ee0d9f6..493a5526b92c 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(); }