tdf#112288 Clarify Reschedule implementations

Application::Reschedule(true) must just process all currently
pending events and ignore all new events generated while processing
them. In contrast to Scheduler::ProcessEventsToIdle, this way it
can't busy-lock the application with background jobs.

This way we also can drop nMaxEvents from the Windows backend. This
limit was also never implemented on OSX and for the KDE4 backend
it's actually impossible to handle single events, and a call to
QAbstractEventDispatcher::processEvents works like this.

Also changes various call sites to just process all pending events
instead of some made up number of times.

Change-Id: I1ab95df89b079cc8c6319a808194fe3127144d1c
Reviewed-on: https://gerrit.libreoffice.org/42659
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>

16 files changed, 119 insertions, 71 deletions

diff --git a/cui/source/dialogs/cuigaldlg.cxx b/cui/source/dialogs/cuigaldlg.cxx
index 3a8c9b60d341..6b1c6a0411d0 100644
--- a/cui/source/dialogs/cuigaldlg.cxx
+++ b/cui/source/dialogs/cuigaldlg.cxx
@@ -483,8 +483,7 @@ IMPL_LINK( ActualizeProgress, TimeoutHdl, Timer*, _pTimer, void)
 
 IMPL_LINK( ActualizeProgress, ActualizeHdl, const INetURLObject&, rURL, void )
 {
-    for( long i = 0; i < 128; i++ )
-        Application::Reschedule();
+    Application::Reschedule( true );
 
     Flush();
 
diff --git a/include/vcl/scheduler.hxx b/include/vcl/scheduler.hxx
index 5c9d8f0d0d6e..2d422c6e9678 100644
--- a/include/vcl/scheduler.hxx
+++ b/include/vcl/scheduler.hxx
@@ -53,7 +53,16 @@ public:
     static void       CallbackTaskScheduling();
     /// Process one pending task ahead of time with highest priority.
     static bool       ProcessTaskScheduling();
-    /// Process all events until we are idle
+    /**
+     * Process all events until none is pending
+     *
+     * This can busy-lock, if some task or system event always generates new
+     * events when being processed. Most time it's called in unit tests to
+     * process all pending events. Internally it just calls
+     * Application::Reschedule( true ) until it fails.
+     *
+     * @see Application::Reschedule
+     */
     static void       ProcessEventsToIdle();
     /**
      * Process events until the parameter turns true,
diff --git a/include/vcl/svapp.hxx b/include/vcl/svapp.hxx
index 3e3938498b75..e8f4b2686f2c 100644
--- a/include/vcl/svapp.hxx
+++ b/include/vcl/svapp.hxx
@@ -466,17 +466,19 @@ public:
     /** Attempt to process current pending event(s)
 
      It doesn't sleep if no events are available for processing.
+     This doesn't processs any events generated after invoking the function.
+     So in contrast to Scheduler::ProcessEventsToIdle, this cannot become
+     busy-locked by an event-generating event in the event queue.
 
-     @param bAllEvents  If set to true, then try to process all the
-        events. If set to false, then only process the current
-        event. Defaults to false.
+     @param bHandleAllCurrentEvents  If set to true, then try to process all
+        the current events. If set to false, then only process one event.
+        Defaults to false.
 
      @returns true if any event was processed.
 
-     @see Execute, Quit, Yield, EndYield, GetSolarMutex,
-          GetMainThreadIdentifier, ReleaseSolarMutex, AcquireSolarMutex,
+     @see Yield, Scheduler::ProcessEventsToIdle
      */
-    static bool                 Reschedule( bool bAllEvents = false );
+    static bool                 Reschedule( bool bHandleAllCurrentEvents = false );
 
     /** Process the next event.
 
diff --git a/sc/qa/unit/tiledrendering/tiledrendering.cxx b/sc/qa/unit/tiledrendering/tiledrendering.cxx
index fd9c71197ded..1f56bba58179 100644
--- a/sc/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sc/qa/unit/tiledrendering/tiledrendering.cxx
@@ -1400,7 +1400,6 @@ void ScTiledRenderingTest::testDisableUndoRepair()
     pModelObj->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, awt::Key::RETURN);
     pModelObj->postKeyEvent(LOK_KEYEVENT_KEYUP, 0, awt::Key::RETURN);
     Scheduler::ProcessEventsToIdle();
-    Scheduler::ProcessEventsToIdle();
     {
         SfxItemSet aSet1(pView1->GetPool(), svl::Items<SID_UNDO, SID_UNDO>{});
         SfxItemSet aSet2(pView2->GetPool(), svl::Items<SID_UNDO, SID_UNDO>{});
diff --git a/svx/source/form/fmsrcimp.cxx b/svx/source/form/fmsrcimp.cxx
index 596e942f2348..75b04fabd1cd 100644
--- a/svx/source/form/fmsrcimp.cxx
+++ b/svx/source/form/fmsrcimp.cxx
@@ -311,13 +311,7 @@ FmSearchEngine::SearchResult FmSearchEngine::SearchSpecial(bool _bSearchForNull,
     bool bMovedAround(false);
     do
     {
-        Application::Reschedule();
-        Application::Reschedule();
-        // do 2 reschedules because of #70226# : some things done within this loop's body may cause an user event
-        // to be posted (deep within vcl), and these user events will be handled before any keyinput or paintings
-        // or anything like that. So within each loop we create one user event and handle one user event (and no
-        // paintings and these), so the office seems to be frozen while searching.
-        // FS - 70226 - 02.12.99
+        Application::Reschedule( true );
 
         // the content to be compared currently
         iterFieldLoop->xContents->getString();  // needed for wasNull
@@ -376,13 +370,7 @@ FmSearchEngine::SearchResult FmSearchEngine::SearchWildcard(const OUString& strE
     bool bMovedAround(false);
     do
     {
-        Application::Reschedule();
-        Application::Reschedule();
-        // do 2 reschedules because of #70226# : some things done within this loop's body may cause an user event
-        // to be posted (deep within vcl), and these user events will be handled before any keyinput or paintings
-        // or anything like that. So within each loop we create one user event and handle one user event (and no
-        // paintings and these), so the office seems to be frozen while searching.
-        // FS - 70226 - 02.12.99
+        Application::Reschedule( true );
 
         // the content to be compared currently
         OUString sCurrentCheck;
@@ -476,13 +464,7 @@ FmSearchEngine::SearchResult FmSearchEngine::SearchRegularApprox(const OUString&
     bool bMovedAround(false);
     do
     {
-        Application::Reschedule();
-        Application::Reschedule();
-        // do 2 reschedules because of #70226# : some things done within this loop's body may cause an user event
-        // to be posted (deep within vcl), and these user events will be handled before any keyinput or paintings
-        // or anything like that. So within each loop we create one user event and handle one user event (and no
-        // paintings and these), so the office seems to be frozen while searching.
-        // FS - 70226 - 02.12.99
+        Application::Reschedule( true );
 
         // the content to be compared currently
         OUString sCurrentCheck;
diff --git a/sw/source/ui/dbui/addresslistdialog.cxx b/sw/source/ui/dbui/addresslistdialog.cxx
index 254bf5a1cd68..2f80c7db6823 100644
--- a/sw/source/ui/dbui/addresslistdialog.cxx
+++ b/sw/source/ui/dbui/addresslistdialog.cxx
@@ -487,8 +487,7 @@ IMPL_LINK(SwAddressListDialog, StaticListBoxSelectHdl_Impl, void*, p, void)
             m_pListLB->SetEntryText(m_sConnecting, pSelect, ITEMID_TABLE - 1);
             // allow painting of the new entry
             m_pListLB->Window::Invalidate(InvalidateFlags::Update);
-            for (int i = 0; i < 10; ++i)
-                Application::Reschedule();
+            Application::Reschedule( true );
         }
 
         pUserData = static_cast<AddressUserData_Impl*>(pSelect->GetUserData());
diff --git a/sw/source/ui/dbui/mmresultdialogs.cxx b/sw/source/ui/dbui/mmresultdialogs.cxx
index 31e64f75a7a7..e24e73121b86 100644
--- a/sw/source/ui/dbui/mmresultdialogs.cxx
+++ b/sw/source/ui/dbui/mmresultdialogs.cxx
@@ -717,8 +717,7 @@ IMPL_LINK(SwMMResultSaveDialog, SaveOutputHdl_Impl, Button*, pButton, void)
             while(true)
             {
                 //time for other slots is needed
-                for(sal_Int16 r = 0; r < 10; ++r)
-                    Application::Reschedule();
+                Application::Reschedule( true );
                 bool bFailed = false;
                 try
                 {
@@ -1086,8 +1085,7 @@ IMPL_LINK(SwMMResultEmailDialog, SendDocumentsHdl_Impl, Button*, pButton, void)
     //help to force painting the dialog
     //TODO/CLEANUP
     //predetermined breaking point
-    for ( sal_Int16 i = 0; i < 25; i++)
-        Application::Reschedule();
+    Application::Reschedule( true );
     for(sal_uInt32 nDoc = nBegin; nDoc < nEnd; ++nDoc)
     {
         SwDocMergeInfo& rInfo = xConfigItem->GetDocumentMergeInfo(nDoc);
@@ -1229,8 +1227,7 @@ IMPL_LINK(SwMMResultEmailDialog, SendDocumentsHdl_Impl, Button*, pButton, void)
         aDesc.sBCC = m_sBCC;
         pDlg->AddDocument( aDesc );
         //help to force painting the dialog
-        for ( sal_Int16 i = 0; i < 25; i++)
-            Application::Reschedule();
+        Application::Reschedule( true );
         //stop creating of data when dialog has been closed
         if(!pDlg->IsVisible())
         {
diff --git a/sw/source/uibase/dbui/dbmgr.cxx b/sw/source/uibase/dbui/dbmgr.cxx
index 2285695fba2f..1c3d6a56679f 100644
--- a/sw/source/uibase/dbui/dbmgr.cxx
+++ b/sw/source/uibase/dbui/dbmgr.cxx
@@ -147,11 +147,6 @@ using namespace ::com::sun::star;
 
 namespace {
 
-void rescheduleGui() {
-    for( sal_uInt16 i = 0; i < 25; i++)
-        Application::Reschedule();
-}
-
 void lcl_emitEvent(SfxEventHintId nEventId, sal_Int32 nStrId, SfxObjectShell* pDocShell)
 {
     SfxGetpApp()->NotifyEvent(SfxEventHint(nEventId,
@@ -1261,7 +1256,7 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell,
         pProgressDlg->SetCancelHdl( LINK(this, SwDBManager, PrtCancelHdl) );
         pProgressDlg->Show();
 
-        rescheduleGui();
+        Application::Reschedule( true );
     }
 
     if( bCreateSingleFile && !pTargetView )
@@ -1404,7 +1399,7 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell,
                 pProgressDlg->Update();
             }
 
-            rescheduleGui();
+            Application::Reschedule( true );
 
             // Create a copy of the source document and work with that one instead of the source.
             // If we're not in the single file mode (which requires modifying the document for the merging),
@@ -1574,7 +1569,7 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell,
     }
     else if( IsMergeOk() ) // && bCreateSingleFile
     {
-        rescheduleGui();
+        Application::Reschedule( true );
 
         // sw::DocumentLayoutManager::CopyLayoutFormat() did not generate
         // unique fly names, do it here once.
@@ -1593,7 +1588,7 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell,
             aLayout->AllCheckPageDescs();
         }
 
-        rescheduleGui();
+        Application::Reschedule( true );
 
         if( IsMergeOk() && bMT_FILE )
         {
@@ -1631,7 +1626,7 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell,
     else if( xTargetDocShell.is() )
         xTargetDocShell->DoClose();
 
-    rescheduleGui();
+    Application::Reschedule( true );
 
     pProgressDlg.disposeAndClear();
 
diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index a7d2cbb04682..9f3d8b203829 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -104,6 +104,24 @@ normally wait for the SolarMutex.
 Eventually this will move into the GenericSolarMutex. KDE / Qt also does main
 thread redirects using Qt::BlockingQueuedConnection.
 
+== General: processing all current events for DoYield ==
+
+This is easily implemented for all non-priority queue based implementations.
+Windows and MacOS both have a timestamp attached to their events / messages,
+so simply get the current time and just process anything < timestamp.
+For the KDE backend this is already the default behaviour - single event
+processing isn't even supported. The headless backend accomplishes this by
+just processing a copy of the list of current events.
+
+Problematic in this regard is the Gtk+ backend. g_main_context_iteration
+dispatches "only those highest priority event sources". There is no real way
+to tell, when these became ready. I've added a workaround idea to the TODO
+list. FWIW: Qt runs just a single timer source in the glib main context,
+basically the same we're doing with the LO scheduler as a system event.
+
+The gen X11 backend has some levels of redirection, but needs quite some work
+to get this fixed.
+
 == MacOS implementation details ==
 
 Generally the Scheduler is handled as expected, except on resize, which is
@@ -224,3 +242,39 @@ workaround using a validation timestamp is better then the current peek,
 remove, re-postEvent, which has to run in the main thread.
 
 Originally I didn't evaluate, if the event is actually lost or just delayed.
+
+== Drop nMaxEvents from Gtk+ based backends ==
+
+gint last_priority = G_MAXINT;
+bool bWasEvent = false;
+do {
+    gint max_priority;
+    g_main_context_acquire( NULL );
+    bool bHasPending = g_main_context_prepare( NULL, &max_priority );
+    g_main_context_release( NULL );
+    if ( bHasPending )
+    {
+        if ( last_priority > max_priority )
+        {
+            bHasPending = g_main_context_iteration( NULL, bWait );
+	    bWasEvent = bWasEvent || bHasPending;
+	}
+	else
+	    bHasPending = false;
+    }
+}
+while ( bHasPending )
+
+The idea is to use g_main_context_prepare and keep the max_priority as an
+indicator. We cannot prevent running newer lower events, but we can prevent
+running new higher events, which should be sufficient for most stuff.
+
+This also touches user event processing, which currently runs as a high
+priority idle in the event loop.
+
+== Drop nMaxEvents from gen (X11) backend ==
+
+A few layers of indirection make this code hard to follow. The SalXLib::Yield
+and SalX11Display::Yield architecture makes it impossible to process just the
+current events. This really needs a refactorung and rearchitecture step, which
+will also affect the Gtk+ and KDE4 backend for the user event handling.
diff --git a/vcl/headless/svpinst.cxx b/vcl/headless/svpinst.cxx
index 302405f8f59d..ab8f91eac31f 100644
--- a/vcl/headless/svpinst.cxx
+++ b/vcl/headless/svpinst.cxx
@@ -180,7 +180,7 @@ bool SvpSalInstance::PostedEventsInQueue()
     bool result = false;
     {
         osl::MutexGuard g(m_aEventGuard);
-        result = m_aUserEvents.size() > 0;
+        result = !m_aUserEvents.empty();
     }
     return result;
 }
diff --git a/vcl/osx/salinst.cxx b/vcl/osx/salinst.cxx
index 947e484b5c1d..e3e101cbc13e 100644
--- a/vcl/osx/salinst.cxx
+++ b/vcl/osx/salinst.cxx
@@ -582,6 +582,7 @@ bool AquaSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
     {
         // handle available events
         NSEvent* pEvent = nil;
+        NSDate *now = [[NSDate alloc] init];
         do
         {
             SolarMutexReleaser aReleaser;
@@ -590,7 +591,7 @@ SAL_WNODEPRECATED_DECLARATIONS_PUSH
     // 'NSAnyEventMask' is deprecated: first deprecated in macOS 10.12
             pEvent = [NSApp nextEventMatchingMask: NSAnyEventMask
 SAL_WNODEPRECATED_DECLARATIONS_POP
-                            untilDate: nil
+                            untilDate: now
                             inMode: NSDefaultRunLoopMode
                             dequeue: YES];
             if( pEvent )
diff --git a/vcl/source/app/svapp.cxx b/vcl/source/app/svapp.cxx
index 612fce86bc2f..be09136b1717 100644
--- a/vcl/source/app/svapp.cxx
+++ b/vcl/source/app/svapp.cxx
@@ -488,7 +488,7 @@ bool Application::Reschedule( bool i_bAllEvents )
 
 void Scheduler::ProcessEventsToSignal(bool& bSignal)
 {
-    while (!bSignal && Application::Reschedule( false ) );
+    while (!bSignal && Application::Reschedule() );
 }
 
 void Scheduler::ProcessEventsToIdle()
@@ -516,7 +516,8 @@ void Scheduler::ProcessEventsToIdle()
             Idle *pIdle = dynamic_cast<Idle*>( pSchedulerData->mpTask );
             if ( pIdle && pIdle->IsActive() )
             {
-                SAL_WARN( "vcl.schedule", "Unprocessed Idle: " << pIdle->GetDebugName() );
+                SAL_WARN( "vcl.schedule", "Unprocessed Idle: "
+                          << pIdle << " " << pIdle->GetDebugName() );
             }
         }
         pSchedulerData = pSchedulerData->mpNext;
diff --git a/vcl/source/gdi/print2.cxx b/vcl/source/gdi/print2.cxx
index d546825e7f50..94a9494ccb27 100644
--- a/vcl/source/gdi/print2.cxx
+++ b/vcl/source/gdi/print2.cxx
@@ -1230,8 +1230,7 @@ bool OutputDevice::RemoveTransparenciesFromMetaFile( const GDIMetaFile& rInMtf,
                                             pCurrAct->Execute( aPaintVDev.get() );
                                         }
 
-                                        if( !( nActionNum % 8 ) )
-                                            Application::Reschedule();
+                                        Application::Reschedule( true );
                                     }
 
                                     const bool bOldMap = mbMap;
diff --git a/vcl/unx/kde4/KDEXLib.cxx b/vcl/unx/kde4/KDEXLib.cxx
index d9e049142c1a..402461e45bc6 100644
--- a/vcl/unx/kde4/KDEXLib.cxx
+++ b/vcl/unx/kde4/KDEXLib.cxx
@@ -293,8 +293,7 @@ bool KDEXLib::Yield( bool bWait, bool bHandleAllCurrentEvents )
         // (it's ok to release it here, since even normal processYield() would
         // temporarily do it while checking for new events)
         SolarMutexReleaser aReleaser;
-        Q_EMIT processYieldSignal( bWait, bHandleAllCurrentEvents );
-        return false;
+        return Q_EMIT processYieldSignal( bWait, bHandleAllCurrentEvents );
     }
 }
 
diff --git a/vcl/unx/kde4/KDEXLib.hxx b/vcl/unx/kde4/KDEXLib.hxx
index 01076286c429..2fe497d019fa 100644
--- a/vcl/unx/kde4/KDEXLib.hxx
+++ b/vcl/unx/kde4/KDEXLib.hxx
@@ -68,7 +68,7 @@ class KDEXLib : public QObject, public SalXLib
 
     Q_SIGNALS:
         void startTimeoutTimerSignal();
-        void processYieldSignal( bool bWait, bool bHandleAllCurrentEvents );
+        bool processYieldSignal( bool bWait, bool bHandleAllCurrentEvents );
         css::uno::Reference< css::ui::dialogs::XFilePicker2 >
             createFilePickerSignal( const css::uno::Reference< css::uno::XComponentContext >& );
 
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index be96ed17c654..23e48532d3b9 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -506,12 +506,17 @@ static void ImplSalDispatchMessage( MSG* pMsg )
 
 static bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
 {
+    static sal_uInt32 nLastTicks = 0;
     MSG aMsg;
     bool bWasMsg = false, bOneEvent = false;
     ImplSVData *const pSVData = ImplGetSVData();
     WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer );
 
-    int nMaxEvents = bHandleAllCurrentEvents ? 100 : 1;
+    sal_uInt32 nCurTicks = 0;
+    if ( bHandleAllCurrentEvents )
+        nCurTicks = GetTickCount();
+
+    bool bHadNewerEvent = false;
     do
     {
         bOneEvent = PeekMessageW( &aMsg, nullptr, 0, 0, PM_REMOVE );
@@ -520,20 +525,26 @@ static bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
             bWasMsg = true;
             TranslateMessage( &aMsg );
             ImplSalDispatchMessage( &aMsg );
+            if ( bHandleAllCurrentEvents
+                    && !bHadNewerEvent && aMsg.time > nCurTicks
+                    && (nLastTicks <= nCurTicks || aMsg.time < nLastTicks) )
+                bHadNewerEvent = true;
+            bOneEvent = !bHadNewerEvent;
         }
-        else
-            // busy loop to catch the 0ms timeout
-            // We don't need to busy loop, if we wait anyway.
-            // Even if we didn't process the event directly, report it.
-            if ( pTimer && pTimer->PollForMessage() && !bWait )
-            {
-                SwitchToThread();
-                nMaxEvents++;
-                bOneEvent = true;
-                bWasMsg = true;
-            }
+        // 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( --nMaxEvents && bOneEvent );
+    while( true );
+
+    if ( bHandleAllCurrentEvents )
+        nLastTicks = nCurTicks;
 
     // Also check that we don't wait when application already has quit
     if ( bWait && !bWasMsg && !pSVData->maAppData.mbAppQuit )
@@ -545,6 +556,7 @@ static bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
             ImplSalDispatchMessage( &aMsg );
         }
     }
+
     return bWasMsg;
 }