LOK: conditionally include part number in invalidation payload

Since desktop/ code queues, compresses and only emits callbacks on idle,
it's possible that two invalidations are in the queue, and there was a
setPart() call between them. In this case it's impossible to tell what
part the invalidation was sent for.

Fix this by conditionally including the part number in the invalidation
payload. It's off by default, a new feature flag is added to request
this behavior.

gtktiledviewer enables this feature flag by default, though just to show
the part number in the debug output. Android doesn't enable it.

Change-Id: I73e6def848c0eb61d64e71026002c7a0e750aab4

2 files changed, 118 insertions, 13 deletions

diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index 1425c1cf6e84..cb2e034fd2d3 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -33,6 +33,7 @@
 #include <sfx2/viewfrm.hxx>
 #include <sfx2/bindings.hxx>
 #include <comphelper/string.hxx>
+#include <comphelper/scopeguard.hxx>
 #include <cairo.h>
 
 #include <lib/init.hxx>
@@ -97,6 +98,7 @@ public:
     void testContextMenuWriter();
     void testContextMenuImpress();
     void testNotificationCompression();
+    void testPartInInvalidation();
     void testRedlineWriter();
     void testTrackChanges();
     void testRedlineCalc();
@@ -129,6 +131,7 @@ public:
     CPPUNIT_TEST(testContextMenuWriter);
     CPPUNIT_TEST(testContextMenuImpress);
     CPPUNIT_TEST(testNotificationCompression);
+    CPPUNIT_TEST(testPartInInvalidation);
     CPPUNIT_TEST(testRedlineWriter);
     CPPUNIT_TEST(testTrackChanges);
     CPPUNIT_TEST(testRedlineCalc);
@@ -1427,6 +1430,77 @@ void DesktopLOKTest::testNotificationCompression()
     CPPUNIT_ASSERT_EQUAL(std::string("1"), std::get<1>(notifs[i++]));
 }
 
+void DesktopLOKTest::testPartInInvalidation()
+{
+    LibLODocument_Impl* pDocument = loadDoc("blank_text.odt");
+    // No part in invalidation: merge.
+    {
+        std::vector<std::tuple<int, std::string>> notifs;
+        std::unique_ptr<CallbackFlushHandler> handler(new CallbackFlushHandler(pDocument, callbackCompressionTest, &notifs));
+
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "10, 10, 20, 10");
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "20, 10, 20, 10");
+
+        Scheduler::ProcessEventsToIdle();
+
+        CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), notifs.size());
+
+        CPPUNIT_ASSERT_EQUAL((int)LOK_CALLBACK_INVALIDATE_TILES, (int)std::get<0>(notifs[0]));
+        CPPUNIT_ASSERT_EQUAL(std::string("10, 10, 30, 10"), std::get<1>(notifs[0]));
+    }
+    // No part in invalidation: don't merge.
+    {
+        std::vector<std::tuple<int, std::string>> notifs;
+        std::unique_ptr<CallbackFlushHandler> handler(new CallbackFlushHandler(pDocument, callbackCompressionTest, &notifs));
+
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "10, 10, 20, 10");
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "40, 10, 20, 10");
+
+        Scheduler::ProcessEventsToIdle();
+
+        CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), notifs.size());
+    }
+
+    // Part in invalidation, intersection and parts match -> merge.
+    {
+        comphelper::LibreOfficeKit::setPartInInvalidation(true);
+        comphelper::ScopeGuard aGuard([]()
+        {
+            comphelper::LibreOfficeKit::setPartInInvalidation(false);
+        });
+
+        std::vector<std::tuple<int, std::string>> notifs;
+        std::unique_ptr<CallbackFlushHandler> handler(new CallbackFlushHandler(pDocument, callbackCompressionTest, &notifs));
+
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "10, 10, 20, 10, 0");
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "20, 10, 20, 10, 0");
+
+        Scheduler::ProcessEventsToIdle();
+
+        CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), notifs.size());
+    }
+    // Part in invalidation, intersection and parts don't match -> don't merge.
+    {
+        comphelper::LibreOfficeKit::setPartInInvalidation(true);
+        comphelper::ScopeGuard aGuard([]()
+        {
+            comphelper::LibreOfficeKit::setPartInInvalidation(false);
+        });
+
+        std::vector<std::tuple<int, std::string>> notifs;
+        std::unique_ptr<CallbackFlushHandler> handler(new CallbackFlushHandler(pDocument, callbackCompressionTest, &notifs));
+
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "10, 10, 20, 10, 0");
+        handler->queue(LOK_CALLBACK_INVALIDATE_TILES, "20, 10, 20, 10, 1");
+
+        Scheduler::ProcessEventsToIdle();
+
+        // This failed as RectangleAndPart::Create() always assumed no part in
+        // payload, so this was merged -> it was 1.
+        CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), notifs.size());
+    }
+}
+
 void DesktopLOKTest::testRedlineWriter()
 {
     // Load a Writer document, enable change recording and press a key.
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index dfaf159d4968..156fc14e467f 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -326,15 +326,42 @@ static boost::property_tree::ptree unoAnyToPropertyTree(const uno::Any& anyItem)
 
 namespace {
 
-Rectangle lcl_ParseRect(const std::string& payload)
+/// Represents an invalidated rectangle inside a given document part.
+struct RectangleAndPart
 {
-    std::istringstream iss(payload);
-    long left, top, right, bottom;
-    char comma;
-    iss >> left >> comma >> top >> comma >> right >> comma >> bottom;
-    Rectangle rc(left, top, left + right, top + bottom);
-    return rc;
-}
+    Rectangle m_aRectangle;
+    int m_nPart;
+
+    RectangleAndPart()
+        : m_nPart(-1)
+    {
+    }
+
+    OString toString() const
+    {
+        std::stringstream ss;
+        ss << m_aRectangle.toString().getStr();
+        if (m_nPart != -1)
+            ss << ", " << m_nPart;
+        return ss.str().c_str();
+    }
+
+    static RectangleAndPart Create(const std::string& rPayload)
+    {
+        std::istringstream aStream(rPayload);
+        long nLeft, nTop, nRight, nBottom;
+        long nPart = -1;
+        char nComma;
+        if (comphelper::LibreOfficeKit::isPartInInvalidation())
+            aStream >> nLeft >> nComma >> nTop >> nComma >> nRight >> nComma >> nBottom >> nComma >> nPart;
+        else
+            aStream >> nLeft >> nComma >> nTop >> nComma >> nRight >> nComma >> nBottom;
+        RectangleAndPart aRet;
+        aRet.m_aRectangle = Rectangle(nLeft, nTop, nLeft + nRight, nTop + nBottom);
+        aRet.m_nPart = nPart;
+        return aRet;
+    }
+};
 
 bool lcl_isViewCallbackType(const int type)
 {
@@ -700,7 +727,7 @@ void CallbackFlushHandler::queue(const int type, const char* data)
 
             case LOK_CALLBACK_INVALIDATE_TILES:
             {
-                Rectangle rcNew = lcl_ParseRect(payload);
+                RectangleAndPart rcNew = RectangleAndPart::Create(payload);
                 //SAL_WARN("lok", "New: " << rcNew.toString());
                 const auto rcOrig = rcNew;
 
@@ -708,15 +735,17 @@ void CallbackFlushHandler::queue(const int type, const char* data)
                     [type, &rcNew] (const queue_type::value_type& elem) {
                         if (elem.first == type)
                         {
-                            const Rectangle rcOld = lcl_ParseRect(elem.second);
+                            const RectangleAndPart rcOld = RectangleAndPart::Create(elem.second);
+                            if (rcOld.m_nPart != rcNew.m_nPart)
+                                return false;
                             //SAL_WARN("lok", "#" << i << " Old: " << rcOld.toString());
-                            const Rectangle rcOverlap = rcNew.GetIntersection(rcOld);
+                            const Rectangle rcOverlap = rcNew.m_aRectangle.GetIntersection(rcOld.m_aRectangle);
                             //SAL_WARN("lok", "#" << i << " Overlap: " << rcOverlap.toString());
                             bool bOverlap = (rcOverlap.GetWidth() > 0 && rcOverlap.GetHeight() > 0);
                             if (bOverlap)
                             {
                                 //SAL_WARN("lok", rcOld.toString() << " U " << rcNew.toString());
-                                rcNew.Union(rcOld);
+                                rcNew.m_aRectangle.Union(rcOld.m_aRectangle);
                             }
                             return bOverlap;
                         }
@@ -727,7 +756,7 @@ void CallbackFlushHandler::queue(const int type, const char* data)
                     }
                 );
 
-                if (rcNew != rcOrig)
+                if (rcNew.m_aRectangle != rcOrig.m_aRectangle)
                 {
                     SAL_WARN("lok", "Replacing: " << rcOrig.toString() << " by " << rcNew.toString());
                     payload = rcNew.toString().getStr();
@@ -2442,6 +2471,8 @@ static void lo_setOptionalFeatures(LibreOfficeKit* pThis, uint64_t const feature
 {
     LibLibreOffice_Impl *const pLib = static_cast<LibLibreOffice_Impl*>(pThis);
     pLib->mOptionalFeatures = features;
+    if (features & LOK_FEATURE_PART_IN_INVALIDATION_CALLBACK)
+        comphelper::LibreOfficeKit::setPartInInvalidation(true);
 }
 
 static void lo_setDocumentPassword(LibreOfficeKit* pThis,