desktop/qa/desktop_lib/test_desktop_lib.cxx |   83 +++-------------------------
 sfx2/source/control/bindings.cxx            |    4 -
 2 files changed, 11 insertions(+), 76 deletions(-)

New commits:
commit 1e6c89bf752ff93efa1b782564257767d0165cc7
Author: Miklos Vajna <vmik...@collabora.co.uk>
Date:   Thu Oct 20 08:14:07 2016 +0200

    CppunitTest_desktop_lib: fix valgrind errors
    
    With this, 'make -sr CppunitTest_desktop_lib
    CPPUNIT_TEST_NAME="DesktopLOKTest::testPaintPartTile
    DesktopLOKTest::testWriterCommentInsertCursor" VALGRIND=memcheck'
    finishes with 'ERROR SUMMARY: 0 errors from 0 contexts'.
    
    The problem is that in sw/sc/sd code it's enough to make sure that the
    on-stack ViewCallback instances are destroyed after the loaded
    component, but in desktop/ code also the emit-callbacks-on-idle code is
    under test, so we need to make sure first the LOK document wrapper is
    destroyed, and only after that the ViewCallback instances are destroyed.
    
    Change-Id: Ie8361233461d00fd252da929fb912a1a0b835c30
    Reviewed-on: https://gerrit.libreoffice.org/30072
    Reviewed-by: Miklos Vajna <vmik...@collabora.co.uk>
    Tested-by: Jenkins <c...@libreoffice.org>
    (cherry picked from commit ef7ab996d8e4479b1944f7eaf506a506647a90dd)

diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx 
b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index 2dc83d9..03f3262 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -1307,16 +1307,17 @@ void DesktopLOKTest::testPaintPartTile()
 {
     // Load an impress doc of 2 slides.
     comphelper::LibreOfficeKit::setActive();
-    LibLODocument_Impl* pDocument = loadDoc("2slides.odp");
-    pDocument->m_pDocumentClass->initializeForRendering(pDocument, "{}");
     ViewCallback aView1;
+    ViewCallback aView2;
+    std::unique_ptr<LibLODocument_Impl> xDocument(loadDoc("2slides.odp"));
+    LibLODocument_Impl* pDocument = xDocument.get();
+    pDocument->m_pDocumentClass->initializeForRendering(pDocument, "{}");
     pDocument->m_pDocumentClass->registerCallback(pDocument, 
&ViewCallback::callback, &aView1);
     int nView1 = pDocument->m_pDocumentClass->getView(pDocument);
 
     // Create a second view.
     pDocument->m_pDocumentClass->createView(pDocument);
     pDocument->m_pDocumentClass->initializeForRendering(pDocument, "{}");
-    ViewCallback aView2;
     pDocument->m_pDocumentClass->registerCallback(pDocument, 
&ViewCallback::callback, &aView2);
 
     // Go to the second slide in the second view.
@@ -1344,7 +1345,6 @@ void DesktopLOKTest::testPaintPartTile()
     CPPUNIT_ASSERT(aView1.m_bTilesInvalidated);
 
     Scheduler::ProcessEventsToIdle();
-    mxComponent->dispose();
     mxComponent.clear();
     comphelper::LibreOfficeKit::setActive(false);
 }
@@ -1353,13 +1353,14 @@ void DesktopLOKTest::testWriterCommentInsertCursor()
 {
     // Load a document and type a character into the body text of the second 
view.
     comphelper::LibreOfficeKit::setActive();
-    LibLODocument_Impl* pDocument = loadDoc("blank_text.odt");
-    pDocument->m_pDocumentClass->initializeForRendering(pDocument, "{}");
     ViewCallback aView1;
+    ViewCallback aView2;
+    std::unique_ptr<LibLODocument_Impl> xDocument(loadDoc("blank_text.odt"));
+    LibLODocument_Impl* pDocument = xDocument.get();
+    pDocument->m_pDocumentClass->initializeForRendering(pDocument, "{}");
     pDocument->m_pDocumentClass->registerCallback(pDocument, 
&ViewCallback::callback, &aView1);
     pDocument->m_pDocumentClass->createView(pDocument);
     pDocument->m_pDocumentClass->initializeForRendering(pDocument, "{}");
-    ViewCallback aView2;
     pDocument->m_pDocumentClass->registerCallback(pDocument, 
&ViewCallback::callback, &aView2);
     pDocument->m_pDocumentClass->postKeyEvent(pDocument, 
LOK_KEYEVENT_KEYINPUT, 'x', 0);
     pDocument->m_pDocumentClass->postKeyEvent(pDocument, LOK_KEYEVENT_KEYUP, 
'x', 0);
@@ -1384,7 +1385,6 @@ void DesktopLOKTest::testWriterCommentInsertCursor()
     CPPUNIT_ASSERT(aView1.m_aOwnCursor.IsEmpty());
 
     Scheduler::ProcessEventsToIdle();
-    mxComponent->dispose();
     mxComponent.clear();
     comphelper::LibreOfficeKit::setActive(false);
 }
commit 77caedfd01e4e38dd30ac1270433ae6ae1d5a53d
Author: Miklos Vajna <vmik...@collabora.co.uk>
Date:   Wed Oct 19 15:17:14 2016 +0200

    Revert "CppunitTest_desktop_lib: add ModifiedStatus callback testcase"
    
    This reverts commit cdf08b3aa74bb32ea18b583a9c0c41b91d7819ac. It breaks
    'make -sr CppunitTest_desktop_lib
    CPPUNIT_TEST_NAME="DesktopLOKTest::testPaintPartTile
    DesktopLOKTest::testWriterCommentInsertCursor" VALGRIND=memcheck' (it is
    terminated by SIGSEGV), and also it's the reason why sometimes the
    lo_ubsan buildbot fails, see e.g.
    <http://ci.libreoffice.org/job/lo_ubsan/329/console>.
    
    This has to be re-introduced once I find a way to process all binding
    updates at once without side-effects.
    
    Conflicts:
        desktop/qa/desktop_lib/test_desktop_lib.cxx
        sfx2/source/control/bindings.cxx
    
    (cherry picked from commit b86b78e0ad9bb1e6ed2e22a0fca18cad8d19ded1)
    
    Change-Id: Id6c49b9b31095ef1a1a8c1cd92cbae5deb316500

diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx 
b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index 990909f..2dc83d9 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -91,7 +91,6 @@ public:
     void testCellCursor();
     void testCommandResult();
     void testWriterComments();
-    void testModifiedStatus();
     void testSheetOperations();
     void testSheetSelections();
     void testContextMenuCalc();
@@ -106,7 +105,6 @@ public:
     void testWriterCommentInsertCursor();
 
     CPPUNIT_TEST_SUITE(DesktopLOKTest);
-    CPPUNIT_TEST(testModifiedStatus);
     CPPUNIT_TEST(testGetStyles);
     CPPUNIT_TEST(testGetFonts);
     CPPUNIT_TEST(testCreateView);
@@ -755,69 +753,6 @@ void DesktopLOKTest::testWriterComments()
     comphelper::LibreOfficeKit::setActive(false);
 }
 
-void DesktopLOKTest::testModifiedStatus()
-{
-    LibLibreOffice_Impl aOffice;
-    comphelper::LibreOfficeKit::setActive();
-    LibLODocument_Impl* pDocument = loadDoc("blank_text.odt");
-    pDocument->pClass->initializeForRendering(pDocument, nullptr);
-    pDocument->pClass->registerCallback(pDocument, &DesktopLOKTest::callback, 
this);
-
-    // Type "t" and check that the document was set as modified
-    m_bModified = false;
-    m_aStateChangedCondition.reset();
-    pDocument->pClass->postKeyEvent(pDocument, LOK_KEYEVENT_KEYINPUT, 't', 0);
-    Scheduler::ProcessEventsToIdle();
-    TimeValue aTimeValue = { 2 , 0 }; // 2 seconds max
-    m_aStateChangedCondition.wait(&aTimeValue);
-    Scheduler::ProcessEventsToIdle();
-    Scheduler::ProcessEventsToIdle();
-
-    // This was false, there was no callback about the modified status change.
-    CPPUNIT_ASSERT(m_bModified);
-
-    // Perform SaveAs with "TakeOwnership" option set, and check that the
-    // modification state was reset
-    m_aStateChangedCondition.reset();
-    utl::TempFile aTempFile;
-    aTempFile.EnableKillingFile();
-    CPPUNIT_ASSERT(pDocument->pClass->saveAs(pDocument, 
aTempFile.GetURL().toUtf8().getStr(), "odt", "TakeOwnership"));
-    Scheduler::ProcessEventsToIdle();
-    m_aStateChangedCondition.wait(&aTimeValue);
-    Scheduler::ProcessEventsToIdle();
-    Scheduler::ProcessEventsToIdle();
-
-    // There was no callback about the modified status change.
-    CPPUNIT_ASSERT(!m_bModified);
-
-    // Modify the document again
-    m_aStateChangedCondition.reset();
-    pDocument->pClass->postKeyEvent(pDocument, LOK_KEYEVENT_KEYINPUT, 't', 0);
-    Scheduler::ProcessEventsToIdle();
-    m_aStateChangedCondition.wait(&aTimeValue);
-    Scheduler::ProcessEventsToIdle();
-    Scheduler::ProcessEventsToIdle();
-
-    // There was no callback about the modified status change.
-    CPPUNIT_ASSERT(m_bModified);
-
-    /*
-    // TODO: move this to a test where LOK is fully bootstrapped, so that we 
can
-    // get back the notification about ".uno:Save" too
-    // Now perform a normal "Save", and check the modified state was reset
-    // again
-    m_aStateChangedCondition.reset();
-    pDocument->pClass->postUnoCommand(pDocument, ".uno:Save", nullptr, false);
-    m_aStateChangedCondition.wait(&aTimeValue);
-    Scheduler::ProcessEventsToIdle();
-
-    // There was no callback about the modified status change.
-    CPPUNIT_ASSERT(!m_bModified);
-    */
-
-    comphelper::LibreOfficeKit::setActive(false);
-}
-
 void DesktopLOKTest::testTrackChanges()
 {
     // Load a document and create two views.
diff --git a/sfx2/source/control/bindings.cxx b/sfx2/source/control/bindings.cxx
index 4f5d0de..c27f1ae 100644
--- a/sfx2/source/control/bindings.cxx
+++ b/sfx2/source/control/bindings.cxx
@@ -1569,9 +1569,7 @@ bool SfxBindings::NextJob_Impl(Timer * pTimer)
     }
 
     // if possible Update all server / happens in its own time slice
-    // but process all events at once when unit testing, for reliability 
reasons
-    static bool bTest = getenv("LO_TESTNAME");
-    if ( pImp->bMsgDirty && !bTest )
+    if ( pImp->bMsgDirty )
     {
         UpdateSlotServer_Impl();
         return false;
commit 644c93970293c4fee0d5de7a92e8e22e9da259f9
Author: Miklos Vajna <vmik...@collabora.co.uk>
Date:   Wed Oct 19 09:45:07 2016 +0200

    CppunitTest_desktop_lib: fix stack-use-after-return
    
    The LOK callback objects registered are on-stack objects, so we must be
    sure that references no longer used after we return.
    
    Process idle events, so that desktop::CallbackHandler::flush() won't try
    to use the reference after return.
    
    Change-Id: I65d324cb9cef4fd1776d0f610d8d096717e0e833
    Reviewed-on: https://gerrit.libreoffice.org/30034
    Reviewed-by: Miklos Vajna <vmik...@collabora.co.uk>
    Tested-by: Jenkins <c...@libreoffice.org>
    (cherry picked from commit 0c34cd41c65973f8ff0e78c76cd6ad52ef89bc4a)

diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx 
b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index bbf17d7..990909f 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -1408,6 +1408,7 @@ void DesktopLOKTest::testPaintPartTile()
     // the first view, so there were no invalidations.
     CPPUNIT_ASSERT(aView1.m_bTilesInvalidated);
 
+    Scheduler::ProcessEventsToIdle();
     mxComponent->dispose();
     mxComponent.clear();
     comphelper::LibreOfficeKit::setActive(false);
@@ -1447,6 +1448,7 @@ void DesktopLOKTest::testWriterCommentInsertCursor()
     // inserted the comment.
     CPPUNIT_ASSERT(aView1.m_aOwnCursor.IsEmpty());
 
+    Scheduler::ProcessEventsToIdle();
     mxComponent->dispose();
     mxComponent.clear();
     comphelper::LibreOfficeKit::setActive(false);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to