sw/source/core/doc: Stop abusing observer pattern for code obfuscation.

The old SwClient/SwModify combo is a questionable implementation of the
observer pattern (among other things). The one thing the observer
pattern is good for is dependency inversion: The creator of the message
does not need to know the type of the receiver. Calling the message
handling on the receiver directly introduces tight coupling and entirely
defeats the purpose, leaving us with the worst of both worlds.

In such case, at least be honest about the tight coupling and call a
somewhat more explicit member function of the target.

Thus introduce SwFootNoteInfo::UpdateFormatOrAttr,
SwContentNode::UpdateAttr, remove some useless SwFormatChg clutter in
Set{Foot,End}NodeInfo, and check for some invariants in
SwContentNode::SwClientNotify to limit some of the "a message are two
void pointers" madness.

Change-Id: I32a8d6973231bb5f65c9e144be72d5bcf98f3f44
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105104
Tested-by: Jenkins
Reviewed-by: Bjoern Michaelsen <bjoern.michaelsen@libreoffice.org>
This commit is contained in:
Bjoern Michaelsen
2020-11-01 10:35:40 +01:00
parent a449fbbb20
commit 59684a321e
15 changed files with 71 additions and 50 deletions
+1
View File
@@ -72,6 +72,7 @@ public:
void SetPrefix(const OUString& rSet) { m_sPrefix = rSet; }
void SetSuffix(const OUString& rSet) { m_sSuffix = rSet; }
void UpdateFormatOrAttr();
};
enum SwFootnotePos
+1
View File
@@ -484,6 +484,7 @@ public:
{
SwClientNotify(*this, sw::LegacyModifyHint(pOld, pNew));
}
void UpdateAttr(const SwUpdateAttr&);
private:
SwContentNode( const SwContentNode & rNode ) = delete;
+2 -2
View File
@@ -248,8 +248,8 @@ namespace
auto InvalidatePosition(SwPosition const& rPos) -> void
{
SwUpdateAttr const hint(rPos.nContent.GetIndex(), rPos.nContent.GetIndex(), 0);
rPos.nNode.GetNode().GetTextNode()->NotifyClients(nullptr, &hint);
SwUpdateAttr const aHint(rPos.nContent.GetIndex(), rPos.nContent.GetIndex(), 0);
rPos.nNode.GetNode().GetTextNode()->CallSwClientNotify(sw::LegacyModifyHint(&aHint, &aHint));
}
}
@@ -2691,8 +2691,8 @@ bool DocumentContentOperationsManager::Overwrite( const SwPaM &rRg, const OUStri
? pNode->GetpSwpHints()->Count() : 0;
if( nOldAttrCnt != nNewAttrCnt )
{
SwUpdateAttr aHint(0,0,0);
pNode->ModifyBroadcast(nullptr, &aHint);
const SwUpdateAttr aHint(0,0,0);
pNode->TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
}
if (!m_rDoc.GetIDocumentUndoRedo().DoesUndo() &&
+2 -1
View File
@@ -194,7 +194,8 @@ IMPL_LINK_NOARG( DocumentTimerManager, DoIdleJobs, Timer*, void )
const bool bOldLockView = pShell->IsViewLocked();
pShell->LockView( true );
m_rDoc.getIDocumentFieldsAccess().GetSysFieldType( SwFieldIds::Chapter )->ModifyNotification( nullptr, nullptr ); // ChapterField
auto pChapterFieldType = m_rDoc.getIDocumentFieldsAccess().GetSysFieldType( SwFieldIds::Chapter );
pChapterFieldType->SwClientNotify(*pChapterFieldType, sw::LegacyModifyHint( nullptr, nullptr )); // ChapterField
m_rDoc.getIDocumentFieldsAccess().UpdateExpFields( nullptr, false ); // Updates ExpressionFields
m_rDoc.getIDocumentFieldsAccess().UpdateTableFields(nullptr); // Tables
m_rDoc.getIDocumentFieldsAccess().UpdateRefFields(); // References
+2 -3
View File
@@ -786,8 +786,7 @@ IMPL_LINK_NOARG( SwDoc, DoUpdateModifiedOLE, Timer *, void )
::StartProgress( STR_STATSTR_SWGPRTOLENOTIFY,
0, pNodes->size(), GetDocShell());
getIDocumentLayoutAccess().GetCurrentLayout()->StartAllAction();
SwMsgPoolItem aMsgHint( RES_UPDATE_ATTR );
SwUpdateAttr aHint(0,0,0);
for( SwOLENodes::size_type i = 0; i < pNodes->size(); ++i )
{
::SetProgressState( i, GetDocShell() );
@@ -799,7 +798,7 @@ IMPL_LINK_NOARG( SwDoc, DoUpdateModifiedOLE, Timer *, void )
// If it doesn't want to be informed
if( pOLENd->GetOLEObj().GetOleRef().is() ) // Broken?
{
pOLENd->ModifyNotification( &aMsgHint, &aMsgHint );
pOLENd->UpdateAttr(aHint);
}
}
getIDocumentLayoutAccess().GetCurrentLayout()->EndAllAction();
+1 -1
View File
@@ -920,7 +920,7 @@ void SwDocUpdateField::MakeFieldList_( SwDoc& rDoc, int eGetMode )
sFormula.clear();
// trigger formatting
const_cast<SwFormatField*>(pFormatField)->ModifyNotification( nullptr, nullptr );
const_cast<SwFormatField*>(pFormatField)->UpdateTextNode( nullptr, nullptr );
}
break;
+2 -2
View File
@@ -629,7 +629,7 @@ void SwDoc::SetDefault( const SfxItemSet& rSet )
{
SwFormatChg aChgFormat( mpDfltCharFormat.get() );
// notify the frames
aCallMod.ModifyNotification( &aChgFormat, &aChgFormat );
aCallMod.SwClientNotify(aCallMod, sw::LegacyModifyHint( &aChgFormat, &aChgFormat ));
}
}
}
@@ -638,7 +638,7 @@ void SwDoc::SetDefault( const SfxItemSet& rSet )
{
SwAttrSetChg aChgOld( aOld, aOld );
SwAttrSetChg aChgNew( aNew, aNew );
aCallMod.ModifyNotification( &aChgOld, &aChgNew ); // all changed are sent
aCallMod.SwClientNotify(aCallMod, sw::LegacyModifyHint( &aChgOld, &aChgNew )); // all changed are sent
}
// remove the default formats from the object again
+25 -24
View File
@@ -210,30 +210,35 @@ SwCharFormat* SwEndNoteInfo::GetCurrentCharFormat(const bool bAnchor) const
: m_pCharFormat;
}
void SwEndNoteInfo::UpdateFormatOrAttr()
{
auto pFormat = GetCurrentCharFormat(m_pCharFormat == nullptr);
if (!pFormat || !m_aDepends.IsListeningTo(pFormat) || pFormat->IsFormatInDTOR())
return;
SwDoc* pDoc = pFormat->GetDoc();
SwFootnoteIdxs& rFootnoteIdxs = pDoc->GetFootnoteIdxs();
for(auto pTextFootnote : rFootnoteIdxs)
{
const SwFormatFootnote &rFootnote = pTextFootnote->GetFootnote();
if(rFootnote.IsEndNote() == m_bEndNote)
pTextFootnote->SetNumber(rFootnote.GetNumber(), rFootnote.GetNumberRLHidden(), rFootnote.GetNumStr());
}
}
void SwEndNoteInfo::SwClientNotify( const SwModify& rModify, const SfxHint& rHint)
{
if (auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
{
const sal_uInt16 nWhich = pLegacyHint->m_pOld ? pLegacyHint->m_pOld->Which() : pLegacyHint->m_pNew ? pLegacyHint->m_pNew->Which() : 0 ;
if (RES_ATTRSET_CHG == nWhich || RES_FMT_CHG == nWhich)
switch(pLegacyHint->GetWhich())
{
auto pFormat = GetCurrentCharFormat(m_pCharFormat == nullptr);
if (!pFormat || !m_aDepends.IsListeningTo(pFormat) || pFormat->IsFormatInDTOR())
return;
SwDoc* pDoc = pFormat->GetDoc();
SwFootnoteIdxs& rFootnoteIdxs = pDoc->GetFootnoteIdxs();
for( size_t nPos = 0; nPos < rFootnoteIdxs.size(); ++nPos )
{
SwTextFootnote *pTextFootnote = rFootnoteIdxs[ nPos ];
const SwFormatFootnote &rFootnote = pTextFootnote->GetFootnote();
if ( rFootnote.IsEndNote() == m_bEndNote )
{
pTextFootnote->SetNumber(rFootnote.GetNumber(), rFootnote.GetNumberRLHidden(), rFootnote.GetNumStr());
}
}
case RES_ATTRSET_CHG:
case RES_FMT_CHG:
UpdateFormatOrAttr();
break;
default:
CheckRegistration( pLegacyHint->m_pOld );
}
else
CheckRegistration( pLegacyHint->m_pOld );
}
else if (auto pModifyChangedHint = dynamic_cast<const sw::ModifyChangedHint*>(&rHint))
{
@@ -346,9 +351,7 @@ void SwDoc::SetFootnoteInfo(const SwFootnoteInfo& rInfo)
GetFootnoteIdxs().UpdateAllFootnote();
else if( bFootnoteChrFormats )
{
SwFormatChg aOld( pOldChrFormat );
SwFormatChg aNew( pNewChrFormat );
mpFootnoteInfo->ModifyNotification( &aOld, &aNew );
mpFootnoteInfo->UpdateFormatOrAttr();
}
// #i81002# no update during loading
@@ -415,9 +418,7 @@ void SwDoc::SetEndNoteInfo(const SwEndNoteInfo& rInfo)
GetFootnoteIdxs().UpdateAllFootnote();
else if( bFootnoteChrFormats )
{
SwFormatChg aOld( pOldChrFormat );
SwFormatChg aNew( pNewChrFormat );
mpEndNoteInfo->ModifyNotification( &aOld, &aNew );
mpEndNoteInfo->UpdateFormatOrAttr();
}
// #i81002# no update during loading
+1 -1
View File
@@ -1267,7 +1267,7 @@ void SwRangeRedline::InvalidateRange(Invalidation const eWhy)
n == nEndNd ? nEndCnt : pNd->GetText().getLength(),
RES_FMT_CHG);
pNd->ModifyNotification(&aHt, &aHt);
pNd->TriggerNodeUpdate(sw::LegacyModifyHint(&aHt, &aHt));
// SwUpdateAttr must be handled first, otherwise indexes are off
if (GetType() == RedlineType::Delete)
+25 -6
View File
@@ -69,6 +69,7 @@
#include <swcrsr.hxx>
#include <hints.hxx>
#include <frameformats.hxx>
#include <sal/backtrace.hxx>
using namespace ::com::sun::star::i18n;
@@ -1090,6 +1091,14 @@ SwContentNode::~SwContentNode()
if ( mpAttrSet && mbSetModifyAtAttr )
const_cast<SwAttrSet*>(static_cast<const SwAttrSet*>(mpAttrSet.get()))->SetModifyAtAttr( nullptr );
}
void SwContentNode::UpdateAttr(const SwUpdateAttr& rUpdate)
{
if (GetNodes().IsDocNodes()
&& IsTextNode()
&& RES_ATTRSET_CHG == rUpdate.getWhichAttr())
static_cast<SwTextNode*>(this)->SetCalcHiddenCharFlags();
CallSwClientNotify(sw::LegacyModifyHint(&rUpdate, &rUpdate));
}
void SwContentNode::SwClientNotify( const SwModify&, const SfxHint& rHint)
{
@@ -1155,12 +1164,22 @@ void SwContentNode::SwClientNotify( const SwModify&, const SfxHint& rHint)
break;
case RES_UPDATE_ATTR:
if (GetNodes().IsDocNodes()
&& IsTextNode()
&& pLegacyHint->m_pNew
&& RES_ATTRSET_CHG == static_cast<const SwUpdateAttr*>(pLegacyHint->m_pNew)->getWhichAttr())
bCalcHidden = true;
break;
// RES_UPDATE_ATTR _should_ always contain a SwUpdateAttr hint in old and new.
// However, faking one with just a basic SfxPoolItem setting a WhichId has been observed.
// This makes the crude "WhichId" type divert from the true type, which is bad.
// Thus we are asserting here, but falling back to an proper
// hint instead. so that we at least will not spread such poison further.
if(pLegacyHint->m_pNew != pLegacyHint->m_pOld)
{
auto pBT = sal::backtrace_get(20);
SAL_WARN("sw.core", "UpdateAttr not matching! " << sal::backtrace_to_string(pBT.get()));
}
assert(pLegacyHint->m_pNew == pLegacyHint->m_pOld);
assert(dynamic_cast<const SwUpdateAttr*>(pLegacyHint->m_pNew));
const SwUpdateAttr aFallbackHint(0,0,0);
const SwUpdateAttr& rUpdateAttr = pLegacyHint->m_pNew ? *static_cast<const SwUpdateAttr*>(pLegacyHint->m_pNew) : aFallbackHint;
UpdateAttr(rUpdateAttr);
return;
}
if(bSetParent && GetpSwAttrSet())
AttrSetHandleHelper::SetParent(mpAttrSet, *this, pFormatColl, pFormatColl);
+1 -1
View File
@@ -2307,7 +2307,7 @@ void SwRefPageGetFieldType::UpdateField( SwTextField const * pTextField,
}
}
// start formatting
const_cast<SwFormatField&>(pTextField->GetFormatField()).ModifyNotification( nullptr, nullptr );
const_cast<SwFormatField&>(pTextField->GetFormatField()).UpdateTextNode(nullptr, nullptr);
}
// queries for relative page numbering
+2 -2
View File
@@ -272,8 +272,8 @@ bool SwGrfNode::ReRead(
// create an updates for the frames
if( bReadGrf && bNewGrf )
{
SwMsgPoolItem aMsgHint( RES_UPDATE_ATTR );
lcl_SwClientNotify(*this, aMsgHint);
const SwUpdateAttr aHint(0,0,0);
lcl_SwClientNotify(*this, aHint);
}
return bReadGrf;
+1 -1
View File
@@ -1953,7 +1953,7 @@ void SwTextNode::CopyAttr( SwTextNode *pDest, const sal_Int32 nTextStartIdx,
nOldPos,
0);
pDest->ModifyNotification( nullptr, &aHint );
pDest->TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
}
}
+3 -4
View File
@@ -3221,7 +3221,7 @@ bool SwpHints::TryInsertHint(
nHtStart,
nWhich);
rNode.ModifyNotification(nullptr,&aHint);
rNode.TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
}
return true;
@@ -3298,9 +3298,8 @@ bool SwpHints::TryInsertHint(
// ... and notify listeners
if ( rNode.HasWriterListeners() )
{
SwUpdateAttr aHint(nHtStart, nHintEnd, nWhich, aWhichSublist);
rNode.ModifyNotification( nullptr, &aHint );
const SwUpdateAttr aHint(nHtStart, nHintEnd, nWhich, aWhichSublist);
rNode.TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
}
#ifdef DBG_UTIL