Bug 27677

The patch adds Observer.Recover support to AbstractClientObserver.
The param is passed in the new constructor of AbstractClientObserver to avoid breaking API/ABI adding a new virtual method.
The old constructor sets the recover flag to false by default, to keep compatibility with older versions.

This is more or less what I had in mind, but:

* I don't like the method name recover() - it sounds like it ought to mean "try to recover [something] [from something] and return true on success" rather than just being an accessor. Could it be hasRecover() or willRecover() or something?

* Could the old (filter) constructor be implemented as a call to the new (filter, bool) constructor? Do C++ constructors work like that?

AbstractClientObserver::AbstractClientObserver(... filter)
    : AbstractClientObserver(filter, false)
{}

AbstractClientObserver::AbstractClientObserver(... filter, bool recover)
    : mPriv(new Private)
{
    mPriv->channelFilter = channelFilter;
    mPriv->recover = recover;
}

(If that doesn't work in C++, then never mind.)

* It'd be nice to have a convention for noting "what we'll do when we break ABI" - perhaps a comment containing "ABI-break"? - and mark these two constructors as "// ABI-break: combine (filter, bool) and (filter) constructors into (filter, bool=false)", or something

(The first of those points, the name of the recover() method, is the only one I feel strongly about.)

(In reply to comment #2)
> This is more or less what I had in mind, but:
> 
> * I don't like the method name recover() - it sounds like it ought to mean "try
> to recover [something] [from something] and return true on success" rather than
> just being an accessor. Could it be hasRecover() or willRecover() or something?
Updated. Changed to shouldRecover as agreed on IRC.

> * Could the old (filter) constructor be implemented as a call to the new
> (filter, bool) constructor? Do C++ constructors work like that?
> 
> AbstractClientObserver::AbstractClientObserver(... filter)
>     : AbstractClientObserver(filter, false)
> {}
> 
> AbstractClientObserver::AbstractClientObserver(... filter, bool recover)
>     : mPriv(new Private)
> {
>     mPriv->channelFilter = channelFilter;
>     mPriv->recover = recover;
> }
> 
> (If that doesn't work in C++, then never mind.)
Unfortunately that is not possible in C++, but I improved this and moved the members initialization to a new Private class constructor.

> * It'd be nice to have a convention for noting "what we'll do when we break
> ABI" - perhaps a comment containing "ABI-break"? - and mark these two
> constructors as "// ABI-break: combine (filter, bool) and (filter) constructors
> into (filter, bool=false)", or something
I didn't change that but I agree that should have a convention here, let's discuss this more and then we can do it later.

Better. Ship it!

Merged upstream. It will be in next release 0.3.2