[Libreoffice-commits] core.git: sw/source

[Michael Stahl (via logerrit)]

 sw/source/core/inc/txtfrm.hxx   |    2 ++
 sw/source/core/text/frmform.cxx |    2 +-
 sw/source/core/text/txtfrm.cxx  |    8 ++++----
 3 files changed, 7 insertions(+), 5 deletions(-)

New commits:
commit 68a5afaaabd0c75bba3439cfdff90fb75d1cdd3f
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Fri Dec 20 17:28:55 2019 +0100
Commit:     Michael Stahl <michael.stahl at cib.de>
CommitDate: Mon Dec 23 10:04:57 2019 +0100

    sw: fix widow loop with as-char flys in text formatting
    
    The document has a paragraph with 4 as-character anchored flys depicting
    Zirbenholz; due to their size and an additional fly that is anchored at
    the paragraph, there are 3 lines that do not fit onto a single page.
    
    This situation causes a loop that proceeds like this:
    
    text frame 80 is the follow of text frame 21.
    when formatting 80:
        the 1 line violates the widow rule (>=2) and PREP_WIDOWS is sent to
            21, invalidating its FrameAreaSize
        80 validates its FrameAreaSize
    when formatting 21:
        PREP_WIDOWS_ORPHANS sent to 21
        CalcPreps() sees IsPrepWidows() and sets a huge height and calls
            SetWidow(true)
        SwTextFrame::WouldFit() sees IsWidow() true and resets it false
        SwTextFrame::WouldFit() sees IsWidow() false and a huge but
            insufficiently huge height
        21 validates its FrameAreaSize
        CalcPreps() sees IsPrepAdjust()
        FindBreak() calls TruncLines() and because of as-char fly
            invalidates FrameAreaSize of 80
    
    The loop is most easily reproduced by printing via the API; it's
    possible to get a loop when loading the document in the UI, but
    typically the UI remains responsive even though the layout never
    finishes.
    
    As it happens, before commit ee299664940139f6f9543592ece3b3c0210b59f4
    "SalInstance::DoYield: Don't drop SolarMutex when accessing user event
    queue" the loop on printing via API was broken by releasing SolarMutex;
    the result, however, was incorrect, with the last line of Zirbenholz
    that should be on the second page missing in the PDF.
    
    This loop is presumably a regression from commit
    f2e3655255db4032738849cd4b77ce67a6e2c984 "Avoid
    -fsanitize=signed-integer-overflow", which changed a magic number in
    SwTextFrame::CalcPreps(), but didn't adapt the poorly documented
    corresponding magic numbers in SwTextFrame::WouldFit(); in LO 5.1.6.2
    the CPU is idle after loading the document.
    
    Change-Id: Ib6563c21edb68945c14a61b51ba34f0ee3f2544a
    Reviewed-on: https://gerrit.libreoffice.org/85623
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at cib.de>

diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx
index 21297f872d35..11e975c6b004 100644
--- a/sw/source/core/inc/txtfrm.hxx
+++ b/sw/source/core/inc/txtfrm.hxx
@@ -1026,6 +1026,8 @@ public:
 };
 
 
+const SwTwips WIDOW_MAGIC = (SAL_MAX_INT32 - 1)/2;
+
 } // namespace sw
 
 #endif
diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx
index 0938a476f657..482823fc8dbf 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -838,7 +838,7 @@ bool SwTextFrame::CalcPreps()
                     // the range of 'long', while the value (SAL_MAX_INT32 - 1)/2 (which matches the
                     // old value on platforms where 'long' is 'sal_Int32') is empirically shown to
                     // be large enough in practice even on platforms where 'long' is 'sal_Int64':
-                    SwTwips nTmp  = (SAL_MAX_INT32 - 1)/2 - (getFrameArea().Top()+10000);
+                    SwTwips const nTmp = sw::WIDOW_MAGIC - (getFrameArea().Top()+10000);
                     SwTwips nDiff = nTmp - getFrameArea().Height();
 
                     {
diff --git a/sw/source/core/text/txtfrm.cxx b/sw/source/core/text/txtfrm.cxx
index 5100ae8eecca..cc32f19ac508 100644
--- a/sw/source/core/text/txtfrm.cxx
+++ b/sw/source/core/text/txtfrm.cxx
@@ -3251,9 +3251,9 @@ bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst )
     // Because the Orphan flag only exists for a short moment, we also check
     // whether the Framesize is set to very huge by CalcPreps, in order to
     // force a MoveFwd
-    if( IsWidow() || ( aRectFnSet.IsVert() ?
-                       ( 0 == getFrameArea().Left() ) :
-                       ( LONG_MAX - 20000 < getFrameArea().Bottom() ) ) )
+    if (IsWidow() || (aRectFnSet.IsVert()
+                        ? (0 == getFrameArea().Left())
+                        : (sw::WIDOW_MAGIC - 20000 < getFrameArea().Bottom())))
     {
         SetWidow(false);
         if ( GetFollow() )
@@ -3262,7 +3262,7 @@ bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst )
             // whether there's a Follow with a real height at all.
             // Else (e.g. for newly created SctFrames) we ignore the IsWidow() and
             // still check if we can find enough room
-            if( ( ( ! aRectFnSet.IsVert() && LONG_MAX - 20000 >= getFrameArea().Bottom() ) ||
+            if (((!aRectFnSet.IsVert() && getFrameArea().Bottom() <= sw::WIDOW_MAGIC - 20000) ||
                   (   aRectFnSet.IsVert() && 0 < getFrameArea().Left() ) ) &&
                   ( GetFollow()->IsVertical() ?
                     !GetFollow()->getFrameArea().Width() :