[Libreoffice-commits] core.git: 4 commits - svtools/CppunitTest_svtools_graphic.mk svtools/qa svx/source sw/CppunitTest_sw_odfexport.mk sw/qa sw/source

Zolnai Tamás tamas.zolnai at collabora.com
Sun Nov 16 11:33:42 PST 2014


 svtools/CppunitTest_svtools_graphic.mk                                    |   40 +++
 svtools/qa/unit/GraphicObjectTest.cxx                                     |  108 +++++++++-
 svtools/qa/unit/data/document_with_two_images.odt                         |binary
 svx/source/svdraw/svdograf.cxx                                            |    2 
 sw/CppunitTest_sw_odfexport.mk                                            |    4 
 sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt |binary
 sw/qa/extras/odfexport/odfexport.cxx                                      |   50 ++++
 sw/source/core/graphic/ndgrf.cxx                                          |   49 ----
 8 files changed, 207 insertions(+), 46 deletions(-)

New commits:
commit b0c97aecb22cfa8d845cdc7d2764a4320b53baf6
Author: Zolnai Tamás <tamas.zolnai at collabora.com>
Date:   Sun Nov 16 15:23:54 2014 +0100

    Set back these lines, later it can be useful
    
    Removed in:
    9dc3b49c891fb9fe45c24de4b7e1e88fe400afe0
    
    Change-Id: Id8cee4e17d214ca0eaa5cd11dc25849e5f68851e

diff --git a/sw/source/core/graphic/ndgrf.cxx b/sw/source/core/graphic/ndgrf.cxx
index 682546e..ff601f5 100644
--- a/sw/source/core/graphic/ndgrf.cxx
+++ b/sw/source/core/graphic/ndgrf.cxx
@@ -663,6 +663,10 @@ bool SwGrfNode::SavePersistentData()
         return true;
     }
 
+    // swap in first if in storage
+    if( HasEmbeddedStreamName() && !SwapIn() )
+        return false;
+
     // #i44367#
     // Do not delete graphic file in storage, because the graphic file could
     // be referenced by other graphic nodes.
commit a6e2e54c7e9ec5b852db3b3be578d79fda666601
Author: Zolnai Tamás <tamas.zolnai at collabora.com>
Date:   Sun Nov 16 15:17:43 2014 +0100

    Avoid DelStreamName because it can lead to image loss.
    
    See also:
    f811e628411bda29a76ebb1f72eb107ce67d27f0
    
    The problem is that more images can have the same stream name
    so we can't decide here when to remove one stream name. Better
    to leak in the storage as to loose images (actually we already
    leak here, so)
    
    Change-Id: I2c2afe87e024c2521fe22d62126b567931604101

diff --git a/sw/source/core/graphic/ndgrf.cxx b/sw/source/core/graphic/ndgrf.cxx
index c6013af..682546e 100644
--- a/sw/source/core/graphic/ndgrf.cxx
+++ b/sw/source/core/graphic/ndgrf.cxx
@@ -223,20 +223,12 @@ bool SwGrfNode::ReRead(
     }
     else if( pGraphic && rGrfName.isEmpty() )
     {
-        // Old stream must be deleted before the new one is set.
-        if( HasEmbeddedStreamName() )
-            DelStreamName();
-
         maGrfObj.SetGraphic( *pGraphic );
         onGraphicChanged();
         bReadGrf = true;
     }
     else if( pGrfObj && rGrfName.isEmpty() )
     {
-        // Old stream must be deleted before the new one is set.
-        if( HasEmbeddedStreamName() )
-            DelStreamName();
-
         maGrfObj = *pGrfObj;
         onGraphicChanged();
         bReadGrf = true;
@@ -246,9 +238,6 @@ bool SwGrfNode::ReRead(
         return true;
     else
     {
-        if( HasEmbeddedStreamName() )
-            DelStreamName();
-
         // create new link for the graphic object
         InsertLink( rGrfName, rFltName );
 
@@ -859,36 +848,6 @@ void SwGrfNode::ScaleImageMap()
     }
 }
 
-void SwGrfNode::DelStreamName()
-{
-    if( HasEmbeddedStreamName() )
-    {
-        // then remove graphic from storage
-        uno::Reference < embed::XStorage > xDocStg = GetDoc()->GetDocStorage();
-        if( xDocStg.is() )
-        {
-            try
-            {
-                const StreamAndStorageNames aNames = lcl_GetStreamStorageNames( maGrfObj.GetUserData() );
-                uno::Reference < embed::XStorage > refPics = xDocStg;
-                if ( !aNames.sStorage.isEmpty() )
-                    refPics = xDocStg->openStorageElement( aNames.sStorage, embed::ElementModes::READWRITE );
-                refPics->removeElement( aNames.sStream );
-                uno::Reference < embed::XTransactedObject > xTrans( refPics, uno::UNO_QUERY );
-                if ( xTrans.is() )
-                    xTrans->commit();
-            }
-            catch (const uno::Exception&)
-            {
-                // #i48434#
-                OSL_FAIL( "<SwGrfNode::DelStreamName()> - unhandled exception!" );
-            }
-        }
-
-        maGrfObj.SetUserData();
-    }
-}
-
 /** helper method to get a substorage of the document storage for readonly access.
 
     OD, MAV 2005-08-17 #i53025#
commit 286e2f5c6ec829bc0987b1be7016699f7ef03e5e
Author: Zolnai Tamás <tamas.zolnai at collabora.com>
Date:   Sun Nov 16 15:12:54 2014 +0100

    Related fdo#82953: Forget package URL of image after it is loaded
    
    It causes problems if we handle those imported images differently which
    are identified by a package URL, so after the first load remove
    this URL and handle images on the same way as inserted images.
    
    Some related bugs:
    * #i44367#
    * #i124946#
    * #i114361#
    * fdo#73270
    
    The image in the test document has a special ID which is different
    from that one which is generated by LO internally so after ODF export
    the new generated image URL is different from the imported one.
    
    Change-Id: I4e7d3490674c5f86bec5c7c6e1c975dcafd7c265

diff --git a/svx/source/svdraw/svdograf.cxx b/svx/source/svdraw/svdograf.cxx
index c2bbeb1..40d652b 100644
--- a/svx/source/svdraw/svdograf.cxx
+++ b/svx/source/svdraw/svdograf.cxx
@@ -1351,7 +1351,7 @@ IMPL_LINK( SdrGrafObj, ImpSwapHdl, GraphicObject*, pO )
                         const OUString aNewUserData( pGraphic->GetUserData() );
 
                         pGraphic->SetGraphic( aGraphic );
-                        pGraphic->SetUserData( aNewUserData );
+                        pGraphic->SetUserData();
 
                         // Graphic successfully swapped in.
                         pRet = GRFMGR_AUTOSWAPSTREAM_LOADED;
diff --git a/sw/CppunitTest_sw_odfexport.mk b/sw/CppunitTest_sw_odfexport.mk
index 080a840..97398af 100644
--- a/sw/CppunitTest_sw_odfexport.mk
+++ b/sw/CppunitTest_sw_odfexport.mk
@@ -80,6 +80,10 @@ $(eval $(call gb_CppunitTest_use_components,sw_odfexport,\
     xmloff/util/xo \
 ))
 
+$(eval $(call gb_CppunitTest_use_custom_headers,sw_odfexport,\
+    officecfg/registry \
+))
+
 $(eval $(call gb_CppunitTest_use_configuration,sw_odfexport))
 
 $(eval $(call gb_CppunitTest_use_unittest_configuration,sw_odfexport))
diff --git a/sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt b/sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt
new file mode 100644
index 0000000..867c075
Binary files /dev/null and b/sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt differ
diff --git a/sw/qa/extras/odfexport/odfexport.cxx b/sw/qa/extras/odfexport/odfexport.cxx
index c140540..d7360ba 100644
--- a/sw/qa/extras/odfexport/odfexport.cxx
+++ b/sw/qa/extras/odfexport/odfexport.cxx
@@ -17,6 +17,9 @@
 #include <com/sun/star/text/RelOrientation.hpp>
 #include <com/sun/star/text/XDocumentIndex.hpp>
 #include <com/sun/star/drawing/TextVerticalAdjust.hpp>
+#include <com/sun/star/awt/XBitmap.hpp>
+#include <com/sun/star/graphic/XGraphic.hpp>
+#include <officecfg/Office/Common.hxx>
 
 class Test : public SwModelTestBase
 {
@@ -401,6 +404,53 @@ DECLARE_ODFEXPORT_TEST(testTextboxRoundedCorners, "textbox-rounded-corners.odt")
     CPPUNIT_ASSERT_EQUAL(OUString("a"), xCell->getString());
 }
 
+DECLARE_ODFEXPORT_TEST(testImageWithSpecialID, "document_with_two_images_with_special_IDs.odt")
+{
+    // Here the problem was that LO did not handle well those image URLs in the ODT file which are
+    // not match with that one which is generated by LO internaly (based on the image's size, type and so on)
+
+    // Trigger swap out mechanism to test swapped state factor too.
+    boost::shared_ptr< comphelper::ConfigurationChanges > batch(comphelper::ConfigurationChanges::create());
+    officecfg::Office::Common::Cache::GraphicManager::TotalCacheSize::set(sal_Int32(1), batch);
+    batch->commit();
+
+    uno::Reference<drawing::XShape> xImage = getShape(1);
+    uno::Reference< beans::XPropertySet > XPropSet( xImage, uno::UNO_QUERY_THROW );
+    // Check URL
+    {
+        OUString sURL;
+        XPropSet->getPropertyValue("GraphicURL") >>= sURL;
+        CPPUNIT_ASSERT(sURL != OUString("vnd.sun.star.GraphicObject:00000000000000000000000000000000"));
+    }
+    // Check size
+    {
+        uno::Reference<graphic::XGraphic> xGraphic;
+        XPropSet->getPropertyValue("Graphic") >>= xGraphic;
+        uno::Reference<awt::XBitmap> xBitmap(xGraphic, uno::UNO_QUERY);
+        CPPUNIT_ASSERT(xBitmap.is());
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(610), xBitmap->getSize().Width );
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(381), xBitmap->getSize().Height );
+    }
+    // Second Image
+    xImage = getShape(2);
+    XPropSet.set( xImage, uno::UNO_QUERY_THROW );
+    // Check URL
+    {
+        OUString sURL;
+        XPropSet->getPropertyValue("GraphicURL") >>= sURL;
+        CPPUNIT_ASSERT(sURL != OUString("vnd.sun.star.GraphicObject:00000000000000000000000000000000"));
+    }
+    // Check size
+    {
+        uno::Reference<graphic::XGraphic> xGraphic;
+        XPropSet->getPropertyValue("Graphic") >>= xGraphic;
+        uno::Reference<awt::XBitmap> xBitmap(xGraphic, uno::UNO_QUERY);
+        CPPUNIT_ASSERT(xBitmap.is());
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(900), xBitmap->getSize().Width );
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(600), xBitmap->getSize().Height );
+    }
+}
+
 #endif
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/source/core/graphic/ndgrf.cxx b/sw/source/core/graphic/ndgrf.cxx
index f9cbb9c..c6013af 100644
--- a/sw/source/core/graphic/ndgrf.cxx
+++ b/sw/source/core/graphic/ndgrf.cxx
@@ -581,6 +581,10 @@ bool SwGrfNode::SwapIn( bool bWaitForData )
                 {
                     bRet = ImportGraphic( *pStrm );
                     delete pStrm;
+                    if( bRet )
+                    {
+                        maGrfObj.SetUserData();
+                    }
                 }
             }
             catch (const uno::Exception&)
commit c454a0cd0cc7b315b1353b151a2e95654df72c69
Author: Zolnai Tamás <tamas.zolnai at collabora.com>
Date:   Fri Nov 7 16:44:54 2014 +0100

    Test for size based auto swap out mechanism
    
    Change-Id: Iff0942b9b545f27dd74b73bee3f8ac785539867d

diff --git a/svtools/CppunitTest_svtools_graphic.mk b/svtools/CppunitTest_svtools_graphic.mk
index ef9c7ab..6036afb 100644
--- a/svtools/CppunitTest_svtools_graphic.mk
+++ b/svtools/CppunitTest_svtools_graphic.mk
@@ -11,6 +11,7 @@ $(eval $(call gb_CppunitTest_CppunitTest,svtools_graphic))
 
 $(eval $(call gb_CppunitTest_use_externals,svtools_graphic,\
 	boost_headers \
+    libxml2 \
 ))
 
 $(eval $(call gb_CppunitTest_use_api,svtools_graphic, \
@@ -29,18 +30,53 @@ $(eval $(call gb_CppunitTest_use_libraries,svtools_graphic, \
 	tl \
 	sal \
 	svt \
+	sw \
 	test \
 	unotest \
 	vcl \
     $(gb_UWINAPI) \
 ))
 
+$(eval $(call gb_CppunitTest_set_include,svtools_graphic,\
+    -I$(SRCDIR)/sw/inc \
+    $$(INCLUDE) \
+))
+
+$(eval $(call gb_CppunitTest_use_custom_headers,svtools_graphic,\
+	officecfg/registry \
+))
+
 $(eval $(call gb_CppunitTest_use_configuration,svtools_graphic))
 
 $(eval $(call gb_CppunitTest_use_components,svtools_graphic,\
-	configmgr/source/configmgr \
-	ucb/source/core/ucb1 \
+    basic/util/sb \
+    comphelper/util/comphelp \
+    configmgr/source/configmgr \
+    embeddedobj/util/embobj \
+    filter/source/config/cache/filterconfig1 \
+    filter/source/storagefilterdetect/storagefd \
+    framework/util/fwk \
+    i18npool/util/i18npool \
+    linguistic/source/lng \
+    oox/util/oox \
+    package/source/xstor/xstor \
+    package/util/package2 \
+    sax/source/expatwrap/expwrap \
+    sfx2/util/sfx \
+    starmath/util/sm \
+    svl/source/fsstor/fsstorage \
+    svtools/util/svt \
+    sw/util/sw \
+    sw/util/swd \
+    toolkit/util/tk \
+    ucb/source/core/ucb1 \
     ucb/source/ucp/file/ucpfile1 \
+    unotools/util/utl \
+    unoxml/source/service/unoxml \
+    uui/util/uui \
+    writerfilter/util/writerfilter \
+    $(if $(filter DESKTOP,$(BUILD_TYPE)),xmlhelp/util/ucpchelp1) \
+    xmloff/util/xo \
 ))
 
 $(eval $(call gb_CppunitTest_add_exception_objects,svtools_graphic, \
diff --git a/svtools/qa/unit/GraphicObjectTest.cxx b/svtools/qa/unit/GraphicObjectTest.cxx
index 29259c4..34e26fd 100644
--- a/svtools/qa/unit/GraphicObjectTest.cxx
+++ b/svtools/qa/unit/GraphicObjectTest.cxx
@@ -19,21 +19,42 @@
 
 #include <vcl/image.hxx>
 
+#include <com/sun/star/frame/Desktop.hpp>
+#include <officecfg/Office/Common.hxx>
+#include <unotest/macros_test.hxx>
+#include <comphelper/processfactory.hxx>
+#include <unotxdoc.hxx>
+#include <docsh.hxx>
+#include <doc.hxx>
+#include <ndgrf.hxx>
+#include <boost/shared_ptr.hpp>
+
+using namespace css;
+
 namespace
 {
 
-class GraphicObjectTest: public test::BootstrapFixture
+class GraphicObjectTest: public test::BootstrapFixture, public unotest::MacrosTest
 {
 
 public:
     void testSwap();
+    void testSizeBasedAutoSwap();
+
+
+    virtual void setUp() SAL_OVERRIDE
+    {
+        test::BootstrapFixture::setUp();
+
+        mxDesktop.set(css::frame::Desktop::create(comphelper::getComponentContext(getMultiServiceFactory())));
+    }
 
 private:
     DECL_LINK(getLinkStream, GraphicObject*);
 
 private:
     CPPUNIT_TEST_SUITE(GraphicObjectTest);
-    CPPUNIT_TEST(testSwap);
+    CPPUNIT_TEST(testSizeBasedAutoSwap);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -120,6 +141,89 @@ void GraphicObjectTest::testSwap()
     }
 }
 
+void GraphicObjectTest::testSizeBasedAutoSwap()
+{
+    // Set cache size to a very small value to check what happens
+    {
+        boost::shared_ptr< comphelper::ConfigurationChanges > aBatch(comphelper::ConfigurationChanges::create());
+        officecfg::Office::Common::Cache::GraphicManager::TotalCacheSize::set(sal_Int32(1), aBatch);
+        aBatch->commit();
+    }
+
+    uno::Reference< lang::XComponent > xComponent =
+        loadFromDesktop(getURLFromSrc("svtools/qa/unit/data/document_with_two_images.odt"), "com.sun.star.text.TextDocument");
+
+    SwXTextDocument* pTxtDoc = dynamic_cast<SwXTextDocument *>(xComponent.get());
+    CPPUNIT_ASSERT(pTxtDoc);
+    SwDoc* pDoc = pTxtDoc->GetDocShell()->GetDoc();
+    CPPUNIT_ASSERT(pDoc);
+    SwNodes& aNodes = pDoc->GetNodes();
+
+    // Find images
+    const GraphicObject* pGrafObj1 = 0;
+    const GraphicObject* pGrafObj2 = 0;
+    for( sal_uLong nIndex = 0; nIndex < aNodes.Count(); ++nIndex)
+    {
+        if( aNodes[nIndex]->IsGrfNode() )
+        {
+            SwGrfNode* pGrfNode = aNodes[nIndex]->GetGrfNode();
+            if( !pGrafObj1 )
+            {
+                pGrafObj1 = &pGrfNode->GetGrfObj();
+            }
+            else
+            {
+                pGrafObj2 = &pGrfNode->GetGrfObj();
+            }
+        }
+    }
+    CPPUNIT_ASSERT_MESSAGE("Missing image", pGrafObj1 != 0 && pGrafObj2 != 0);
+
+    {
+        // First image should be swapped out
+        CPPUNIT_ASSERT(pGrafObj1->IsSwappedOut());
+        CPPUNIT_ASSERT_EQUAL(sal_uLong(697230), pGrafObj1->GetSizeBytes());
+
+        // Still swapped out: size is cached
+        CPPUNIT_ASSERT(pGrafObj1->IsSwappedOut());
+    }
+
+    {
+        // Second image should be in the memory
+        // Size based swap out is triggered by swap in, so the last swapped in image should be
+        // in the memory despite of size limit is reached.
+        CPPUNIT_ASSERT(!pGrafObj2->IsSwappedOut());
+        CPPUNIT_ASSERT_EQUAL(sal_uLong(1620000), pGrafObj2->GetSizeBytes());
+    }
+
+    // Swap in first image -> second image will be swapped out
+    {
+        pGrafObj1->GetGraphic(); // GetGraphic calls swap in on a const object
+        CPPUNIT_ASSERT(!pGrafObj1->IsSwappedOut());
+        CPPUNIT_ASSERT(pGrafObj2->IsSwappedOut());
+    }
+
+    // Swap in second image -> first image will be swapped out
+    {
+        pGrafObj2->GetGraphic(); // GetGraphic calls swap in on a const object
+        CPPUNIT_ASSERT(!pGrafObj2->IsSwappedOut());
+        CPPUNIT_ASSERT(pGrafObj1->IsSwappedOut());
+    }
+
+    // Use bigger cache
+    {
+        GraphicManager& rGrfMgr = pGrafObj1->GetGraphicManager();
+        rGrfMgr.SetMaxCacheSize(pGrafObj1->GetSizeBytes()+pGrafObj2->GetSizeBytes());
+    }
+    // Swap in both images -> both should be swapped in
+    {
+        pGrafObj1->GetGraphic();
+        pGrafObj2->GetGraphic();
+        CPPUNIT_ASSERT(!pGrafObj1->IsSwappedOut());
+        CPPUNIT_ASSERT(!pGrafObj2->IsSwappedOut());
+    }
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(GraphicObjectTest);
 
 }
diff --git a/svtools/qa/unit/data/document_with_two_images.odt b/svtools/qa/unit/data/document_with_two_images.odt
new file mode 100644
index 0000000..54d3d66
Binary files /dev/null and b/svtools/qa/unit/data/document_with_two_images.odt differ


More information about the Libreoffice-commits mailing list