[Libreoffice-commits] core.git: 6 commits - sc/inc sc/qa sc/source

Eike Rathke erack at redhat.com
Thu Mar 9 21:10:58 UTC 2017


 sc/inc/postit.hxx              |   18 ++++++++++-
 sc/qa/unit/ucalc.cxx           |   49 +++++++++++++++++++++++---------
 sc/qa/unit/ucalc_sort.cxx      |    3 +
 sc/source/core/data/postit.cxx |   62 +++++++++++++++++++++++++++++++----------
 4 files changed, 102 insertions(+), 30 deletions(-)

New commits:
commit 646a1d20974ff13b908a85cdff37c2701d582d8f
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Mar 9 22:09:55 2017 +0100

    add/use ScCaptionPtr::removeFromDrawPage()
    
    Change-Id: Ibe073f071b120b61738b7e813a14824248f1fcfc

diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index dfb88c1..c61df47 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -63,6 +63,11 @@ public:
      */
     void insertToDrawPage( SdrPage& rDrawPage );
 
+    /** Remove from draw page. The caption object is not owned anymore by the
+        draw page then.
+     */
+    void removeFromDrawPage( SdrPage& rDrawPage );
+
     /** Release all management of the SdrCaptionObj* in all instances of this
         list and dissolve. The SdrCaptionObj pointer returned is ready to be
         managed elsewhere.
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 8ed2e9f..c01b6d7 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -710,6 +710,20 @@ void ScCaptionPtr::insertToDrawPage( SdrPage& rDrawPage )
     mpHead->mbInDrawPage = true;
 }
 
+void ScCaptionPtr::removeFromDrawPage( SdrPage& rDrawPage )
+{
+    assert(mpHead && mpCaption);
+    SAL_WARN_IF(!mpHead->mbInDrawPage,"sc.core","ScCaptionPtr::removeFromDrawPage - not in draw page");
+    /* FIXME: that should assert, but currently fails in
+     * Test::testCopyToDocument() probably due to CopyStaticToDocument()
+     * lacking something. */
+    //assert(mpHead->mbInDrawPage);   // did we lose track anywhere?
+
+    SdrObject* pObj = rDrawPage.RemoveObject( mpCaption->GetOrdNum() );
+    assert(pObj == mpCaption); (void)pObj;
+    mpHead->mbInDrawPage = false;
+}
+
 SdrCaptionObj* ScCaptionPtr::release()
 {
     SdrCaptionObj* pTmp = mpCaption;
@@ -1081,10 +1095,10 @@ void ScPostIt::RemoveCaption()
             if( bRecording )
                 pDrawLayer->AddCalcUndo( new SdrUndoDelObj( *maNoteData.mxCaption.get() ) );
             // remove the object from the drawing page, delete if undo is disabled
-            SdrObject* pObj = pDrawPage->RemoveObject( maNoteData.mxCaption->GetOrdNum() );
+            maNoteData.mxCaption.removeFromDrawPage( *pDrawPage );
             if( !bRecording )
             {
-                maNoteData.mxCaption.release();
+                SdrObject* pObj = maNoteData.mxCaption.release();
                 SdrObject::Free( pObj );
             }
         }
commit b3354ad2482737b49bd8d7593d355671197c4551
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Mar 9 22:08:38 2017 +0100

    sprinkle some drawing layers over test cases
    
    ... so things actually work like intended and creation of caption objects
    doesn't silently fail. Well, it does SAL_WARN or OSL_ENSURE but that's never
    displayed unless a test fails.
    
    Change-Id: Ibf4cc075cc3d6dadbe8f6208b2949310124b5749

diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index af2e645..dcb9a90 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -790,22 +790,29 @@ void Test::testCopyToDocument()
 
     // Copy statically to another document.
 
-    ScDocument aDestDoc(SCDOCMODE_DOCUMENT);
-    aDestDoc.InsertTab(0, "src");
-    m_pDoc->CopyStaticToDocument(ScRange(0,1,0,0,3,0), 0, &aDestDoc); // Copy A2:A4
-    m_pDoc->CopyStaticToDocument(ScAddress(0,0,0), 0, &aDestDoc); // Copy A1
-    m_pDoc->CopyStaticToDocument(ScRange(0,4,0,0,7,0), 0, &aDestDoc); // Copy A5:A8
-
-    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,0,0), aDestDoc.GetString(0,0,0));
-    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,1,0), aDestDoc.GetString(0,1,0));
-    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,2,0), aDestDoc.GetString(0,2,0));
-    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,3,0), aDestDoc.GetString(0,3,0));
-    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,4,0), aDestDoc.GetString(0,4,0));
+    ScDocShellRef xDocSh2;
+    getNewDocShell(xDocSh2);
+    ScDocument* pDestDoc = &xDocSh2->GetDocument();
+    pDestDoc->InsertTab(0, "src");
+    pDestDoc->InitDrawLayer(xDocSh2.get());     // for note caption objects
+
+    m_pDoc->CopyStaticToDocument(ScRange(0,1,0,0,3,0), 0, pDestDoc); // Copy A2:A4
+    m_pDoc->CopyStaticToDocument(ScAddress(0,0,0), 0,     pDestDoc); // Copy A1
+    m_pDoc->CopyStaticToDocument(ScRange(0,4,0,0,7,0), 0, pDestDoc); // Copy A5:A8
+
+    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,0,0), pDestDoc->GetString(0,0,0));
+    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,1,0), pDestDoc->GetString(0,1,0));
+    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,2,0), pDestDoc->GetString(0,2,0));
+    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,3,0), pDestDoc->GetString(0,3,0));
+    CPPUNIT_ASSERT_EQUAL(m_pDoc->GetString(0,4,0), pDestDoc->GetString(0,4,0));
 
     // verify note
-    CPPUNIT_ASSERT_MESSAGE("There should be a note in A1 destDocument", aDestDoc.HasNote(ScAddress(0, 0, 0)));
+    CPPUNIT_ASSERT_MESSAGE("There should be a note in A1 destDocument", pDestDoc->HasNote(ScAddress(0, 0, 0)));
     CPPUNIT_ASSERT_EQUAL_MESSAGE("The notes content should be the same on both documents",
-            m_pDoc->GetNote(ScAddress(0, 0, 0))->GetText(), aDestDoc.GetNote(ScAddress(0, 0, 0))->GetText());
+            m_pDoc->GetNote(ScAddress(0, 0, 0))->GetText(), pDestDoc->GetNote(ScAddress(0, 0, 0))->GetText());
+
+    pDestDoc->DeleteTab(0);
+    closeDocShell(xDocSh2);
 
     m_pDoc->DeleteTab(0);
 }
@@ -3236,6 +3243,10 @@ void Test::testCopyPaste()
 {
     m_pDoc->InsertTab(0, "Sheet1");
     m_pDoc->InsertTab(1, "Sheet2");
+
+    // We need a drawing layer in order to create caption objects.
+    m_pDoc->InitDrawLayer(&getDocShell());
+
     //test copy&paste + ScUndoPaste
     //copy local and global range names in formulas
     //string cells and value cells
@@ -3451,6 +3462,9 @@ void Test::testCopyPasteTranspose()
     m_pDoc->InsertTab(0, "Sheet1");
     m_pDoc->InsertTab(1, "Sheet2");
 
+    // We need a drawing layer in order to create caption objects.
+    m_pDoc->InitDrawLayer(&getDocShell());
+
     m_pDoc->SetValue(0, 0, 0, 1);
     m_pDoc->SetString(1, 0, 0, "=A1+1");
     m_pDoc->SetString(2, 0, 0, "test");
@@ -4032,6 +4046,9 @@ void Test::testMoveBlock()
 {
     m_pDoc->InsertTab(0, "SheetNotes");
 
+    // We need a drawing layer in order to create caption objects.
+    m_pDoc->InitDrawLayer(&getDocShell());
+
     m_pDoc->SetValue(0, 0, 0, 1);
     m_pDoc->SetString(1, 0, 0, "=A1+1");
     m_pDoc->SetString(2, 0, 0, "test");
@@ -5020,6 +5037,9 @@ void Test::testShiftCells()
 {
     m_pDoc->InsertTab(0, "foo");
 
+    // We need a drawing layer in order to create caption objects.
+    m_pDoc->InitDrawLayer(&getDocShell());
+
     OUString aTestVal("Some Text");
 
     // Text into cell E5.
@@ -5057,6 +5077,9 @@ void Test::testNoteBasic()
 {
     m_pDoc->InsertTab(0, "PostIts");
 
+    // We need a drawing layer in order to create caption objects.
+    m_pDoc->InitDrawLayer(&getDocShell());
+
     CPPUNIT_ASSERT(!m_pDoc->HasNotes());
 
     // Check for note's presence in all tables before inserting any notes.
diff --git a/sc/qa/unit/ucalc_sort.cxx b/sc/qa/unit/ucalc_sort.cxx
index b24a3da..7a071c3 100644
--- a/sc/qa/unit/ucalc_sort.cxx
+++ b/sc/qa/unit/ucalc_sort.cxx
@@ -31,6 +31,9 @@ void Test::testSort()
 {
     m_pDoc->InsertTab(0, "test1");
 
+    // We need a drawing layer in order to create caption objects.
+    m_pDoc->InitDrawLayer(&getDocShell());
+
     ScRange aDataRange;
     ScAddress aPos(0,0,0);
     {
commit 60c5b392b77f15d11dd98890565abc68daee02a8
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Mar 9 19:50:31 2017 +0100

    probably should work like sketched
    
    Change-Id: I5ad52c718cf53c2f3fb14a7970917e0012d01875

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 0864d2d..8ed2e9f 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -683,13 +683,19 @@ void ScCaptionPtr::decRefAndDestroy()
 {
     if (decRef())
     {
-        /* FIXME: this should eventually remove the caption from drawing layer
-         * foo and call SdrObject::Free(), likely with mpHead->mpCaption, see
-         * ScPostIt::RemoveCaption(). Further work needed to be able to do so.
-         * */
         assert(mpHead->mpFirst == this);    // this must be one and only one
-        mpCaption = nullptr;
         assert(!mpNext);                    // this must be one and only one
+        assert(mpCaption);
+        if (mpHead->mbInDrawPage)
+        {
+            /* FIXME: this should eventually remove the caption from drawing layer
+             * foo and call SdrObject::Free(), likely with mpCaption, see
+             * ScPostIt::RemoveCaption(). Further work needed to be able to do so.
+             * */
+        }
+        /* FIXME: once we got ownership right */
+        //SdrObject::Free( mpCaption );
+        mpCaption = nullptr;
         delete mpHead;
         mpHead = nullptr;
     }
commit 5047871d7fe44747ec7f6ee4dd48e1a1cdfafa9d
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Mar 9 16:05:12 2017 +0100

    use ScCaptionPtr::insertToDrawPage()
    
    Change-Id: I98dafdf8e571e4745e05df6cbcbf00fd9ecd8ec6

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 0fa5589..0864d2d 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -419,7 +419,7 @@ ScNoteCaptionCreator::ScNoteCaptionCreator( ScDocument& rDoc, const ScAddress& r
             // store note position in user data of caption object
             ScCaptionUtil::SetCaptionUserData( *rNoteData.mxCaption, rPos );
             // insert object into draw page
-            pDrawPage->InsertObject( rNoteData.mxCaption.get() );
+            rNoteData.mxCaption.insertToDrawPage( *pDrawPage );
         }
     }
 }
@@ -1117,10 +1117,11 @@ ScCaptionPtr ScNoteUtil::CreateTempCaption(
 
     // create the caption object
     ScCaptionCreator aCreator( rDoc, rPos, bTailFront );
-    SdrCaptionObj* pCaption = aCreator.GetCaption().get();  // just for ease of use
 
     // insert caption into page (needed to set caption text)
-    rDrawPage.InsertObject( pCaption );
+    aCreator.GetCaption().insertToDrawPage( rDrawPage );
+
+    SdrCaptionObj* pCaption = aCreator.GetCaption().get();  // just for ease of use
 
     // clone the edit text object, unless user text is present, then set this text
     if( pNoteCaption && rUserText.isEmpty() )
commit 89596a1a000cb355b0e9eddd070e9d840298d85f
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Mar 9 15:22:57 2017 +0100

    add ScCaptionPtr::insertToDrawPage()
    
    Change-Id: I1266b55c2558d306b20b0f2d9fba07b0bc46544e

diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index 7cdb5e0..dfb88c1 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -31,6 +31,7 @@ class EditTextObject;
 class OutlinerParaObject;
 class SdrCaptionObj;
 class SdrPage;
+
 class SfxItemSet;
 class ScDocument;
 class Rectangle;
@@ -58,6 +59,10 @@ public:
     // Does not default to nullptr to make it visually obvious where such is used.
     void reset( SdrCaptionObj* p );
 
+    /** Insert to draw page. The caption object is owned by the draw page then.
+     */
+    void insertToDrawPage( SdrPage& rDrawPage );
+
     /** Release all management of the SdrCaptionObj* in all instances of this
         list and dissolve. The SdrCaptionObj pointer returned is ready to be
         managed elsewhere.
@@ -77,8 +82,12 @@ private:
 
     struct Head
     {
-        ScCaptionPtr*       mpFirst;    ///< first in list
-        oslInterlockedCount mnRefs;     ///< use count
+        ScCaptionPtr*       mpFirst;        ///< first in list
+        oslInterlockedCount mnRefs;         ///< use count
+        bool                mbInDrawPage;   ///< caption object is owned by draw page
+
+        Head() = delete;
+        explicit Head( ScCaptionPtr* );
     };
 
     Head*                 mpHead;       ///< points to the "master" entry
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index dee726c..0fa5589 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -534,12 +534,15 @@ ScCaptionPtr& ScCaptionPtr::operator=( const ScCaptionPtr& r )
     return *this;
 }
 
+ScCaptionPtr::Head::Head( ScCaptionPtr* p ) :
+    mpFirst(p), mnRefs(1), mbInDrawPage(false)
+{
+}
+
 void ScCaptionPtr::newHead()
 {
     assert(!mpHead);
-    mpHead = new Head;
-    mpHead->mpFirst = this;
-    mpHead->mnRefs = 1;
+    mpHead = new Head(this);
 }
 
 void ScCaptionPtr::replaceInList( ScCaptionPtr* pNew )
@@ -692,6 +695,15 @@ void ScCaptionPtr::decRefAndDestroy()
     }
 }
 
+void ScCaptionPtr::insertToDrawPage( SdrPage& rDrawPage )
+{
+    assert(mpHead && mpCaption);
+    assert(!mpHead->mbInDrawPage);  // multiple assignments not possible
+
+    rDrawPage.InsertObject( mpCaption );
+    mpHead->mbInDrawPage = true;
+}
+
 SdrCaptionObj* ScCaptionPtr::release()
 {
     SdrCaptionObj* pTmp = mpCaption;
commit 5f376561549552a22e9a2b8f07ec5f3378b99e73
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Mar 9 14:29:40 2017 +0100

    move assignment onto self should not happen
    
    Change-Id: Ic44f4362762cb1c1fe027b69a78baf768c0a53da

diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 0e840e7..dee726c 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -485,8 +485,7 @@ ScCaptionPtr::ScCaptionPtr( ScCaptionPtr&& r ) :
 
 ScCaptionPtr& ScCaptionPtr::operator=( ScCaptionPtr&& r )
 {
-    if (this == &r)
-        return *this;
+    assert(this != &r);
 
     mpHead = r.mpHead;
     mpNext = r.mpNext;


More information about the Libreoffice-commits mailing list