Resolves: tdf#94925 proper recalc mode and dirty broadcast for OOXML import

This is a combination of 4 commits.

Rework FormulaTokenArray ScRecalcMode in preparation for tdf#94925

Strictly order the exclusive bits by priority, let AddRecalcMode()
handle all sets except forced ALWAYS or NORMAL.

Introduce ONLOAD_LENIENT and ONLOAD_MUST splitting ONLOAD to be
able to distinguish later during OOXML import.

Reviewed-on: https://gerrit.libreoffice.org/57402
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins

Resolves: tdf#94925 do not unset dirty if formula cell must be recalculated

Reviewed-on: https://gerrit.libreoffice.org/57404
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins

Broadcast formula cells marked for recalc, tdf#94925 related

In fact the ScDocument::CalcFormulaTree() call in
WorkbookFragment::recalcFormulaCells() never did anything because
no formula cell was added to the tree. Only visible dirty cells
were recalculated, but not their dependents.

Reviewed-on: https://gerrit.libreoffice.org/57431
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins

Remove the check for IsRecalcModeMustAfterImport(), tdf#94925 follow-up

It's now superfluous as we set those cells dirty and broadcast in
ScDocumentImport::broadcastRecalcAfterImport()

Reviewed-on: https://gerrit.libreoffice.org/57439
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Eike Rathke <erack@redhat.com>

f70860b8babf1cce7fda2ae63412659e72dbb4c3
a9dd4ad16c20b23ee8a1d46b69a4702b1ad4c81f
188de2d53a2d54df32d24eeeb148c4f9e87e7cfc

Change-Id: I11217fa19adb766f509d0d6854502112de547c59
Reviewed-on: https://gerrit.libreoffice.org/57438
Tested-by: Jenkins
Reviewed-by: Markus Mohrhard <markus.mohrhard@googlemail.com>

11 files changed, 136 insertions, 64 deletions

diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx
index f5e06e77deba..4979310041e7 100644
--- a/formula/source/core/api/FormulaCompiler.cxx
+++ b/formula/source/core/api/FormulaCompiler.cxx
@@ -1424,14 +1424,15 @@ void FormulaCompiler::Factor()
             switch( eOp )
             {
                     // Functions recalculated on every document load.
-                    // Don't use SetExclusiveRecalcModeOnLoad() which would
-                    // override ModeAlways, use
-                    // AddRecalcMode(ScRecalcMode::ONLOAD) instead.
+                    // ONLOAD_LENIENT here to be able to distinguish and not
+                    // force a recalc (if not in an ALWAYS or ONLOAD_MUST
+                    // context) but keep an imported result from for example
+                    // OOXML a DDE call. Will be recalculated for ODFF.
                 case ocConvertOOo :
                 case ocDde:
                 case ocMacro:
                 case ocExternal:
-                    pArr->AddRecalcMode( ScRecalcMode::ONLOAD );
+                    pArr->AddRecalcMode( ScRecalcMode::ONLOAD_LENIENT );
                 break;
                     // If the referred cell is moved the value changes.
                 case ocColumn :
@@ -1439,15 +1440,15 @@ void FormulaCompiler::Factor()
                     pArr->SetRecalcModeOnRefMove();
                 break;
                     // ocCell needs recalc on move for some possible type values.
-                    // and recalc mode on load, fdo#60646
+                    // And recalc mode on load, tdf#60645
                 case ocCell :
                     pArr->SetRecalcModeOnRefMove();
-                    pArr->AddRecalcMode( ScRecalcMode::ONLOAD );
+                    pArr->AddRecalcMode( ScRecalcMode::ONLOAD_MUST );
                 break;
                 case ocHyperLink :
-                    // cell with hyperlink needs to be calculated on load to
+                    // Cell with hyperlink needs to be calculated on load to
                     // get its matrix result generated.
-                    pArr->AddRecalcMode( ScRecalcMode::ONLOAD );
+                    pArr->AddRecalcMode( ScRecalcMode::ONLOAD_MUST );
                     pArr->SetHyperLink( true);
                 break;
                 default:
diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx
index 42b19f8543c0..a8fb0b42541d 100644
--- a/formula/source/core/api/token.cxx
+++ b/formula/source/core/api/token.cxx
@@ -845,15 +845,27 @@ FormulaToken* FormulaTokenArray::AddStringXML( const OUString& rStr )
 
 void FormulaTokenArray::AddRecalcMode( ScRecalcMode nBits )
 {
-    //! Order is important.
-    if ( nBits & ScRecalcMode::ALWAYS )
-        SetExclusiveRecalcModeAlways();
-    else if ( !IsRecalcModeAlways() )
+    const unsigned nExclusive = static_cast<sal_uInt8>(nBits & ScRecalcMode::EMask);
+    if (nExclusive)
     {
-        if ( nBits & ScRecalcMode::ONLOAD )
-            SetExclusiveRecalcModeOnLoad();
-        else if ( nBits & ScRecalcMode::ONLOAD_ONCE && !IsRecalcModeOnLoad() )
-            SetExclusiveRecalcModeOnLoadOnce();
+        unsigned nExBit;
+        if (nExclusive & (nExclusive - 1))
+        {
+            // More than one bit set, use highest priority.
+            for (nExBit = 1; (nExBit & static_cast<sal_uInt8>(ScRecalcMode::EMask)) != 0; nExBit <<= 1)
+            {
+                if (nExclusive & nExBit)
+                    break;
+            }
+        }
+        else
+        {
+            // Only one bit is set.
+            nExBit = nExclusive;
+        }
+        // Set exclusive bit if priority is higher than existing.
+        if (nExBit < static_cast<sal_uInt8>(nMode & ScRecalcMode::EMask))
+            SetMaskedRecalcMode( static_cast<ScRecalcMode>(nExBit));
     }
     SetCombinedBitsRecalcMode( nBits );
 }
diff --git a/include/formula/tokenarray.hxx b/include/formula/tokenarray.hxx
index 45669d1ad529..2c422d94faf8 100644
--- a/include/formula/tokenarray.hxx
+++ b/include/formula/tokenarray.hxx
@@ -48,24 +48,26 @@ class SharedStringPool;
 
 }
 
-// RecalcMode access only via TokenArray SetRecalcMode / IsRecalcMode...
+// RecalcMode access only via TokenArray SetExclusiveRecalcMode...() /
+// IsRecalcMode...()
 
-// Only one of the exclusive bits can be set,
-// handled by TokenArray SetRecalcMode... methods
+// Only one of the exclusive bits can be set and one must be set,
+// handled by TokenArray SetExclusiveRecalcMode...() methods.
+// Exclusive bits are ordered by priority, AddRecalcMode() relies on that.
 enum class ScRecalcMode : sal_uInt8
 {
-    NORMAL       = 0x01,    // exclusive
-    ALWAYS       = 0x02,    // exclusive, always
-    ONLOAD       = 0x04,    // exclusive, always after load
-    ONLOAD_ONCE  = 0x08,    // exclusive, once after load
-    FORCED       = 0x10,    // combined, also if cell isn't visible
-    ONREFMOVE    = 0x20,    // combined, if reference was moved
-    EMask        = NORMAL | ALWAYS | ONLOAD | ONLOAD_ONCE  // mask of exclusive bits
+    ALWAYS         = 0x01,  // exclusive, always
+    ONLOAD_MUST    = 0x02,  // exclusive, always after load
+    ONLOAD_ONCE    = 0x04,  // exclusive, once after load, import filter
+    ONLOAD_LENIENT = 0x08,  // exclusive, lenient after load (eg. macros not always, aliens, ...)
+    NORMAL         = 0x10,  // exclusive
+    FORCED         = 0x20,  // combined, also if cell isn't visible, for macros with side effects
+    ONREFMOVE      = 0x40,  // combined, if reference was moved
+    EMask          = ALWAYS | ONLOAD_MUST | ONLOAD_LENIENT | ONLOAD_ONCE | NORMAL   // mask of exclusive bits
 };
-// If new bits are to be defined, AddRecalcMode has to be adjusted!
 namespace o3tl
 {
-    template<> struct typed_flags<ScRecalcMode> : is_typed_flags<ScRecalcMode, 0x3f> {};
+    template<> struct typed_flags<ScRecalcMode> : is_typed_flags<ScRecalcMode, 0x7f> {};
 }
 
 namespace formula
@@ -281,16 +283,6 @@ public:
      */
     sal_uInt16              RemoveToken( sal_uInt16 nOffset, sal_uInt16 nCount );
 
-    void            SetCombinedBitsRecalcMode( ScRecalcMode nBits )
-                                { nMode |= (nBits & ~ScRecalcMode::EMask); }
-    ScRecalcMode    GetCombinedBitsRecalcMode() const
-                                { return nMode & ~ScRecalcMode::EMask; }
-                            /** Exclusive bits already set in nMode are
-                                zero'ed, nBits may contain combined bits, but
-                                only one exclusive bit may be set! */
-    void            SetMaskedRecalcMode( ScRecalcMode nBits )
-                                { nMode = GetCombinedBitsRecalcMode() | nBits; }
-
     FormulaTokenArray();
     /** Assignment with incrementing references of FormulaToken entries
         (not copied!) */
@@ -390,20 +382,28 @@ public:
     bool      IsHyperLink() const       { return bHyperLink; }
 
     ScRecalcMode    GetRecalcMode() const { return nMode; }
-                            /** Bits aren't set directly but validated and
-                                maybe handled according to priority if more
-                                than one exclusive bit was set. */
-            void            AddRecalcMode( ScRecalcMode nBits );
+
+    void            SetCombinedBitsRecalcMode( ScRecalcMode nBits )
+                                { nMode |= (nBits & ~ScRecalcMode::EMask); }
+    ScRecalcMode    GetCombinedBitsRecalcMode() const
+                                { return nMode & ~ScRecalcMode::EMask; }
+
+                    /** Exclusive bits already set in nMode are zero'ed, nBits
+                        may contain combined bits, but only one exclusive bit
+                        may be set! */
+    void            SetMaskedRecalcMode( ScRecalcMode nBits )
+                                { nMode = GetCombinedBitsRecalcMode() | nBits; }
+
+                    /** Bits aren't set directly but validated and handled
+                        according to priority if more than one exclusive bit
+                        was set. */
+    void            AddRecalcMode( ScRecalcMode nBits );
 
     void            ClearRecalcMode() { nMode = ScRecalcMode::NORMAL; }
     void            SetExclusiveRecalcModeNormal()
                                 { SetMaskedRecalcMode( ScRecalcMode::NORMAL ); }
     void            SetExclusiveRecalcModeAlways()
                                 { SetMaskedRecalcMode( ScRecalcMode::ALWAYS ); }
-    void            SetExclusiveRecalcModeOnLoad()
-                                { SetMaskedRecalcMode( ScRecalcMode::ONLOAD ); }
-    void            SetExclusiveRecalcModeOnLoadOnce()
-                                { SetMaskedRecalcMode( ScRecalcMode::ONLOAD_ONCE ); }
     void            SetRecalcModeForced()
                                 { nMode |= ScRecalcMode::FORCED; }
     void            SetRecalcModeOnRefMove()
@@ -412,14 +412,14 @@ public:
                                 { return bool(nMode & ScRecalcMode::NORMAL); }
     bool            IsRecalcModeAlways() const
                                 { return bool(nMode & ScRecalcMode::ALWAYS); }
-    bool            IsRecalcModeOnLoad() const
-                                { return bool(nMode & ScRecalcMode::ONLOAD); }
-    bool            IsRecalcModeOnLoadOnce() const
-                                { return bool(nMode & ScRecalcMode::ONLOAD_ONCE); }
     bool            IsRecalcModeForced() const
                                 { return bool(nMode & ScRecalcMode::FORCED); }
     bool            IsRecalcModeOnRefMove() const
                                 { return bool(nMode & ScRecalcMode::ONREFMOVE); }
+                    /** Whether recalculation must happen after import, for
+                        example OOXML. */
+    bool            IsRecalcModeMustAfterImport() const
+                                { return (nMode & ScRecalcMode::EMask) <= ScRecalcMode::ONLOAD_ONCE; }
 
                             /** Get OpCode of the most outer function */
     inline OpCode           GetOuterFuncOpCode();
diff --git a/sc/inc/documentimport.hxx b/sc/inc/documentimport.hxx
index f902e1858741..1a8b1cbd514f 100644
--- a/sc/inc/documentimport.hxx
+++ b/sc/inc/documentimport.hxx
@@ -125,8 +125,15 @@ public:
 
     void finalize();
 
+    /** Broadcast all formula cells that are marked with
+        FormulaTokenArray::IsRecalcModeMustAfterImport() for a subsequent
+        ScDocument::CalcFormulaTree().
+     */
+    void broadcastRecalcAfterImport();
+
 private:
     void initColumn(ScColumn& rCol);
+    void broadcastRecalcAfterImportColumn(ScColumn& rCol);
 };
 
 #endif
diff --git a/sc/source/core/data/documentimport.cxx b/sc/source/core/data/documentimport.cxx
index 55aaadae1ce7..a7a3ba8e0326 100644
--- a/sc/source/core/data/documentimport.cxx
+++ b/sc/source/core/data/documentimport.cxx
@@ -22,6 +22,8 @@
 #include <listenercontext.hxx>
 #include <attarray.hxx>
 #include <sharedformula.hxx>
+#include <bcaslot.hxx>
+#include <scopetools.hxx>
 
 #include <svl/sharedstringpool.hxx>
 #include <svl/languageoptions.hxx>
@@ -720,4 +722,54 @@ void ScDocumentImport::initColumn(ScColumn& rCol)
     rCol.CellStorageModified();
 }
 
+namespace {
+
+class CellStoreAfterImportBroadcaster
+{
+public:
+
+    CellStoreAfterImportBroadcaster() {}
+
+    void operator() (const sc::CellStoreType::value_type& node)
+    {
+        if (node.type == sc::element_type_formula)
+        {
+            // Broadcast all formula cells marked for recalc.
+            ScFormulaCell** pp = &sc::formula_block::at(*node.data, 0);
+            ScFormulaCell** ppEnd = pp + node.size;
+            for (; pp != ppEnd; ++pp)
+            {
+                if ((*pp)->GetCode()->IsRecalcModeMustAfterImport())
+                    (*pp)->SetDirty();
+            }
+        }
+    }
+};
+
+}
+
+void ScDocumentImport::broadcastRecalcAfterImport()
+{
+    sc::AutoCalcSwitch aACSwitch( mpImpl->mrDoc, false);
+    ScBulkBroadcast aBulkBroadcast( mpImpl->mrDoc.GetBASM(), SfxHintId::ScDataChanged);
+
+    ScDocument::TableContainer::iterator itTab = mpImpl->mrDoc.maTabs.begin(), itTabEnd = mpImpl->mrDoc.maTabs.end();
+    for (; itTab != itTabEnd; ++itTab)
+    {
+        if (!*itTab)
+            continue;
+
+        ScTable& rTab = **itTab;
+        SCCOL nNumCols = rTab.aCol.size();
+        for (SCCOL nColIdx = 0; nColIdx < nNumCols; ++nColIdx)
+            broadcastRecalcAfterImportColumn(rTab.aCol[nColIdx]);
+    }
+}
+
+void ScDocumentImport::broadcastRecalcAfterImportColumn(ScColumn& rCol)
+{
+    CellStoreAfterImportBroadcaster aFunc;
+    std::for_each(rCol.maCells.begin(), rCol.maCells.end(), aFunc);
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index d20ea590441d..b7465341b15e 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -1366,8 +1366,7 @@ void ScFormulaCell::CompileXML( sc::CompileFormulaContext& rCxt, ScProgress& rPr
     pDocument->CheckLinkFormulaNeedingCheck(*pCode);
 
     //volatile cells must be added here for import
-    if( pCode->IsRecalcModeAlways() || pCode->IsRecalcModeForced() ||
-        pCode->IsRecalcModeOnLoad() || pCode->IsRecalcModeOnLoadOnce() )
+    if( !pCode->IsRecalcModeNormal() || pCode->IsRecalcModeForced())
     {
         // During load, only those cells that are marked explicitly dirty get
         // recalculated.  So we need to set it dirty here.
diff --git a/sc/source/core/tool/interpr2.cxx b/sc/source/core/tool/interpr2.cxx
index 7846d0d162b6..45cefa1d0423 100644
--- a/sc/source/core/tool/interpr2.cxx
+++ b/sc/source/core/tool/interpr2.cxx
@@ -2748,10 +2748,8 @@ void ScInterpreter::ScDde()
             return;
         }
 
-            // Need to reinterpret after loading (build links)
-
-        if ( rArr.IsRecalcModeNormal() )
-            rArr.SetExclusiveRecalcModeOnLoad();
+        // Need to reinterpret after loading (build links)
+        rArr.AddRecalcMode( ScRecalcMode::ONLOAD_LENIENT );
 
             //  while the link is not evaluated, idle must be disabled (to avoid circular references)
 
diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx
index 7c149d95d94a..ecb3bdb95e07 100644
--- a/sc/source/core/tool/interpr4.cxx
+++ b/sc/source/core/tool/interpr4.cxx
@@ -2698,8 +2698,7 @@ void ScInterpreter::ScExternal()
                 else
                 {
                     // enable asyncs after loading
-                    if ( rArr.IsRecalcModeNormal() )
-                        rArr.SetExclusiveRecalcModeOnLoad();
+                    rArr.AddRecalcMode( ScRecalcMode::ONLOAD_LENIENT );
                     // assure identical handler with identical call?
                     double nErg = 0.0;
                     ppParam[0] = &nErg;
@@ -3059,10 +3058,7 @@ void ScInterpreter::ScExternal()
 
             if ( aCall.HasVarRes() )                        // handle async functions
             {
-                if ( rArr.IsRecalcModeNormal() )
-                {
-                    rArr.SetExclusiveRecalcModeOnLoad();
-                }
+                rArr.AddRecalcMode( ScRecalcMode::ONLOAD_LENIENT );
                 uno::Reference<sheet::XVolatileResult> xRes = aCall.GetVarRes();
                 ScAddInListener* pLis = ScAddInListener::Get( xRes );
                 if ( !pLis )
diff --git a/sc/source/core/tool/interpr7.cxx b/sc/source/core/tool/interpr7.cxx
index 274bb69e5243..7b11e6faddab 100644
--- a/sc/source/core/tool/interpr7.cxx
+++ b/sc/source/core/tool/interpr7.cxx
@@ -310,8 +310,7 @@ void ScInterpreter::ScWebservice()
         }
 
         // Need to reinterpret after loading (build links)
-        if (rArr.IsRecalcModeNormal())
-            rArr.SetExclusiveRecalcModeOnLoad();
+        rArr.AddRecalcMode( ScRecalcMode::ONLOAD_LENIENT );
 
         //  while the link is not evaluated, idle must be disabled (to avoid circular references)
         bool bOldEnabled = pDok->IsIdleEnabled();
diff --git a/sc/source/filter/oox/formulabuffer.cxx b/sc/source/filter/oox/formulabuffer.cxx
index f09a11244de9..feb1bc6d8908 100644
--- a/sc/source/filter/oox/formulabuffer.cxx
+++ b/sc/source/filter/oox/formulabuffer.cxx
@@ -158,6 +158,11 @@ void applySharedFormulas(
                 case XML_n:
                     // numeric value.
                     pCell->SetResultDouble(rDesc.maCellValue.toDouble());
+                    /* TODO: is it on purpose that we never reset dirty here
+                     * and thus recalculate anyway if cell was dirty? Or is it
+                     * never dirty and therefor set dirty below otherwise? This
+                     * is different from the non-shared case in
+                     * applyCellFormulaValues(). */
                 break;
                 case XML_str:
                     if (bGeneratorKnownGood)
diff --git a/sc/source/filter/oox/workbookfragment.cxx b/sc/source/filter/oox/workbookfragment.cxx
index 0899b3c557b8..75bd31cb6397 100644
--- a/sc/source/filter/oox/workbookfragment.cxx
+++ b/sc/source/filter/oox/workbookfragment.cxx
@@ -575,7 +575,10 @@ void WorkbookFragment::recalcFormulaCells()
     if (bHardRecalc)
         rDocSh.DoHardRecalc();
     else
+    {
+        getDocImport().broadcastRecalcAfterImport();
         rDoc.CalcFormulaTree(false, true, false);
+    }
 }
 
 // private --------------------------------------------------------------------