[Libreoffice-commits] core.git: Branch 'distro/lhm/libreoffice-5-2+backports' - 2 commits - vcl/inc vcl/win

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Wed Jan 16 18:33:22 UTC 2019


 vcl/inc/win/saltimer.h   |   24 ++---
 vcl/win/app/salinst.cxx  |  218 +++++++++++++++++++++++++++++++++++------------
 vcl/win/app/saltimer.cxx |   52 ++++++-----
 3 files changed, 204 insertions(+), 90 deletions(-)

New commits:
commit 3185bf228cbeb8f716444930b04afb0924c60ef3
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Thu Oct 12 10:20:17 2017 +0200
Commit:     Thorsten Behrens <Thorsten.Behrens at CIB.de>
CommitDate: Wed Jan 16 19:33:12 2019 +0100

    tdf#112975 WIN correctly handle VclInputFlags::OTHER
    
    On Windows we can just check the message queue for existing
    messages. But VclInputFlags::OTHER is used to check for any
    messages, which can't be explicitly checked.
    
    In the case of checking for VclInputFlags::OTHER while
    excluding an other message type, we have to make multiple
    PeekMessage calls and exclude all non-checked message ids.
    
    Change-Id: I1cedd4b76444769842c74228fc547f0d924f8b60
    Reviewed-on: https://gerrit.libreoffice.org/43337
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    Reviewed-on: https://gerrit.libreoffice.org/66451
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    Tested-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>

diff --git a/vcl/inc/win/saltimer.h b/vcl/inc/win/saltimer.h
index 37976bbfaf8b..68973e1cadc3 100644
--- a/vcl/inc/win/saltimer.h
+++ b/vcl/inc/win/saltimer.h
@@ -47,6 +47,7 @@ public:
     virtual void Stop() override;
 
     inline bool IsDirectTimeout() const;
+    inline bool HasTimerElapsed() const;
 };
 
 inline bool WinSalTimer::IsDirectTimeout() const
@@ -54,6 +55,11 @@ inline bool WinSalTimer::IsDirectTimeout() const
     return m_bDirectTimeout;
 }
 
+inline bool WinSalTimer::HasTimerElapsed() const
+{
+    return m_bDirectTimeout || ExistsValidEvent();
+}
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 054602c46641..7bef41e8ac4c 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -793,10 +793,119 @@ LRESULT CALLBACK SalComWndProcW( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lPa
     return nRet;
 }
 
+struct MsgRange
+{
+    UINT nStart;
+    UINT nEnd;
+};
+
+static std::vector<MsgRange> GetOtherRanges( VclInputFlags nType )
+{
+    assert( nType != VCL_INPUT_ANY );
+
+    // this array must be kept sorted!
+    const UINT nExcludeMsgIds[] =
+    {
+        0,
+
+        WM_MOVE, // 3
+        WM_SIZE, // 5
+        WM_PAINT, // 15
+        WM_KEYDOWN, // 256
+        WM_TIMER, // 275
+
+        WM_MOUSEFIRST, // 512
+        513,
+        514,
+        515,
+        516,
+        517,
+        518,
+        519,
+        520,
+        WM_MOUSELAST, // 521
+
+        SAL_MSG_POSTMOVE, // WM_USER+136
+        SAL_MSG_POSTCALLSIZE, // WM_USER+137
+
+        SAL_MSG_TIMER_CALLBACK, // WM_USER+162
+
+        UINT_MAX
+    };
+    const unsigned MAX_EXCL = SAL_N_ELEMENTS( nExcludeMsgIds );
+
+    bool aExcludeMsgList[ MAX_EXCL ] = { false, };
+    std::vector<MsgRange> aResult;
+
+    // set the excluded values
+    if ( !(nType & VclInputFlags::MOUSE) )
+    {
+        for ( unsigned i = 0; nExcludeMsgIds[ 6 + i ] <= WM_MOUSELAST; ++i )
+            aExcludeMsgList[ 6 + i ] = true;
+    }
+
+    if ( !(nType & VclInputFlags::KEYBOARD) )
+        aExcludeMsgList[ 4 ] = true;
+
+    if ( !(nType & VclInputFlags::PAINT) )
+    {
+        aExcludeMsgList[ 1 ] = true;
+        aExcludeMsgList[ 2 ] = true;
+        aExcludeMsgList[ 3 ] = true;
+        aExcludeMsgList[ 16 ] = true;
+        aExcludeMsgList[ 17 ] = true;
+    }
+
+    if ( !(nType & VclInputFlags::TIMER) )
+    {
+        aExcludeMsgList[ 5 ]  = true;
+        aExcludeMsgList[ 18 ]  = true;
+    }
+
+    // build the message ranges to check
+    MsgRange aRange = { 0, 0 };
+    bool doEnd = true;
+    for ( unsigned i = 1; i < MAX_EXCL; ++i )
+    {
+        if ( aExcludeMsgList[ i ] )
+        {
+            if ( !doEnd )
+            {
+                if ( nExcludeMsgIds[ i ] == aRange.nStart )
+                    ++aRange.nStart;
+                else
+                    doEnd = true;
+            }
+            if ( doEnd )
+            {
+                aRange.nEnd = nExcludeMsgIds[ i ] - 1;
+                aResult.push_back( aRange );
+                doEnd = false;
+                aRange.nStart = aRange.nEnd + 2;
+            }
+        }
+    }
+
+    if ( aRange.nStart != UINT_MAX )
+    {
+        aRange.nEnd = UINT_MAX;
+        aResult.push_back( aRange );
+    }
+
+    return aResult;
+}
+
 bool WinSalInstance::AnyInput( VclInputFlags nType )
 {
     MSG aMsg;
 
+    if ( nType & VclInputFlags::TIMER )
+    {
+        const WinSalTimer* pTimer = static_cast<WinSalTimer*>( ImplGetSVData()->maSchedCtx.mpSalTimer );
+        if ( pTimer && pTimer->HasTimerElapsed() )
+            return true;
+    }
+
     if ( (nType & VCL_INPUT_ANY) == VCL_INPUT_ANY )
     {
         // revert bugfix for #108919# which never reported timeouts when called from the timer handler
@@ -806,32 +915,52 @@ bool WinSalInstance::AnyInput( VclInputFlags nType )
     }
     else
     {
-        if ( nType & VclInputFlags::MOUSE )
-        {
-            // Test for mouse input
-            if ( PeekMessageW( &aMsg, 0, WM_MOUSEFIRST, WM_MOUSELAST,
-                                  PM_NOREMOVE | PM_NOYIELD ) )
-                return true;
-        }
+        const bool bCheck_KEYBOARD (nType & VclInputFlags::KEYBOARD);
+        const bool bCheck_OTHER    (nType & VclInputFlags::OTHER);
 
-        if ( nType & VclInputFlags::KEYBOARD )
+        // If there is a modifier key event, it counts as OTHER
+        // Previously we were simply ignoring these events...
+        if ( bCheck_KEYBOARD || bCheck_OTHER )
         {
-            // Test for key input
-            if ( PeekMessageW( &aMsg, 0, WM_KEYDOWN, WM_KEYDOWN,
+            if ( PeekMessageW( &aMsg, nullptr, WM_KEYDOWN, WM_KEYDOWN,
                                   PM_NOREMOVE | PM_NOYIELD ) )
             {
-                if ( (aMsg.wParam == VK_SHIFT)   ||
-                     (aMsg.wParam == VK_CONTROL) ||
-                     (aMsg.wParam == VK_MENU) )
-                    return false;
-                else
+                const bool bIsModifier = ( (aMsg.wParam == VK_SHIFT) ||
+                    (aMsg.wParam == VK_CONTROL) || (aMsg.wParam == VK_MENU) );
+                if ( bCheck_KEYBOARD && !bIsModifier )
+                    return true;
+                if ( bCheck_OTHER && bIsModifier )
                     return true;
             }
         }
 
+        // Other checks for all messages not excluded.
+        // The less we exclude, the less ranges have to be checked!
+        if ( bCheck_OTHER )
+        {
+            // TIMER and KEYBOARD are already handled, so always exclude them!
+            VclInputFlags nOtherType = nType &
+                ~VclInputFlags(VclInputFlags::KEYBOARD | VclInputFlags::TIMER);
+
+            std::vector<MsgRange> aMsgRangeList( GetOtherRanges( nOtherType ) );
+            for ( MsgRange aRange : aMsgRangeList )
+                if ( PeekMessageW( &aMsg, nullptr, aRange.nStart,
+                                   aRange.nEnd, PM_NOREMOVE | PM_NOYIELD ) )
+                    return true;
+
+            // MOUSE and PAINT already handled, so skip futher checks
+            return false;
+        }
+
+        if ( nType & VclInputFlags::MOUSE )
+        {
+            if ( PeekMessageW( &aMsg, nullptr, WM_MOUSEFIRST, WM_MOUSELAST,
+                                  PM_NOREMOVE | PM_NOYIELD ) )
+                return true;
+        }
+
         if ( nType & VclInputFlags::PAINT )
         {
-            // Test for paint input
             if ( PeekMessageW( &aMsg, 0, WM_PAINT, WM_PAINT,
                                   PM_NOREMOVE | PM_NOYIELD ) )
                 return true;
@@ -852,22 +981,6 @@ bool WinSalInstance::AnyInput( VclInputFlags nType )
                                   PM_NOREMOVE | PM_NOYIELD ) )
                 return true;
         }
-
-        if ( nType & VclInputFlags::TIMER )
-        {
-            // Test for timer input
-            if ( PeekMessageW( &aMsg, 0, WM_TIMER, WM_TIMER,
-                                  PM_NOREMOVE | PM_NOYIELD ) )
-                return true;
-
-        }
-
-        if ( nType & VclInputFlags::OTHER )
-        {
-            // Test for any input
-            if ( PeekMessageW( &aMsg, 0, 0, 0, PM_NOREMOVE | PM_NOYIELD ) )
-                return true;
-        }
     }
 
     return false;
commit 59ef49b5b31e3338c8a3e7675d6c83424fe73244
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Thu Oct 12 16:00:42 2017 +0200
Commit:     Thorsten Behrens <Thorsten.Behrens at CIB.de>
CommitDate: Wed Jan 16 19:32:57 2019 +0100

    WIN another system loop integration attempt
    
    This time we skip the intention to handle our Scheduler
    completely via the system event loop. Instead we basically
    guarantee to process a Scheduler event during each DoYield,
    if one is available. This way we won't block system events
    when busy in our event loop.
    
    Change-Id: I37094e61cbf928733151d9cc3299cdac76b3a5cd
    Reviewed-on: https://gerrit.libreoffice.org/43349
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    Reviewed-on: https://gerrit.libreoffice.org/66450
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    Tested-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>

diff --git a/vcl/inc/win/saltimer.h b/vcl/inc/win/saltimer.h
index 06e77b171cb3..37976bbfaf8b 100644
--- a/vcl/inc/win/saltimer.h
+++ b/vcl/inc/win/saltimer.h
@@ -26,15 +26,18 @@ class WinSalTimer final : public SalTimer, protected VersionedEvent
 {
     // for access to Impl* functions
     friend LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, int& rDef );
-    // for access to m_bPollForMessage
-    friend static void CALLBACK SalTimerProc( PVOID data, BOOLEAN );
+    // for access to GetNextVersionedEvent
+    friend void CALLBACK SalTimerProc( PVOID data, BOOLEAN );
+    // for access to ImplHandleElapsedTimer
+    friend bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents );
 
     HANDLE       m_nTimerId;          ///< Windows timer id
-    bool         m_bPollForMessage;   ///< Run yield until a message is caught (most likely the 0ms timer)
+    bool         m_bDirectTimeout;    ///< timeout can be processed directly
 
     void ImplStart( sal_uIntPtr nMS );
     void ImplStop();
-    void ImplEmitTimerCallback();
+    void ImplHandleTimerEvent( WPARAM aWPARAM );
+    void ImplHandleElapsedTimer();
 
 public:
     WinSalTimer();
@@ -43,19 +46,12 @@ public:
     virtual void Start(sal_uIntPtr nMS) override;
     virtual void Stop() override;
 
-    inline bool IsValidWPARAM( WPARAM wParam ) const;
-
-    inline bool PollForMessage() const;
+    inline bool IsDirectTimeout() const;
 };
 
-inline bool WinSalTimer::IsValidWPARAM( WPARAM aWPARAM ) const
-{
-    return IsValidEventVersion( static_cast<sal_Int32>( aWPARAM ) );
-}
-
-inline bool WinSalTimer::PollForMessage() const
+inline bool WinSalTimer::IsDirectTimeout() const
 {
-    return m_bPollForMessage;
+    return m_bDirectTimeout;
 }
 
 #endif
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 7d5bdde80f75..054602c46641 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -575,12 +575,11 @@ static void ImplSalDispatchMessage( MSG* pMsg )
         ImplSalPostDispatchMsg( pMsg, lResult );
 }
 
-bool
-ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
+bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
 {
     static sal_uInt32 nLastTicks = 0;
     MSG aMsg;
-    bool bWasMsg = false, bOneEvent = false;
+    bool bWasMsg = false, bOneEvent = false, bWasTimeoutMsg = false;
     ImplSVData *const pSVData = ImplGetSVData();
     WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer );
 
@@ -595,6 +594,8 @@ ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
         if ( bOneEvent )
         {
             bWasMsg = true;
+            if ( !bWasTimeoutMsg )
+                bWasTimeoutMsg = (SAL_MSG_TIMER_CALLBACK == aMsg.message);
             TranslateMessage( &aMsg );
             ImplSalDispatchMessage( &aMsg );
             if ( bHandleAllCurrentEvents
@@ -603,23 +604,26 @@ ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
                 bHadNewerEvent = true;
             bOneEvent = !bHadNewerEvent;
         }
-        // busy loop to catch a message, eventually the 0ms timer.
-        // we don't need to loop, if we wait anyway.
-        if ( !bWait && !bWasMsg && pTimer && pTimer->PollForMessage() )
-        {
-            SwitchToThread();
-            continue;
-        }
+
         if ( !(bHandleAllCurrentEvents && bOneEvent) )
             break;
     }
     while( true );
 
+    // 0ms timeouts are handled out-of-bounds to prevent busy-locking the
+    // event loop with timeout massages.
+    // We ensure we never handle more then one timeout per call.
+    // This way we'll always process a normal system message.
+    if ( !bWasTimeoutMsg && pTimer && pTimer->IsDirectTimeout() )
+    {
+        pTimer->ImplHandleElapsedTimer();
+        bWasMsg = true;
+    }
+
     if ( bHandleAllCurrentEvents )
         nLastTicks = nCurTicks;
 
-    // Also check that we don't wait when application already has quit
-    if ( bWait && !bWasMsg && !pSVData->maAppData.mbAppQuit )
+    if ( bWait && !bWasMsg )
     {
         if ( GetMessageW( &aMsg, 0, 0, 0 ) )
         {
@@ -645,6 +649,7 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong
     //       so don't do anything dangerous before releasing it here
     sal_uLong const nCount = (nReleased != 0)
                              ? nReleased : ImplSalReleaseYieldMutex();
+
     if ( !IsMainThread() )
     {
         // #97739# A SendMessage call blocks until the called thread (here: the main thread)
@@ -749,17 +754,7 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i
         {
             WinSalTimer *const pTimer = static_cast<WinSalTimer*>( ImplGetSVData()->maSchedCtx.mpSalTimer );
             assert( pTimer != nullptr );
-            MSG aMsg;
-            bool bValidMSG = pTimer->IsValidWPARAM( wParam );
-            // PM_QS_POSTMESSAGE is needed, so we don't process the SendMessage from DoYield!
-            while ( PeekMessageW(&aMsg, GetSalData()->mpFirstInstance->mhComWnd, SAL_MSG_TIMER_CALLBACK,
-                                 SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) )
-            {
-                assert( !bValidMSG && "Unexpected non-last valid message" );
-                bValidMSG = pTimer->IsValidWPARAM( aMsg.wParam );
-            }
-            if ( bValidMSG )
-                pTimer->ImplEmitTimerCallback();
+            pTimer->ImplHandleTimerEvent( wParam );
             break;
         }
     }
diff --git a/vcl/win/app/saltimer.cxx b/vcl/win/app/saltimer.cxx
index fff4cb65a2c7..f155de0d4ce8 100644
--- a/vcl/win/app/saltimer.cxx
+++ b/vcl/win/app/saltimer.cxx
@@ -28,7 +28,7 @@
 #include <sehandler.hxx>
 #endif
 
-static void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired);
+void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired);
 
 // See http://msdn.microsoft.com/en-us/library/windows/desktop/ms687003%28v=vs.85%29.aspx
 // (and related pages) for details about the Timer Queues.
@@ -48,16 +48,11 @@ void WinSalTimer::ImplStop()
         return;
 
     m_nTimerId = nullptr;
+    m_bDirectTimeout = false;
     DeleteTimerQueueTimer( nullptr, hTimer, INVALID_HANDLE_VALUE );
-    // Keep both after DeleteTimerQueueTimer, because they are set in SalTimerProc
+    // Keep InvalidateEvent after DeleteTimerQueueTimer, because the event id
+    // is set in SalTimerProc, which DeleteTimerQueueTimer will finish or cancel.
     InvalidateEvent();
-    m_bPollForMessage = false;
-
-    // remove as many pending SAL_MSG_TIMER_CALLBACK messages as possible
-    // PM_QS_POSTMESSAGE is needed, so we don't process the SendMessage from DoYield!
-    MSG aMsg;
-    while ( PeekMessageW(&aMsg, pInst->mhComWnd, SAL_MSG_TIMER_CALLBACK,
-                         SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) );
 }
 
 void WinSalTimer::ImplStart( sal_uLong nMS )
@@ -72,20 +67,21 @@ void WinSalTimer::ImplStart( sal_uLong nMS )
     // cannot change a one-shot timer, so delete it and create a new one
     ImplStop();
 
-    // keep the yield running, if a 0ms Idle is scheduled
-    m_bPollForMessage = ( 0 == nMS );
+    // directly indicate an elapsed timer
+    m_bDirectTimeout = ( 0 == nMS );
     // probably WT_EXECUTEONLYONCE is not needed, but it enforces Period
     // to be 0 and should not hurt; also see
     // https://www.microsoft.com/msj/0499/pooling/pooling.aspx
-    CreateTimerQueueTimer(&m_nTimerId, nullptr, SalTimerProc,
-                          reinterpret_cast<void*>(
-                              sal_IntPtr(GetNextEventVersion())),
-                          nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
+    if ( !m_bDirectTimeout )
+        CreateTimerQueueTimer(&m_nTimerId, nullptr, SalTimerProc, this,
+                              nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
+    // We don't need any wakeup message, as this code can just run in the
+    // main thread!
 }
 
 WinSalTimer::WinSalTimer()
     : m_nTimerId( nullptr )
-    , m_bPollForMessage( false )
+    , m_bDirectTimeout( false )
 {
 }
 
@@ -124,7 +120,7 @@ void WinSalTimer::Stop()
  * This gets invoked from a Timer Queue thread.
  * Don't acquire the SolarMutex to avoid deadlocks.
  */
-static void CALLBACK SalTimerProc(PVOID data, BOOLEAN)
+void CALLBACK SalTimerProc(PVOID data, BOOLEAN)
 {
 #if defined ( __MINGW32__ ) && !defined ( _WIN64 )
     jmp_buf jmpbuf;
@@ -136,11 +132,10 @@ static void CALLBACK SalTimerProc(PVOID data, BOOLEAN)
     __try
     {
 #endif
-        // always post message when the timer fires, we will remove the ones
-        // that happened during execution of the callback later directly from
-        // the message queue
-        BOOL const ret = PostMessageW(GetSalData()->mpFirstInstance->mhComWnd,
-                                      SAL_MSG_TIMER_CALLBACK, (WPARAM) data, 0);
+        WinSalTimer *pTimer = reinterpret_cast<WinSalTimer*>( data );
+        BOOL const ret = PostMessageW(
+            GetSalData()->mpFirstInstance->mhComWnd, SAL_MSG_TIMER_CALLBACK,
+            static_cast<WPARAM>(pTimer->GetNextEventVersion()), 0 );
 #if OSL_DEBUG_LEVEL > 0
         if (0 == ret) // SEH prevents using SAL_WARN here?
             fputs("ERROR: PostMessage() failed!\n", stderr);
@@ -158,15 +153,24 @@ static void CALLBACK SalTimerProc(PVOID data, BOOLEAN)
 
 }
 
-void WinSalTimer::ImplEmitTimerCallback()
+void WinSalTimer::ImplHandleElapsedTimer()
 {
     // Test for MouseLeave
     SalTestMouseLeave();
 
-    m_bPollForMessage = false;
+    m_bDirectTimeout = false;
     ImplSalYieldMutexAcquireWithWait();
     CallCallback();
     ImplSalYieldMutexRelease();
 }
 
+void WinSalTimer::ImplHandleTimerEvent( const WPARAM aWPARAM )
+{
+    assert( aWPARAM <= SAL_MAX_INT32 );
+    if ( !IsValidEventVersion( static_cast<sal_Int32>( aWPARAM ) ) )
+        return;
+
+    ImplHandleElapsedTimer();
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */


More information about the Libreoffice-commits mailing list