[Libreoffice-commits] core.git: Branch 'libreoffice-6-0' - 5 commits - include/vcl sc/inc sc/source sw/inc sw/source vcl/inc vcl/source vcl/win

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Wed Oct 3 13:50:21 UTC 2018


 include/vcl/scheduler.hxx                   |   12 +++++
 sc/inc/dbdata.hxx                           |    3 -
 sc/source/core/tool/dbdata.cxx              |    6 +-
 sw/inc/IDocumentTimerAccess.hxx             |   43 +++++++++++----------
 sw/source/core/doc/DocumentTimerManager.cxx |   57 ++++++++++++++++------------
 sw/source/core/inc/DocumentTimerManager.hxx |   10 +---
 sw/source/core/inc/docfld.hxx               |    2 
 sw/source/core/inc/rootfrm.hxx              |    4 -
 vcl/inc/win/salinst.h                       |    2 
 vcl/source/app/scheduler.cxx                |    5 ++
 vcl/win/app/salinst.cxx                     |   49 +++++++++++++++++-------
 vcl/win/window/salframe.cxx                 |    9 ----
 12 files changed, 125 insertions(+), 77 deletions(-)

New commits:
commit 8755c98657b0ab5a08742f38d7856ca63de3ffeb
Author:     Eike Rathke <erack at redhat.com>
AuthorDate: Thu Sep 20 18:12:03 2018 +0200
Commit:     Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
CommitDate: Wed Oct 3 15:49:29 2018 +0200

    Resolves: tdf#119954 do not copy the old parent, init with new parent instead
    
    The parent is always the ScDBCollection that contains the
    ScDBCollection::NamedDBs container, not the one the
    ScDBCollection::NamedDBs was copy-constructed from.
    
    Change-Id: Ia409347f3aeb9ad7a5e68da7af727adfac98d6a2
    Reviewed-on: https://gerrit.libreoffice.org/60833
    Reviewed-by: Eike Rathke <erack at redhat.com>
    Tested-by: Jenkins
    (cherry picked from commit 7ea5d339dc4d8412f436f3affa589bfbd0b1ef68)
    Reviewed-on: https://gerrit.libreoffice.org/60842
    Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>

diff --git a/sc/inc/dbdata.hxx b/sc/inc/dbdata.hxx
index 7752693765a1..a72a3130e29b 100644
--- a/sc/inc/dbdata.hxx
+++ b/sc/inc/dbdata.hxx
@@ -241,7 +241,8 @@ public:
         DBsType m_DBs;
         ScDBCollection& mrParent;
         NamedDBs(ScDBCollection& rParent, ScDocument& rDoc);
-        NamedDBs(const NamedDBs& r);
+        NamedDBs(const NamedDBs& r, ScDBCollection& rParent);
+        NamedDBs(const NamedDBs&) = delete;
         virtual ~NamedDBs() override;
         NamedDBs & operator=(NamedDBs const&) = delete;
         void initInserted( ScDBData* p );
diff --git a/sc/source/core/tool/dbdata.cxx b/sc/source/core/tool/dbdata.cxx
index 0b7ebd3b6bfe..689e15551796 100644
--- a/sc/source/core/tool/dbdata.cxx
+++ b/sc/source/core/tool/dbdata.cxx
@@ -1070,9 +1070,9 @@ ScRangeList& ScDBDataContainerBase::GetDirtyTableColumnNames()
 ScDBCollection::NamedDBs::NamedDBs(ScDBCollection& rParent, ScDocument& rDoc) :
     ScDBDataContainerBase(rDoc), mrParent(rParent) {}
 
-ScDBCollection::NamedDBs::NamedDBs(const NamedDBs& r)
+ScDBCollection::NamedDBs::NamedDBs(const NamedDBs& r, ScDBCollection& rParent)
     : ScDBDataContainerBase(r.mrDoc)
-    , mrParent(r.mrParent)
+    , mrParent(rParent)
 {
     for (auto const& it : r.m_DBs)
     {
@@ -1286,7 +1286,7 @@ ScDBCollection::ScDBCollection(ScDocument* pDocument) :
     pDoc(pDocument), nEntryIndex(1), maNamedDBs(*this, *pDocument) {}
 
 ScDBCollection::ScDBCollection(const ScDBCollection& r) :
-    pDoc(r.pDoc), nEntryIndex(r.nEntryIndex), maNamedDBs(r.maNamedDBs), maAnonDBs(r.maAnonDBs) {}
+    pDoc(r.pDoc), nEntryIndex(r.nEntryIndex), maNamedDBs(r.maNamedDBs, *this), maAnonDBs(r.maAnonDBs) {}
 
 const ScDBData* ScDBCollection::GetDBAtCursor(SCCOL nCol, SCROW nRow, SCTAB nTab, ScDBDataPortion ePortion) const
 {
commit d546859719a7f0f6bb19a80a811c5e7724b25f3d
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Aug 24 09:14:47 2018 +0200
Commit:     Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
CommitDate: Wed Oct 3 15:48:25 2018 +0200

    tdf#118786 WIN just assert in Yield
    
    This reminds me - again - that Jenkins doesn't run make check.
    
    It turns out InSendMessage() also returns true, if you process a
    nested SendMessage in the same thread. Therefore we have to
    remove the SalComWndProc assert and just keep the one in the
    Yield call.
    
    Why? Because there seem to be no way to get the information
    ReplyMessage has access to, so we could detect the caller /
    origin of the send message and implement proper nested call
    checks. The alternative would be to change all call sites of
    SendMessage to:
    
    if ( !pSalData->mpInstance->IsMainThread() )
        SendMessage(...)
    else
        SalComWndProc(...)
    
    which is the same SendMessage already does.
    
    Change-Id: I991d68a64952dc5d47ba51edd8635c9e8c46614c
    Reviewed-on: https://gerrit.libreoffice.org/59538
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
    (cherry picked from commit bf0a63dc93a746a008fb1656457c77de8df693ba)
    Reviewed-on: https://gerrit.libreoffice.org/59565
    (cherry picked from commit a42c65176f2791cf5e48578a8898bf03185adc89)
    Reviewed-on: https://gerrit.libreoffice.org/60726
    Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>

diff --git a/vcl/inc/win/salinst.h b/vcl/inc/win/salinst.h
index 9f5dbdf955b1..89a12c895bf9 100644
--- a/vcl/inc/win/salinst.h
+++ b/vcl/inc/win/salinst.h
@@ -39,7 +39,7 @@ public:
     SalYieldMutex*      mpSalYieldMutex;
 
     osl::Condition      maWaitingYieldCond;
-    bool                mbNoYieldLock;
+    unsigned            m_nNoYieldLock;
 
 public:
     WinSalInstance();
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index f6e17bcf7faa..288c5c2cc6e0 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -138,7 +138,7 @@ void SalYieldMutex::doAcquire( sal_uInt32 nLockCount )
     WinSalInstance* pInst = GetSalData()->mpInstance;
     if ( pInst && pInst->IsMainThread() )
     {
-        if ( pInst->mbNoYieldLock )
+        if ( pInst->m_nNoYieldLock )
             return;
         // tdf#96887 If this is the main thread, then we must wait for two things:
         // - the mpSalYieldMutex being freed
@@ -169,7 +169,7 @@ void SalYieldMutex::doAcquire( sal_uInt32 nLockCount )
 sal_uInt32 SalYieldMutex::doRelease( const bool bUnlockAll )
 {
     WinSalInstance* pInst = GetSalData()->mpInstance;
-    if ( pInst && pInst->mbNoYieldLock && pInst->IsMainThread() )
+    if ( pInst && pInst->m_nNoYieldLock && pInst->IsMainThread() )
         return 1;
 
     sal_uInt32 nCount = comphelper::GenericSolarMutex::doRelease( bUnlockAll );
@@ -184,7 +184,7 @@ bool SalYieldMutex::tryToAcquire()
     WinSalInstance* pInst = GetSalData()->mpInstance;
     if ( pInst )
     {
-        if ( pInst->mbNoYieldLock && pInst->IsMainThread() )
+        if ( pInst->m_nNoYieldLock && pInst->IsMainThread() )
             return true;
         else
             return comphelper::GenericSolarMutex::tryToAcquire();
@@ -218,7 +218,7 @@ void ImplSalYieldMutexRelease()
 
 bool SalYieldMutex::IsCurrentThread() const
 {
-    if ( !GetSalData()->mpInstance->mbNoYieldLock )
+    if ( !GetSalData()->mpInstance->m_nNoYieldLock )
         // For the Windows backend, the LO identifier is the system thread ID
         return m_nThreadId == GetCurrentThreadId();
     else
@@ -431,7 +431,7 @@ void DestroySalInstance( SalInstance* pInst )
 
 WinSalInstance::WinSalInstance()
     : mhComWnd( nullptr )
-    , mbNoYieldLock( false )
+    , m_nNoYieldLock( 0 )
 {
     mpSalYieldMutex = new SalYieldMutex();
     mpSalYieldMutex->acquire();
@@ -477,7 +477,7 @@ bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
     bool bWasMsg = false, bOneEvent = false, bWasTimeoutMsg = false;
     ImplSVData *const pSVData = ImplGetSVData();
     WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer );
-    const bool bNoYieldLock = GetSalData()->mpInstance->mbNoYieldLock;
+    const bool bNoYieldLock = (GetSalData()->mpInstance->m_nNoYieldLock > 0);
 
     assert( !bNoYieldLock );
     if ( bNoYieldLock )
@@ -578,10 +578,9 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
     case salmsg: \
         if (bIsOtherThreadMessage) \
         { \
-            assert( !pInst->mbNoYieldLock ); \
-            pInst->mbNoYieldLock = true; \
+            ++pInst->m_nNoYieldLock; \
             function; \
-            pInst->mbNoYieldLock = false; \
+            --pInst->m_nNoYieldLock; \
         } \
         else \
         { \
@@ -594,10 +593,9 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
     case salmsg: \
         if (bIsOtherThreadMessage) \
         { \
-            assert( !pInst->mbNoYieldLock ); \
-            pInst->mbNoYieldLock = true; \
+            ++pInst->m_nNoYieldLock; \
             nRet = reinterpret_cast<LRESULT>( function ); \
-            pInst->mbNoYieldLock = false; \
+            --pInst->m_nNoYieldLock; \
         } \
         else \
         { \
commit 316447dd0284d79516cc313636e5857f542d6417
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Aug 17 19:41:53 2018 +0200
Commit:     Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
CommitDate: Wed Oct 3 15:48:15 2018 +0200

    tdf#118786 WIN allow nested SendMessage calls
    
    This bug trips "assert( !pInst->mbNoYieldLock )".
    
    There is already a special case, introduced in commit 4baec725e0dc
    ("WIN run main thread redirects ignoring SolarMutex"), to prevent
    tripping the assert for a nested SendMessage call.
    
    So this implements a general solution for nested SendMessage calls.
    We just have to prevent yielding in a call from an other thread,
    as the sending thread still owns the SolarMutex.
    
    This way we can also drop the special handling in
    WinSalFrame::ReleaseFrameGraphicsDC.
    
    Conflicts:
            vcl/win/app/salinst.cxx
    
    Change-Id: I7024b081b26f3545af12a3a3a038fe5e5671af3c
    Reviewed-on: https://gerrit.libreoffice.org/59275
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    (cherry picked from commit 35a254750392dcd738481f5d6e8719cee9fb41b3)
    Reviewed-on: https://gerrit.libreoffice.org/59318
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>
    (cherry picked from commit 3e392904c558d1f9fc91926511d148762813537e)
    Reviewed-on: https://gerrit.libreoffice.org/60725
    Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>

diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index dcea8d53f6d7..f6e17bcf7faa 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -477,6 +477,11 @@ bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
     bool bWasMsg = false, bOneEvent = false, bWasTimeoutMsg = false;
     ImplSVData *const pSVData = ImplGetSVData();
     WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer );
+    const bool bNoYieldLock = GetSalData()->mpInstance->mbNoYieldLock;
+
+    assert( !bNoYieldLock );
+    if ( bNoYieldLock )
+        return false;
 
     sal_uInt32 nCurTicks = 0;
     if ( bHandleAllCurrentEvents )
@@ -571,26 +576,46 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
 
 #define CASE_NOYIELDLOCK( salmsg, function ) \
     case salmsg: \
-        assert( !pInst->mbNoYieldLock ); \
-        pInst->mbNoYieldLock = true; \
-        function; \
-        pInst->mbNoYieldLock = false; \
+        if (bIsOtherThreadMessage) \
+        { \
+            assert( !pInst->mbNoYieldLock ); \
+            pInst->mbNoYieldLock = true; \
+            function; \
+            pInst->mbNoYieldLock = false; \
+        } \
+        else \
+        { \
+            DBG_TESTSOLARMUTEX(); \
+            function; \
+        } \
         break;
 
 #define CASE_NOYIELDLOCK_RESULT( salmsg, function ) \
     case salmsg: \
-        assert( !pInst->mbNoYieldLock ); \
-        pInst->mbNoYieldLock = true; \
-        nRet = reinterpret_cast<LRESULT>( function ); \
-        pInst->mbNoYieldLock = false; \
+        if (bIsOtherThreadMessage) \
+        { \
+            assert( !pInst->mbNoYieldLock ); \
+            pInst->mbNoYieldLock = true; \
+            nRet = reinterpret_cast<LRESULT>( function ); \
+            pInst->mbNoYieldLock = false; \
+        } \
+        else \
+        { \
+            DBG_TESTSOLARMUTEX(); \
+            nRet = reinterpret_cast<LRESULT>( function ); \
+        } \
         break;
 
 LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, bool& rDef )
 {
+    const BOOL bIsOtherThreadMessage = InSendMessage();
     LRESULT nRet = 0;
     WinSalInstance *pInst = GetSalData()->mpInstance;
     WinSalTimer *const pTimer = static_cast<WinSalTimer*>( ImplGetSVData()->maSchedCtx.mpSalTimer );
 
+    SAL_INFO("vcl.gdi.wndproc", "SalComWndProc(nMsg=" << nMsg << ", wParam=" << wParam
+                                << ", lParam=" << lParam << "); inSendMsg: " << bIsOtherThreadMessage);
+
     switch ( nMsg )
     {
         case SAL_MSG_THREADYIELD:
diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index f34c9d0936cc..27081ed647bd 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -921,13 +921,8 @@ bool WinSalFrame::ReleaseFrameGraphicsDC( WinSalGraphics* pGraphics )
     if ( pGraphics->getDefPal() )
         SelectPalette( hDC, pGraphics->getDefPal(), TRUE );
     pGraphics->DeInitGraphics();
-    // we don't want to run the WinProc in the main thread directly
-    // so we don't hit the mbNoYieldLock assert
-    if ( !pSalData->mpInstance->IsMainThread() )
-        SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC,
-            reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) );
-    else
-        ReleaseDC( mhWnd, hDC );
+    SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC,
+        reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) );
     if ( pGraphics == mpThreadGraphics )
         pSalData->mnCacheDCInUse--;
     pGraphics->setHDC(nullptr);
commit 36556f2f2c30c10f34d5495a7f7cc735b9b9417a
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Aug 24 08:05:26 2018 +0200
Commit:     Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
CommitDate: Wed Oct 3 15:48:06 2018 +0200

    tdf#119458 reorganize sw background Idle handling
    
    This is the squashed version of the following commits:
    - ce68889678bc tdf#119458 always start the SwDocIdle
    - 107f0e6dba57 tdf#119458 fix sw background Idle state handling
    - 771544544ee1 tdf#119458 fix sw background Idle unblocking
    - 3bd8316718fd tdf#119458 just wakeup Scheduler on active Idle
    
    While the first commit was really wrong, the third is just an
    optimization to prevent unnecessary wakeups and Idle task
    interrups from new calls to StartIdling(). The 4th one contains
    the reset of the Idle state on run, which was probably the main
    origin of the problem and allows to wake up the Scheduler
    without moving the Task to the end of the queue.
    
    Starting the SwDocIdle in StartIdeling() just reintroduces the
    bug fixed in commit 401cba4c20fb ("tdf#116370 cleanup Writer idle
    job handing"), so trading one bug for the other.
    
    For the real solution we have to handle two states:
    1. The SwDocIdle being active
    2. The SwDocIdle being blocked
    
    For the first state we can just use the active state of the Idle
    itself. Since it's not a AutoIdle, it will be turned of, if
    invoked. Either some Idle handler will want to be run it again or
    some other thread might start it again, if needed.
    Since we're now tracking the wanted Idle state via the active task
    flag, we can drop the explicit mbStartIdleTimer handling.
    
    As a result of the first change, we can't stop the Idle anymore
    when blocking it. But the Idle itself checks its ready state via
    IsDocIdle() in its UpdateMinPeriod function, so we have to add
    the blocking state to IsDocIdle().
    
    But we can't handle the correct state when blocking via a tasks
    mbActive bool, as this also schedules the task and starts the
    scheduler timer. So reintroduce a bool for the unblock state.
    
    This should prevent fdo#73165, which I couldn't reproduce, but
    just to be sure.
    
    More importantly this patch resets the m_bStartOnUnblock when
    the Idle job actually runs. This run should already determinates
    if more Idle work needs to be done, and others can still call
    BeginIdling() to ensure further processing.
    
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    Tested-by: Jenkins
    Reviewed-on: https://gerrit.libreoffice.org/59536
    (cherry picked from commit ce68889678bca3b51c258452f7bfdd21982e4a0d)
    Reviewed-on: https://gerrit.libreoffice.org/59586
    (cherry picked from commit 107f0e6dba57a5b27366a35a5cdb184079546df0)
    Reviewed-on: https://gerrit.libreoffice.org/59692
    (cherry picked from commit 771544544ee13ec98961f93b5313a7d2e29429cd)
    Reviewed-on: https://gerrit.libreoffice.org/59730
    (cherry picked from commit 3bd8316718fdfed454c01a9c4ae6af6beb34437d)
    
    Change-Id: I2d4328f31644017b09786ff4cba45cf7643f0a4b
    (cherry picked from commit f1fb30cf43eda45137ab96177238a29c3725789d)
    Reviewed-on: https://gerrit.libreoffice.org/59757
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>
    (cherry picked from commit 20e46a6ca5b62f7177f799eae9e0f26e659fd03b)
    Reviewed-on: https://gerrit.libreoffice.org/59769
    Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
    Reviewed-by: Tomaž Vajngerl <quikee at gmail.com>
    Reviewed-by: Xisco Faulí <xiscofauli at libreoffice.org>
    Tested-by: Xisco Faulí <xiscofauli at libreoffice.org>
    (cherry picked from commit 3abec366a3fdb8657d69e24b69b4338121f80b22)
    Reviewed-on: https://gerrit.libreoffice.org/60724

diff --git a/include/vcl/scheduler.hxx b/include/vcl/scheduler.hxx
index ac429ed33b09..6233d134a8ed 100644
--- a/include/vcl/scheduler.hxx
+++ b/include/vcl/scheduler.hxx
@@ -65,6 +65,18 @@ public:
      */
     static void       ProcessEventsToIdle();
 
+    /**
+     * Wakes up the scheduler
+     *
+     * This doesn't handle any events! It just ensures the Scheduler is run as
+     * soon as possible by forcing the Scheduler timer to fire.
+     *
+     * Can be used for complex UpdateMinPeriod function, where the task is
+     * actually active but not ready and we want to skip the Task::Start()
+     * queue append for faster reaction.
+     */
+    static void       Wakeup();
+
     /// Control the deterministic mode.  In this mode, two subsequent runs of
     /// LibreOffice fire about the same amount idles.
     static void       SetDeterministicMode(bool bDeterministic);
diff --git a/sw/inc/IDocumentTimerAccess.hxx b/sw/inc/IDocumentTimerAccess.hxx
index 1ed8679c00ac..f3e2738485af 100644
--- a/sw/inc/IDocumentTimerAccess.hxx
+++ b/sw/inc/IDocumentTimerAccess.hxx
@@ -21,7 +21,7 @@
 #define INCLUDED_SW_INC_IDOCUMENTTIMERACCESS_HXX
 
 /**
- * Handle the background job of the Writer document.
+ * Handle the background jobs of a Writer document.
  *
  * Initially it's disabled and unblocked.
  *
@@ -34,7 +34,9 @@ class IDocumentTimerAccess
 {
 public:
     /**
-     * Start the idle job depending on the block count.
+     * Start the idle task.
+     *
+     * Depends on the block count and various document states.
      */
     virtual void StartIdling() = 0;
 
@@ -47,13 +49,14 @@ public:
      * Increment block count.
      *
      * Prevents further background idle processing.
+     * This doesn't guarantee the Idle task is not currently running!
      */
     virtual void BlockIdling() = 0;
 
     /**
      * Decrement block count.
      *
-     * May start the idle job.
+     * May re-start the idle task, if active.
      */
     virtual void UnblockIdling() = 0;
 
diff --git a/sw/source/core/doc/DocumentTimerManager.cxx b/sw/source/core/doc/DocumentTimerManager.cxx
index 35f2eb94dcbd..7bd2e505a3a4 100644
--- a/sw/source/core/doc/DocumentTimerManager.cxx
+++ b/sw/source/core/doc/DocumentTimerManager.cxx
@@ -33,50 +33,56 @@
 #include <docsh.hxx>
 #include <docfld.hxx>
 #include <fldbas.hxx>
+#include <vcl/scheduler.hxx>
 
 namespace sw
 {
 
 DocumentTimerManager::DocumentTimerManager( SwDoc& i_rSwdoc ) : m_rDoc( i_rSwdoc ),
-                                                                mbStartIdleTimer( false ),
-                                                                mIdleBlockCount( 0 ),
-                                                                maDocIdle( i_rSwdoc )
+                                                                m_nIdleBlockCount( 0 ),
+                                                                m_bStartOnUnblock( false ),
+                                                                m_aDocIdle( i_rSwdoc )
 {
-    maDocIdle.SetPriority( TaskPriority::LOWEST );
-    maDocIdle.SetInvokeHandler( LINK( this, DocumentTimerManager, DoIdleJobs) );
-    maDocIdle.SetDebugName( "sw::DocumentTimerManager maDocIdle" );
+    m_aDocIdle.SetPriority(TaskPriority::LOWEST);
+    m_aDocIdle.SetInvokeHandler(LINK( this, DocumentTimerManager, DoIdleJobs));
+    m_aDocIdle.SetDebugName("sw::DocumentTimerManager m_aDocIdle");
 }
 
 void DocumentTimerManager::StartIdling()
 {
-    if( !mIdleBlockCount && !maDocIdle.IsActive() )
+    m_bStartOnUnblock = true;
+    if (0 == m_nIdleBlockCount)
     {
-        mbStartIdleTimer = false;
-        maDocIdle.Start();
+        if (!m_aDocIdle.IsActive())
+            m_aDocIdle.Start();
+        else
+            Scheduler::Wakeup();
     }
-    else
-        mbStartIdleTimer = true;
 }
 
 void DocumentTimerManager::StopIdling()
 {
-    mbStartIdleTimer = false;
-    maDocIdle.Stop();
+    m_bStartOnUnblock = false;
+    m_aDocIdle.Stop();
 }
 
 void DocumentTimerManager::BlockIdling()
 {
-    maDocIdle.Stop();
-    ++mIdleBlockCount;
+    assert(SAL_MAX_UINT32 != m_nIdleBlockCount);
+    ++m_nIdleBlockCount;
 }
 
 void DocumentTimerManager::UnblockIdling()
 {
-    --mIdleBlockCount;
-    if( !mIdleBlockCount && mbStartIdleTimer && !maDocIdle.IsActive() )
+    assert(0 != m_nIdleBlockCount);
+    --m_nIdleBlockCount;
+
+    if ((0 == m_nIdleBlockCount) && m_bStartOnUnblock)
     {
-        mbStartIdleTimer = false;
-        maDocIdle.Start();
+        if (!m_aDocIdle.IsActive())
+            m_aDocIdle.Start();
+        else
+            Scheduler::Wakeup();
     }
 }
 
@@ -131,6 +137,7 @@ IMPL_LINK_NOARG( DocumentTimerManager, DoIdleJobs, Timer*, void )
         pModLogFile = new ::rtl::Logfile( "First DoIdleJobs" );
 #endif
     BlockIdling();
+    StopIdling();
 
     IdleJob eJob = GetNextIdleJob();
 
diff --git a/sw/source/core/inc/DocumentTimerManager.hxx b/sw/source/core/inc/DocumentTimerManager.hxx
index 214c0f626a75..2caaf608c40d 100644
--- a/sw/source/core/inc/DocumentTimerManager.hxx
+++ b/sw/source/core/inc/DocumentTimerManager.hxx
@@ -66,14 +66,14 @@ private:
 
     SwDoc& m_rDoc;
 
-    bool mbStartIdleTimer; //< idle timer mode start/stop
-    sal_Int32 mIdleBlockCount;
-    SwDocIdle maDocIdle;
+    sal_uInt32 m_nIdleBlockCount; ///< Don't run the Idle, if > 0
+    bool m_bStartOnUnblock; ///< true, if the last unblock should start the timer
+    SwDocIdle m_aDocIdle;
 };
 
 inline bool DocumentTimerManager::IsDocIdle() const
 {
-    return( GetNextIdleJob() != IdleJob::Busy );
+    return ((0 == m_nIdleBlockCount) && (GetNextIdleJob() != IdleJob::Busy));
 }
 
 }
diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx
index b53a58ecf555..91404233fe55 100644
--- a/vcl/source/app/scheduler.cxx
+++ b/vcl/source/app/scheduler.cxx
@@ -502,6 +502,11 @@ next_entry:
     return !!pMostUrgent;
 }
 
+void Scheduler::Wakeup()
+{
+    Scheduler::ImplStartTimer( 0, false, tools::Time::GetSystemTicks() );
+}
+
 void Task::StartTimer( sal_uInt64 nMS )
 {
     Scheduler::ImplStartTimer( nMS, false, tools::Time::GetSystemTicks() );
commit cc360c414d9c9e16ca039300c57fd941d68d66ad
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Aug 17 23:10:00 2018 +0200
Commit:     Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
CommitDate: Wed Oct 3 15:47:55 2018 +0200

    tdf#116370 cleanup Writer idle job handing
    
    This prevents the start of the idle job, while processing itself,
    so the fixed WinSalInstance::AnyInput of commit 3bf6c97029d2
    ("tdf#112975 WIN correctly handle VclInputFlags::OTHER") won't
    report the timer events of the re-started idle job to process.
    
    Fixes the early abort of the background job, which resulted in
    the busy loop of the reported bug and this strange printing
    behaviour.
    
    P.S. I'm not sure, why this was just broken on Windows.
    
    Change-Id: I6503dcd925c9a0ed843e794a31eea32a4a4b2889
    Reviewed-on: https://gerrit.libreoffice.org/59279
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    (cherry picked from commit 401cba4c20fbc930f034168872642428d7459218)
    Reviewed-on: https://gerrit.libreoffice.org/60723
    Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>

diff --git a/sw/inc/IDocumentTimerAccess.hxx b/sw/inc/IDocumentTimerAccess.hxx
index 6efe1a114963..1ed8679c00ac 100644
--- a/sw/inc/IDocumentTimerAccess.hxx
+++ b/sw/inc/IDocumentTimerAccess.hxx
@@ -20,42 +20,44 @@
 #ifndef INCLUDED_SW_INC_IDOCUMENTTIMERACCESS_HXX
 #define INCLUDED_SW_INC_IDOCUMENTTIMERACCESS_HXX
 
-/** Manipulate background jobs of the document. It starts with a mode of
- 'started' and a block count of 0.
+/**
+ * Handle the background job of the Writer document.
+ *
+ * Initially it's disabled and unblocked.
+ *
+ * Jobs include:
+ *  * grammar checking
+ *  * field updating
+ *  * document layouting
  */
 class IDocumentTimerAccess
 {
 public:
     /**
-    Set mode to 'start'.
-    */
+     * Start the idle job depending on the block count.
+     */
     virtual void StartIdling() = 0;
 
     /**
-    Set mode to 'stopped'.
-    */
+     * Stop idle processing.
+     */
     virtual void StopIdling() = 0;
 
     /**
-    Increment block count.
-    */
+     * Increment block count.
+     *
+     * Prevents further background idle processing.
+     */
     virtual void BlockIdling() = 0;
 
     /**
-    Decrement block count.
-    */
+     * Decrement block count.
+     *
+     * May start the idle job.
+     */
     virtual void UnblockIdling() = 0;
 
     /**
-    Do these jobs asynchronously: do grammar checking,
-    do layout, and update fields.
-    They will be delayed until mode is start AND block count == 0.
-    The implementation might delay them further, for example
-    it might wait until the application is idle.
-    */
-    virtual void StartBackgroundJobs() = 0;
-
-    /**
      * Is the document ready to be processed?
      */
     virtual bool IsDocIdle() const = 0;
diff --git a/sw/source/core/doc/DocumentTimerManager.cxx b/sw/source/core/doc/DocumentTimerManager.cxx
index 5429c6edbda6..35f2eb94dcbd 100644
--- a/sw/source/core/doc/DocumentTimerManager.cxx
+++ b/sw/source/core/doc/DocumentTimerManager.cxx
@@ -49,9 +49,13 @@ DocumentTimerManager::DocumentTimerManager( SwDoc& i_rSwdoc ) : m_rDoc( i_rSwdoc
 
 void DocumentTimerManager::StartIdling()
 {
-    mbStartIdleTimer = true;
-    if( !mIdleBlockCount )
+    if( !mIdleBlockCount && !maDocIdle.IsActive() )
+    {
+        mbStartIdleTimer = false;
         maDocIdle.Start();
+    }
+    else
+        mbStartIdleTimer = true;
 }
 
 void DocumentTimerManager::StopIdling()
@@ -70,14 +74,10 @@ void DocumentTimerManager::UnblockIdling()
 {
     --mIdleBlockCount;
     if( !mIdleBlockCount && mbStartIdleTimer && !maDocIdle.IsActive() )
+    {
+        mbStartIdleTimer = false;
         maDocIdle.Start();
-}
-
-void DocumentTimerManager::StartBackgroundJobs()
-{
-    // Trigger DoIdleJobs(), asynchronously.
-    if (!maDocIdle.IsActive()) //fdo#73165 if the timer is already running don't restart from 0
-        maDocIdle.Start();
+    }
 }
 
 DocumentTimerManager::IdleJob DocumentTimerManager::GetNextIdleJob() const
@@ -123,13 +123,14 @@ DocumentTimerManager::IdleJob DocumentTimerManager::GetNextIdleJob() const
     return IdleJob::None;
 }
 
-IMPL_LINK( DocumentTimerManager, DoIdleJobs, Timer*, pIdle, void )
+IMPL_LINK_NOARG( DocumentTimerManager, DoIdleJobs, Timer*, void )
 {
 #ifdef TIMELOG
     static ::rtl::Logfile* pModLogFile = 0;
     if( !pModLogFile )
         pModLogFile = new ::rtl::Logfile( "First DoIdleJobs" );
 #endif
+    BlockIdling();
 
     IdleJob eJob = GetNextIdleJob();
 
@@ -183,7 +184,8 @@ IMPL_LINK( DocumentTimerManager, DoIdleJobs, Timer*, pIdle, void )
     }
 
     if ( IdleJob::None != eJob )
-        pIdle->Start();
+        StartIdling();
+    UnblockIdling();
 
 #ifdef TIMELOG
     if( pModLogFile && 1 != (long)pModLogFile )
diff --git a/sw/source/core/inc/DocumentTimerManager.hxx b/sw/source/core/inc/DocumentTimerManager.hxx
index d8c1a76b2a14..214c0f626a75 100644
--- a/sw/source/core/inc/DocumentTimerManager.hxx
+++ b/sw/source/core/inc/DocumentTimerManager.hxx
@@ -54,8 +54,6 @@ public:
 
     void UnblockIdling() override;
 
-    void StartBackgroundJobs() override;
-
     bool IsDocIdle() const override;
 
 private:
diff --git a/sw/source/core/inc/docfld.hxx b/sw/source/core/inc/docfld.hxx
index 9979d557a8ee..fea8face62de 100644
--- a/sw/source/core/inc/docfld.hxx
+++ b/sw/source/core/inc/docfld.hxx
@@ -169,7 +169,7 @@ public:
 
         if (b)
         {
-            pDocument->getIDocumentTimerAccess().StartBackgroundJobs();
+            pDocument->getIDocumentTimerAccess().StartIdling();
         }
     }
 
diff --git a/sw/source/core/inc/rootfrm.hxx b/sw/source/core/inc/rootfrm.hxx
index 8dfc3452dd94..ae32cc4e6278 100644
--- a/sw/source/core/inc/rootfrm.hxx
+++ b/sw/source/core/inc/rootfrm.hxx
@@ -244,7 +244,7 @@ public:
         // May be NULL if called from SfxBaseModel::dispose
         // (this happens in the build test 'rtfexport').
         if (pCurrShell != nullptr)
-            pCurrShell->GetDoc()->getIDocumentTimerAccess().StartBackgroundJobs();
+            pCurrShell->GetDoc()->getIDocumentTimerAccess().StartIdling();
     }
     bool IsIdleFormat()  const { return mbIdleFormat; }
     void ResetIdleFormat()     { mbIdleFormat = false; }
@@ -260,7 +260,7 @@ public:
             // May be NULL if called from SfxBaseModel::dispose
             // (this happens in the build test 'rtfexport').
             if (pCurrShell != nullptr)
-                pCurrShell->GetDoc()->getIDocumentTimerAccess().StartBackgroundJobs();
+                pCurrShell->GetDoc()->getIDocumentTimerAccess().StartIdling();
         }
     }
 


More information about the Libreoffice-commits mailing list