From e82335afd6e3e9a75275865661cf14a0a2959603 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sun, 2 Aug 2020 14:52:02 -0400 Subject: sfx2: lok: reliably support multi-documents Instead of using the current view to set the DocId, we instead make sure that the ShellView object has the DocId set at construction time. This turned out to be necessary in at least one case (which has a unit-test that failed), which is when events fired during the creation of a new view. The cursor position is notified before we have a chance to set the DocId and because of that we miss the notifications (or worse, we end up sending them to all other documents' views in an effort to fix this bug). This approach is clean and always guarantees that all views have the correct DocId set as soon as possible and that all notifications are sent as expected. A unit-test is added to exercise mult-document usage, which exposed a number of bugs and issues that have been addressed in this patch. Change-Id: Icf5145fb1dabd0d029368310c2b9bf73ae927ccc Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99975 Tested-by: Jenkins CollaboraOffice Reviewed-by: Michael Meeks Reviewed-by: Ashod Nakashian Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106252 Tested-by: Jenkins Reviewed-by: Tor Lillqvist --- desktop/qa/desktop_lib/test_desktop_lib.cxx | 100 +++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 1 deletion(-) (limited to 'desktop/qa') diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx b/desktop/qa/desktop_lib/test_desktop_lib.cxx index a02347450a38..0a19d5582e80 100644 --- a/desktop/qa/desktop_lib/test_desktop_lib.cxx +++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx @@ -195,6 +195,7 @@ public: void testCalcSaveAs(); void testControlState(); void testMetricField(); + void testMultiDocuments(); void testABI(); CPPUNIT_TEST_SUITE(DesktopLOKTest); @@ -257,6 +258,7 @@ public: CPPUNIT_TEST(testCalcSaveAs); CPPUNIT_TEST(testControlState); CPPUNIT_TEST(testMetricField); + CPPUNIT_TEST(testMultiDocuments); CPPUNIT_TEST(testABI); CPPUNIT_TEST_SUITE_END(); @@ -322,10 +324,13 @@ DesktopLOKTest::loadDocUrlImpl(const OUString& rFileURL, LibreOfficeKitDocumentT break; } + static int nDocumentIdCounter = 0; + SfxViewShell::SetCurrentDocId(ViewShellDocId(nDocumentIdCounter)); uno::Reference xComponent = loadFromDesktop(rFileURL, aService); CPPUNIT_ASSERT(xComponent.is()); - std::unique_ptr pDocument(new LibLODocument_Impl(xComponent)); + std::unique_ptr pDocument(new LibLODocument_Impl(xComponent, nDocumentIdCounter)); + ++nDocumentIdCounter; return std::make_pair(std::move(pDocument), xComponent); } @@ -2933,6 +2938,99 @@ void DesktopLOKTest::testSpellcheckerMultiView() CPPUNIT_ASSERT_EQUAL(1, pDocument->m_pDocumentClass->getViewsCount(pDocument)); } +void DesktopLOKTest::testMultiDocuments() +{ + // Load a document. + uno::Reference xComponent1; + std::unique_ptr document1; + std::tie(document1, xComponent1) = loadDocImpl("blank_text.odt"); + LibLODocument_Impl* pDocument1 = document1.get(); + CPPUNIT_ASSERT_EQUAL(1, pDocument1->m_pDocumentClass->getViewsCount(pDocument1)); + const int nDocId1 = pDocument1->mnDocumentId; + + const int nDoc1View0 = pDocument1->m_pDocumentClass->getView(pDocument1); + CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View0)); + const int nDoc1View1 = pDocument1->m_pDocumentClass->createView(pDocument1); + CPPUNIT_ASSERT_EQUAL(nDoc1View1, pDocument1->m_pDocumentClass->getView(pDocument1)); + CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View1)); + CPPUNIT_ASSERT_EQUAL(2, pDocument1->m_pDocumentClass->getViewsCount(pDocument1)); + + // Validate the views of document 1. + std::vector aViewIdsDoc1(2); + CPPUNIT_ASSERT(pDocument1->m_pDocumentClass->getViewIds(pDocument1, aViewIdsDoc1.data(), aViewIdsDoc1.size())); + CPPUNIT_ASSERT_EQUAL(nDoc1View0, aViewIdsDoc1[0]); + CPPUNIT_ASSERT_EQUAL(nDoc1View1, aViewIdsDoc1[1]); + + CPPUNIT_ASSERT_EQUAL(nDoc1View1, pDocument1->m_pDocumentClass->getView(pDocument1)); + CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View1)); + pDocument1->m_pDocumentClass->setView(pDocument1, nDoc1View0); + CPPUNIT_ASSERT_EQUAL(nDoc1View0, pDocument1->m_pDocumentClass->getView(pDocument1)); + CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View0)); + pDocument1->m_pDocumentClass->setView(pDocument1, nDoc1View1); + CPPUNIT_ASSERT_EQUAL(nDoc1View1, pDocument1->m_pDocumentClass->getView(pDocument1)); + CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View1)); + CPPUNIT_ASSERT_EQUAL(2, pDocument1->m_pDocumentClass->getViewsCount(pDocument1)); + + // Load another document. + uno::Reference xComponent2; + std::unique_ptr document2; + std::tie(document2, xComponent2) = loadDocImpl("blank_presentation.odp"); + LibLODocument_Impl* pDocument2 = document2.get(); + CPPUNIT_ASSERT_EQUAL(1, pDocument2->m_pDocumentClass->getViewsCount(pDocument2)); + const int nDocId2 = pDocument2->mnDocumentId; + + const int nDoc2View0 = pDocument2->m_pDocumentClass->getView(pDocument2); + CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View0)); + const int nDoc2View1 = pDocument2->m_pDocumentClass->createView(pDocument2); + CPPUNIT_ASSERT_EQUAL(nDoc2View1, pDocument2->m_pDocumentClass->getView(pDocument2)); + CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View1)); + CPPUNIT_ASSERT_EQUAL(2, pDocument2->m_pDocumentClass->getViewsCount(pDocument2)); + + // Validate the views of document 2. + std::vector aViewIdsDoc2(2); + CPPUNIT_ASSERT(pDocument2->m_pDocumentClass->getViewIds(pDocument2, aViewIdsDoc2.data(), aViewIdsDoc2.size())); + CPPUNIT_ASSERT_EQUAL(nDoc2View0, aViewIdsDoc2[0]); + CPPUNIT_ASSERT_EQUAL(nDoc2View1, aViewIdsDoc2[1]); + + CPPUNIT_ASSERT_EQUAL(nDoc2View1, pDocument2->m_pDocumentClass->getView(pDocument2)); + CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View1)); + pDocument2->m_pDocumentClass->setView(pDocument2, nDoc2View0); + CPPUNIT_ASSERT_EQUAL(nDoc2View0, pDocument2->m_pDocumentClass->getView(pDocument2)); + CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View0)); + pDocument2->m_pDocumentClass->setView(pDocument2, nDoc2View1); + CPPUNIT_ASSERT_EQUAL(nDoc2View1, pDocument2->m_pDocumentClass->getView(pDocument2)); + CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View1)); + CPPUNIT_ASSERT_EQUAL(2, pDocument2->m_pDocumentClass->getViewsCount(pDocument2)); + + // The views of document1 should be unchanged. + CPPUNIT_ASSERT(pDocument1->m_pDocumentClass->getViewIds(pDocument1, aViewIdsDoc1.data(), aViewIdsDoc1.size())); + CPPUNIT_ASSERT_EQUAL(nDoc1View0, aViewIdsDoc1[0]); + CPPUNIT_ASSERT_EQUAL(nDoc1View1, aViewIdsDoc1[1]); + // Switch views in the first doc. + CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View0)); + pDocument1->m_pDocumentClass->setView(pDocument1, nDoc1View0); + CPPUNIT_ASSERT_EQUAL(nDoc1View0, pDocument1->m_pDocumentClass->getView(pDocument1)); + CPPUNIT_ASSERT_EQUAL(nDocId1, SfxLokHelper::getDocumentIdOfView(nDoc1View1)); + pDocument1->m_pDocumentClass->destroyView(pDocument1, nDoc1View1); + CPPUNIT_ASSERT_EQUAL(1, pDocument1->m_pDocumentClass->getViewsCount(pDocument1)); + + // The views of document2 should be unchanged. + CPPUNIT_ASSERT(pDocument2->m_pDocumentClass->getViewIds(pDocument2, aViewIdsDoc2.data(), aViewIdsDoc2.size())); + CPPUNIT_ASSERT_EQUAL(nDoc2View0, aViewIdsDoc2[0]); + CPPUNIT_ASSERT_EQUAL(nDoc2View1, aViewIdsDoc2[1]); + // Switch views in the second doc. + CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View0)); + pDocument2->m_pDocumentClass->setView(pDocument2, nDoc2View0); + CPPUNIT_ASSERT_EQUAL(nDoc2View0, pDocument2->m_pDocumentClass->getView(pDocument2)); + CPPUNIT_ASSERT_EQUAL(nDocId2, SfxLokHelper::getDocumentIdOfView(nDoc2View1)); + pDocument2->m_pDocumentClass->destroyView(pDocument2, nDoc2View1); + CPPUNIT_ASSERT_EQUAL(1, pDocument2->m_pDocumentClass->getViewsCount(pDocument2)); + + closeDoc(document2, xComponent2); + + closeDoc(document1, xComponent1); +} + void DesktopLOKTest::testControlState() { LibLODocument_Impl* pDocument = loadDoc("search.ods"); -- cgit