Implement concrete class RefCountedBase.

Implement concrete class RefCountedBase.

BUG=

[nisse-webrtc, 2017-09-01 08:56:13]

nisse@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[nisse-webrtc, 2017-09-01 08:56:14]

Please have a look, it didn't get so bad.

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
File webrtc/rtc_base/refcountedobject.h (right):

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
webrtc/rtc_base/refcountedobject.h:49: class RefCountedObject : public T,
protected virtual RefCountedBase {
I understand roughly why "protected virtual" inheritance is needed. Scary...

[kwiberg-webrtc, 2017-09-01 09:08:19]

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
File webrtc/rtc_base/refcountedobject.h (right):

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
webrtc/rtc_base/refcountedobject.h:43: virtual ~RefCountedBase() {}
This one doesn't have to be virtual, right?
And you should probably define it with =default, since that will allow the
compiler to treat it as trivially destructible IIRC.

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
webrtc/rtc_base/refcountedobject.h:49: class RefCountedObject : public T,
protected virtual RefCountedBase {
On 2017/09/01 08:56:14, nisse-webrtc wrote:
> I understand roughly why "protected virtual" inheritance is needed. Scary...

I would have thought that just "private" would suffice. Why does it have to be
virtual? (A private base works kind of like a data member, and ref_count_ used
to be a data member of this class.)

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
webrtc/rtc_base/refcountedobject.h:67: virtual ~RefCountedObject() {}
=default here too, although it doesn't matter since the virtualness of the
destructor probably makes the class non-trivially destructible. But it's good to
get into the habit, like with ++i vs. i++.

[nisse-webrtc, 2017-09-01 09:32:51]

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
File webrtc/rtc_base/refcountedobject.h (right):

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
webrtc/rtc_base/refcountedobject.h:43: virtual ~RefCountedBase() {}
On 2017/09/01 09:08:19, kwiberg-webrtc wrote:
> This one doesn't have to be virtual, right?

I think it has to, to make sure that Release calls the right destructor. The way
I understand it, this class can be inherited in several steps, and it's just
that the AddRef and Release methods which are supposed to not be overridden
(BTW, whould they be marked final?)

> And you should probably define it with =default, since that will allow the
> compiler to treat it as trivially destructible IIRC.

I'll change to default, but if it's virtual, that shouldn't make much
difference, I guess?

https://codereview.webrtc.org/3010883002/diff/1/webrtc/rtc_base/refcountedobj...
webrtc/rtc_base/refcountedobject.h:49: class RefCountedObject : public T,
protected virtual RefCountedBase {
On 2017/09/01 09:08:19, kwiberg-webrtc wrote:
> On 2017/09/01 08:56:14, nisse-webrtc wrote:
> > I understand roughly why "protected virtual" inheritance is needed. Scary...
> 
> I would have thought that just "private" would suffice. 

Changing protected to private gives this error:

In file included from ../../webrtc/pc/rtpreceiver.cc:11:
../../webrtc/pc/rtpreceiver.h:42:7: error: inherited virtual base class
'rtc::RefCountedBase' has private destructor
class AudioRtpReceiver : public ObserverInterface,
      ^
../../webrtc/rtc_base/refcountedobject.h:49:36: note: declared private here
class RefCountedObject : public T, private virtual RefCountedBase {
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> Why does it have to be virtual?

If I drop virtual, I get this error:

../../webrtc/rtc_base/refcountedobject.h:49:36: error: direct base
'rtc::RefCountedBase' is inaccessible due to ambiguity:
    class rtc::RefCountedObject<class webrtc::MockRtpSender> -> class
webrtc::MockRtpSender -> rtc::RefCountedObject<RtpSenderInterface> -> class
rtc::RefCountedBase
    class rtc::RefCountedObject<class webrtc::MockRtpSender> -> class
rtc::RefCountedBase [-Werror,-Winaccessible-base]
class RefCountedObject : public T, protected RefCountedBase {
                                   ^~~~~~~~~~~~~~~~~~~~~~~~
../../webrtc/pc/trackmediainfomap_unittest.cc:55:11: note: in instantiation of
template class 'rtc::RefCountedObject<webrtc::MockRtpSender>' requested here
      new rtc::RefCountedObject<MockRtpSender>());
          ^

Maybe this is a bug elsewhere in the code where we get multiple RefCountedObject
wrappers, which earlier just gave us multiple unused ref_count_ members?