[Libreoffice-commits] core.git: 2 commits - sw/inc sw/source vcl/win

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Mon Aug 20 07:36:12 UTC 2018


 sw/inc/IDocumentTimerAccess.hxx             |   40 ++++++++++++++-------------
 sw/source/core/doc/DocumentTimerManager.cxx |   24 ++++++++--------
 sw/source/core/inc/DocumentTimerManager.hxx |    2 -
 sw/source/core/inc/docfld.hxx               |    2 -
 sw/source/core/inc/rootfrm.hxx              |    4 +-
 vcl/win/app/salinst.cxx                     |   41 +++++++++++++++++++++-------
 vcl/win/window/salframe.cxx                 |    9 +-----
 7 files changed, 71 insertions(+), 51 deletions(-)

New commits:
commit 401cba4c20fbc930f034168872642428d7459218
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Aug 17 23:10:00 2018 +0200
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Mon Aug 20 09:35:59 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>

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 ab0b6a1f9492..33b9a1961108 100644
--- a/sw/source/core/inc/docfld.hxx
+++ b/sw/source/core/inc/docfld.hxx
@@ -170,7 +170,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 238cb9519af4..7824d4ff5eca 100644
--- a/sw/source/core/inc/rootfrm.hxx
+++ b/sw/source/core/inc/rootfrm.hxx
@@ -245,7 +245,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; }
@@ -261,7 +261,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();
         }
     }
 
commit 35a254750392dcd738481f5d6e8719cee9fb41b3
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Aug 17 19:41:53 2018 +0200
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Mon Aug 20 09:35:51 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.
    
    Change-Id: I7024b081b26f3545af12a3a3a038fe5e5671af3c
    Reviewed-on: https://gerrit.libreoffice.org/59275
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>

diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 778d0a4b2e7e..154a125f2069 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -469,6 +469,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 )
@@ -563,27 +568,45 @@ 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 << ")");
+    SAL_INFO("vcl.gdi.wndproc", "SalComWndProc(nMsg=" << nMsg << ", wParam=" << wParam
+                                << ", lParam=" << lParam << "); inSendMsg: " << bIsOtherThreadMessage);
 
     switch ( nMsg )
     {
diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index 7d37bdcc652c..334b518cb340 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -917,13 +917,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);


More information about the Libreoffice-commits mailing list