some notes on SwClient and SwModify

[Bjoern Michaelsen]

Hi,

as you might have noted I did some changes deep in the bowels of LibreOffice
Writer, namely the SwClient/SwModify classes that almost all Writer objects
derive from. So what do these do? They are a rather unfortunate attempt at
implementing the observer pattern. Namely, the original intend was to allow all
SwClients to watch a SwModify for events:

- You could call pModify->ModifyBroadcast(...) on a SwModify, which ...
- ... hands the arguments to all SwClients on this SwClient and calls the
  event handler pClient->Modify(...) on them. The pClient deregistering from
  the pModify in this event handler should be handled gracefully.

This sounds not too bad so far, however it has been tainted by some design
decisions, namely:
- The clients are kept in a intrusive double linked list, thus each client can
  only observe at most _one_ SwModify (and creating likely unexpected behaviour
  in "dreaded diamond" classes).
- The "message" is a rather unwieldy pair of SfxPoolItem pointers.
- pModify->ModifyBroadcast(...) was extended to allow sending the message to
  only a specific "type" (as in tools/rtti.hxx type), thus breaking the
  possible dependency inversion by the observer pattern
- There are some weird side-effects triggered by 'magic bools' in SwModify.
- If a SwModify is dying it re-registers all SwClients registered at it on the
  object it itself is registered in as a client.

This has lead to some hacks around this design, making things even more
"interesting":
- Often, instead of passing the clumsy two-pointer message to
  ModifyBroadcast(), instead the SwModify directly iterates over clients and
  does downcasting to interesting classes and triggers code on those,
  creating really tight coupling.
- Because of the "client can only listen to one modify" limitation, hacks are
  needed for objects that need to observe more event sources. This is SwDepend.
- Because of the "client can only listen to one modify", many SwClients are
  assumed only registered to a specific SwModify: They then use the
  GetRegisteredIn() function to find that SwModify and downcast it to whatever
  they expect it to be.

So, what did my recent changes do? In fact not, as much as should be done, but
its a start:
- add unittests for these
- allow inlining most of this code
- killed the untyped SwClientIter, the slightly better (because stronger typed)
  SwIterator<> template class should be used instead
- added a template specialization for SwIterator<SwClient,...>, which should
  make it as fast as the old SwClientIter for iterating over (~untyped) SwClients
- mba already added the option to send a more generic SfxHint& as a message to
  clients, with the ultimate goal to unify this SwClient/SwModify mess with the
  SfxBroadcaster/SfxListener implementation, to have at least one (weird)
  observer pattern less in our codebase. These changes make SfxHints the
  default message -- the old SfxPoolItem*-pairs are tunneled through these.
- made the SwIterator, SwDepend and LegacyModifyHint classes final
- killed some accessor functions, that were unused and should rather be kept
  private 

IMHO what needs to happen to further untangle this mess:
- Pass final classes derived from SfxHint as messages, instead of the clumsy
  old SfxPoolItem*-pair where appropriate.
- While SwIterator<> gives some more typesafety than the old SwClientIter, it
  still creates quite a mess dependency-wise. Also replace these with passing a
  SfxHint& that is just handled properly by the receiving SwClients.
- Often something like SwIterator<Foo,...>(this).First() is used with
  confidence that there is only exactly one registree of a certain type.
  Instead of iterating through a possibly longer list of clients, consider having
  a point reserved for this specific client at the modify.
- Ultimately: Use a SfxBroadcaster/SfxListener API only in Writer, instead of
  yet-another-message-passing. And yes, SfxBroadcaster etc. might be
  wrapped/merged down itself around something like boost::signals/whatever.
- Ultimately: Slowly soften the implied "everything in writer need to derive
  from SwClient" situation.
- Ultimately: Once this SwIterator<> mess is gone, we also have one barrier
  less for getting rid of the hackish tools/rtti.hxx stuff.

Given that:

 grp SwIterator sw/|wc -l

shows 220 cases of SwIterator being used (58 of which seemingly only interested
in just using .First() on it once), this is still a bit of work though ...

FWIW, the changes done so far killed some ~150 LOC and were measured to have no
negative performance impact (actually 0.1% faster according to callgrind).

If there is passionate objection to any of the above, we might discuss it on
the next ESC call.

Best,

Bjoern