From c327bb5c88573c96f22e9a8cfc4b8a733ae6b671 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Tue, 19 Apr 2016 21:26:42 -0400 Subject: New LOKDocument callback queue to flush events lazily on idle Since desktop now queues up callback notifications and flushes them to the client on idle, the unit-tests must yield and process all tasks before they validate post-conditions. (cherry picked from commit e6a429770bde5da75239961ae88c06c78cfa5686) (cherry picked from commit 1f278848117080cd6e819f04ba428be52416af7c) (cherry picked from commit 6ca6f22777eb3651109cbf403577d0022a735c9b) (cherry picked from commit 548faf728cf097d93c3f6478ceea5f8747e789c6) Change-Id: I78307db29a9ce647ffaed3539f953227c605968e Reviewed-on: https://gerrit.libreoffice.org/24377 Tested-by: Jenkins Reviewed-by: Ashod Nakashian --- desktop/inc/lib/init.hxx | 112 +++++++++++++++++++++++++-- desktop/qa/desktop_lib/test_desktop_lib.cxx | 10 +++ desktop/source/lib/init.cxx | 35 ++++----- desktop/source/lib/lokinteractionhandler.cxx | 4 +- 4 files changed, 134 insertions(+), 27 deletions(-) (limited to 'desktop') diff --git a/desktop/inc/lib/init.hxx b/desktop/inc/lib/init.hxx index 5b386e4820d7..5c987bec57ab 100644 --- a/desktop/inc/lib/init.hxx +++ b/desktop/inc/lib/init.hxx @@ -10,24 +10,126 @@ #ifndef INCLUDED_DESKTOP_INC_LIB_INIT_HXX #define INCLUDED_DESKTOP_INC_LIB_INIT_HXX +#include +#include +#include + +#include +#include #include #include #include #include -#include -#include + #include -#include class LOKInteractionHandler; namespace desktop { + + class CallbackFlushHandler : public Idle + { + public: + explicit CallbackFlushHandler(LibreOfficeKitCallback pCallback, void* pData) + : Idle( "lokit timer callback" ), + m_pCallback(pCallback), + m_pData(pData) + { + SetPriority(SchedulerPriority::POST_PAINT); + + // Add the states that are safe to skip duplicates on, + // even when not consequent. + m_states.emplace(LOK_CALLBACK_TEXT_SELECTION, "NIL"); + m_states.emplace(LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR, "NIL"); + m_states.emplace(LOK_CALLBACK_STATE_CHANGED, "NIL"); + + Start(); + } + + virtual ~CallbackFlushHandler() + { + Stop(); + + // We might have important notification (.uno:save?). + flush(); + } + + virtual void Invoke() override + { + flush(); + } + + static + void callback(const int type, const char* payload, void* data) + { + CallbackFlushHandler* self = reinterpret_cast(data); + if (self) + { + self->queue(type, payload); + } + } + + void queue(const int type, const char* data) + { + const std::string payload(data ? data : "(nil)"); + std::unique_lock lock(m_mutex); + + const auto stateIt = m_states.find(type); + if (stateIt != m_states.end()) + { + // If the state didn't change, it's safe to ignore. + if (stateIt->second == payload) + { + return; + } + + stateIt->second = payload; + } + + if (type == LOK_CALLBACK_INVALIDATE_TILES && + !m_queue.empty() && std::get<0>(m_queue.back()) == type && std::get<1>(m_queue.back()) == payload) + { + // Supress duplicate invalidation only when they are in sequence. + return; + } + + m_queue.emplace_back(type, payload); + + lock.unlock(); + if (!IsActive()) + { + Start(); + } + } + + private: + void flush() + { + if (m_pCallback) + { + std::unique_lock lock(m_mutex); + for (auto& pair : m_queue) + { + m_pCallback(std::get<0>(pair), std::get<1>(pair).c_str(), m_pData); + } + + m_queue.clear(); + } + } + + private: + std::vector> m_queue; + std::map m_states; + LibreOfficeKitCallback m_pCallback; + void *m_pData; + std::mutex m_mutex; + }; + struct DESKTOP_DLLPUBLIC LibLODocument_Impl : public _LibreOfficeKitDocument { css::uno::Reference mxComponent; std::shared_ptr< LibreOfficeKitDocumentClass > m_pDocumentClass; - LibreOfficeKitCallback mpCallback; - void *mpCallbackData; + std::shared_ptr mpCallbackFlushHandler; explicit LibLODocument_Impl(const css::uno::Reference &xComponent); ~LibLODocument_Impl(); diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx b/desktop/qa/desktop_lib/test_desktop_lib.cxx index 93bde3decb7f..5ad6c0df8275 100644 --- a/desktop/qa/desktop_lib/test_desktop_lib.cxx +++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx @@ -353,6 +353,7 @@ void DesktopLOKTest::testSearchCalc() {"SearchItem.Command", uno::makeAny(static_cast(SvxSearchCmd::FIND_ALL))}, })); comphelper::dispatchCommand(".uno:ExecuteSearch", aPropertyValues); + Scheduler::ProcessEventsToIdle(); std::vector aSelections; sal_Int32 nIndex = 0; @@ -635,6 +636,7 @@ void DesktopLOKTest::testCommandResult() // the condition var. m_aCommandResultCondition.reset(); pDocument->pClass->postUnoCommand(pDocument, ".uno:Bold", nullptr, true); + Scheduler::ProcessEventsToIdle(); m_aCommandResultCondition.wait(aTimeValue); CPPUNIT_ASSERT(m_aCommandResult.isEmpty()); @@ -644,6 +646,7 @@ void DesktopLOKTest::testCommandResult() m_aCommandResultCondition.reset(); pDocument->pClass->postUnoCommand(pDocument, ".uno:Bold", nullptr, true); + Scheduler::ProcessEventsToIdle(); m_aCommandResultCondition.wait(aTimeValue); boost::property_tree::ptree aTree; @@ -666,6 +669,7 @@ void DesktopLOKTest::testWriterComments() TimeValue aTimeValue = {2 , 0}; // 2 seconds max m_aCommandResultCondition.reset(); pDocument->pClass->postUnoCommand(pDocument, ".uno:InsertAnnotation", nullptr, true); + Scheduler::ProcessEventsToIdle(); m_aCommandResultCondition.wait(aTimeValue); CPPUNIT_ASSERT(!m_aCommandResult.isEmpty()); xToolkit->reschedule(); @@ -706,6 +710,7 @@ void DesktopLOKTest::testModifiedStatus() 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(); @@ -719,6 +724,7 @@ void DesktopLOKTest::testModifiedStatus() 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(); @@ -728,6 +734,7 @@ void DesktopLOKTest::testModifiedStatus() // Modify the document again m_aStateChangedCondition.reset(); pDocument->pClass->postKeyEvent(pDocument, LOK_KEYEVENT_KEYINPUT, 't', 0); + Scheduler::ProcessEventsToIdle(); m_aStateChangedCondition.wait(aTimeValue); Scheduler::ProcessEventsToIdle(); @@ -976,6 +983,7 @@ void DesktopLOKTest::testContextMenuCalc() LOK_MOUSEEVENT_MOUSEBUTTONDOWN, aPointOnImage.X(), aPointOnImage.Y(), 1, 4, 0); + Scheduler::ProcessEventsToIdle(); TimeValue aTimeValue = {2 , 0}; // 2 seconds max m_aContextMenuCondition.wait(aTimeValue); @@ -1085,6 +1093,7 @@ void DesktopLOKTest::testContextMenuWriter() LOK_MOUSEEVENT_MOUSEBUTTONDOWN, aRandomPoint.X(), aRandomPoint.Y(), 1, 4, 0); + Scheduler::ProcessEventsToIdle(); TimeValue aTimeValue = {2 , 0}; // 2 seconds max m_aContextMenuCondition.wait(aTimeValue); @@ -1140,6 +1149,7 @@ void DesktopLOKTest::testContextMenuImpress() LOK_MOUSEEVENT_MOUSEBUTTONDOWN, aRandomPoint.X(), aRandomPoint.Y(), 1, 4, 0); + Scheduler::ProcessEventsToIdle(); TimeValue aTimeValue = {2 , 0}; // 2 seconds max m_aContextMenuCondition.wait(aTimeValue); diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx index 20c48fd5a74b..554a4f4abaf2 100644 --- a/desktop/source/lib/init.cxx +++ b/desktop/source/lib/init.cxx @@ -371,8 +371,6 @@ static char* doc_getPartHash(LibreOfficeKitDocument* pThis, int nPart); LibLODocument_Impl::LibLODocument_Impl(const uno::Reference &xComponent) : mxComponent(xComponent) - , mpCallback(nullptr) - , mpCallbackData(nullptr) { if (!(m_pDocumentClass = gDocumentClass.lock())) { @@ -960,11 +958,11 @@ static void doc_setPartMode(LibreOfficeKitDocument* pThis, } } -void doc_paintTile (LibreOfficeKitDocument* pThis, - unsigned char* pBuffer, - const int nCanvasWidth, const int nCanvasHeight, - const int nTilePosX, const int nTilePosY, - const int nTileWidth, const int nTileHeight) +void doc_paintTile(LibreOfficeKitDocument* pThis, + unsigned char* pBuffer, + const int nCanvasWidth, const int nCanvasHeight, + const int nTilePosX, const int nTilePosY, + const int nTileWidth, const int nTileHeight) { SAL_INFO( "lok.tiledrendering", "paintTile: painting [" << nTileWidth << "x" << nTileHeight << "]@(" << nTilePosX << ", " << nTilePosY << ") to [" << @@ -1070,13 +1068,12 @@ static void doc_registerCallback(LibreOfficeKitDocument* pThis, { LibLODocument_Impl* pDocument = static_cast(pThis); - pDocument->mpCallback = pCallback; - pDocument->mpCallbackData = pData; + pDocument->mpCallbackFlushHandler.reset(new CallbackFlushHandler(pCallback, pData)); if (comphelper::LibreOfficeKit::isViewCallback()) { if (SfxViewShell* pViewShell = SfxViewFrame::Current()->GetViewShell()) - pViewShell->registerLibreOfficeKitViewCallback(pCallback, pData); + pViewShell->registerLibreOfficeKitViewCallback(CallbackFlushHandler::callback, pDocument->mpCallbackFlushHandler.get()); } else { @@ -1087,7 +1084,7 @@ static void doc_registerCallback(LibreOfficeKitDocument* pThis, return; } - pDoc->registerCallback(pCallback, pData); + pDoc->registerCallback(CallbackFlushHandler::callback, pDocument->mpCallbackFlushHandler.get()); } } @@ -1113,14 +1110,12 @@ static void doc_postKeyEvent(LibreOfficeKitDocument* pThis, int nType, int nChar class DispatchResultListener : public cppu::WeakImplHelper { OString maCommand; ///< Command for which this is the result. - LibreOfficeKitCallback mpCallback; ///< Callback to call. - void* mpCallbackData; ///< The callback's data. + std::shared_ptr mpCallback; ///< Callback to call. public: - DispatchResultListener(const char* pCommand, LibreOfficeKitCallback pCallback, void* pCallbackData) + DispatchResultListener(const char* pCommand, std::shared_ptr& pCallback) : maCommand(pCommand) , mpCallback(pCallback) - , mpCallbackData(pCallbackData) { assert(mpCallback); } @@ -1141,7 +1136,7 @@ public: std::stringstream aStream; boost::property_tree::write_json(aStream, aTree); - mpCallback(LOK_CALLBACK_UNO_COMMAND_RESULT, strdup(aStream.str().c_str()), mpCallbackData); + mpCallback->queue(LOK_CALLBACK_UNO_COMMAND_RESULT, strdup(aStream.str().c_str())); } virtual void SAL_CALL disposing(const css::lang::EventObject&) throw (css::uno::RuntimeException, std::exception) override {} @@ -1170,10 +1165,10 @@ static void doc_postUnoCommand(LibreOfficeKitDocument* pThis, const char* pComma bool bResult = false; - if (bNotifyWhenFinished && pDocument->mpCallback) + if (bNotifyWhenFinished && pDocument->mpCallbackFlushHandler) { bResult = comphelper::dispatchCommand(aCommand, comphelper::containerToSequence(aPropertyValuesVector), - new DispatchResultListener(pCommand, pDocument->mpCallback, pDocument->mpCallbackData)); + new DispatchResultListener(pCommand, pDocument->mpCallbackFlushHandler)); } else bResult = comphelper::dispatchCommand(aCommand, comphelper::containerToSequence(aPropertyValuesVector)); @@ -1205,9 +1200,9 @@ static void doc_postMouseEvent(LibreOfficeKitDocument* pThis, int nType, int nX, } LibLODocument_Impl* pLib = static_cast(pThis); - if (pLib->mpCallback && pLib->mpCallbackData) + if (pLib->mpCallbackFlushHandler) { - pLib->mpCallback(LOK_CALLBACK_MOUSE_POINTER, aPointerString.getStr(), pLib->mpCallbackData); + pLib->mpCallbackFlushHandler->queue(LOK_CALLBACK_MOUSE_POINTER, aPointerString.getStr()); } } diff --git a/desktop/source/lib/lokinteractionhandler.cxx b/desktop/source/lib/lokinteractionhandler.cxx index 8ebff623b254..bb1b495a292e 100644 --- a/desktop/source/lib/lokinteractionhandler.cxx +++ b/desktop/source/lib/lokinteractionhandler.cxx @@ -114,8 +114,8 @@ void LOKInteractionHandler::postError(css::task::InteractionClassification class std::stringstream aStream; boost::property_tree::write_json(aStream, aTree); - if (m_pLOKDocument && m_pLOKDocument->mpCallback) - m_pLOKDocument->mpCallback(LOK_CALLBACK_ERROR, aStream.str().c_str(), m_pLOKDocument->mpCallbackData); + if (m_pLOKDocument && m_pLOKDocument->mpCallbackFlushHandler) + m_pLOKDocument->mpCallbackFlushHandler->queue(LOK_CALLBACK_ERROR, aStream.str().c_str()); else if (m_pLOKit->mpCallback) m_pLOKit->mpCallback(LOK_CALLBACK_ERROR, aStream.str().c_str(), m_pLOKit->mpCallbackData); } -- cgit